Re: [PATCH] 9p: init_in_iov_from_pdu can truncate the size

[Christian Schoenebeck]

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.

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.

Best regards,
Christian Schoenebeck