On Mon, 6 Jan 2020, Greg Kurz wrote:
> On Mon, 06 Jan 2020 16:24:18 +0100
> Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
>
> > On Montag, 6. Januar 2020 14:42:54 CET Greg Kurz wrote:
> > > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > > index 775e8ff766..68873c3f5f 100644
> > > > --- a/hw/9pfs/virtio-9p-device.c
> > > > +++ b/hw/9pfs/virtio-9p-device.c
> > > > @@ -145,19 +145,15 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu,
> > > > size_t offset,>
> > > > }
> > > >
> > > > static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec
> > > > **piov,
> > > >
> > > > - unsigned int *pniov, size_t
> > > > size)
> > > > + unsigned int *pniov, size_t
> > > > *size)
> > > >
> > > > {
> > > >
> > > > V9fsState *s = pdu->s;
> > > > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> > > > VirtQueueElement *elem = v->elems[pdu->idx];
> > > > size_t buf_size = iov_size(elem->in_sg, elem->in_num);
> > > >
> > > > - if (buf_size < size) {
> > > > - VirtIODevice *vdev = VIRTIO_DEVICE(v);
> > > > -
> > > > - virtio_error(vdev,
> > > > - "VirtFS reply type %d needs %zu bytes, buffer has
> > > > %zu", - pdu->id + 1, size, buf_size);
> > > > + if (buf_size < *size) {
> > > > + *size = buf_size;
> > > >
> > > > }
> > >
> > > As suggested by Christian in some other mail, it could still make sense to
> > > raise the error if there isn't even enough space to pack a 9p message
> > > header.
> >
> > Another option: Instead of handling this as a hard error (which they
> > probably
> > try to avoid in their use case): putting the handler asleep for a while by
> > calling qemu_co_sleep_ns_wakeable() in this case. Then a bit later
> > transport
> > would eventually have the required buffer size and handler could continue
> > the
> > request without an error.
> >
>
> Waiting for an arbitrary amount of time (how much?) and retrying doesn't give
> any guarantee either that things will go smoothly. If anything, I'd rather
> have
> the transport to wake up the request when more buffer space gets available.
>
> > But this would require more care. For instance subsequent request handlers
> > would need to check if there was already an event handler asleep, and if so
> > it
> > would either need to wake it up or put itself asleep as well to prevent the
> > request order being processed by server being messed up.
> >
>
> And so on... ie. we would need to handle a queue of sleeping requests IIUC.
> Not really fan to go this way to address what looks like a corner case.
I agree with you that handling requests queuing is complex and overkill
for this. As I don't have a better suggestion, I am going to go with
retaining the error if the allocated buf_size is less than P9_IOHDRSZ.
