Re: [PATCH v4 0/5] vhost-user block device backend implementation

[Coiby Xu]

Thank you for reminding me of this socket short read issue! It seems
we still need customized vu_message_read because libvhost-user assumes
we will always get a full-size VhostUserMsg and hasn't taken care of
this short read case. I will improve libvhost-user's vu_message_read
by making it keep reading from socket util getting enough bytes. I
assume short read is a rare case thus introduced performance penalty
would be negligible.


On Thu, Feb 27, 2020 at 3:41 PM Stefan Hajnoczi <address@hidden> wrote:
>
> On Wed, Feb 26, 2020 at 11:18:41PM +0800, Coiby Xu wrote:
> > Hi Stefan,
> >
> > Thank you for reviewing my code!
> >
> > I tried to reach you on IRC. But somehow either you missed my message
> > or I missed your reply. So I will reply by email instead.
> >
> > If we use qio_channel_set_aio_fd_handler to monitor G_IO_IN event,
> > i.e. use vu_dispatch as the read handler, then we can re-use
> > vu_message_read. And "removing the blocking recv from libvhost-user"
> > isn't necessary because "the operation of poll() and ppoll() is not
> > affected by the O_NONBLOCK flag" despite that we use
> > qio_channel_set_blocking before calling qio_channel_set_aio_fd_handler
> > to make recv non-blocking.
>
> I'm not sure I understand.  poll() just says whether the file descriptor
> is readable.  It does not say whether enough bytes are readable :).  So
> our callback will be invoked if there is 1 byte ready, but when we try
> to read 20 bytes either it will block (without O_NONBLOCK) or return
> only 1 byte (with O_NONBLOCK).  Neither case is okay, so I expect that
> code changes will be necessary.
>
> But please go ahead and send the next revision and I'll take a look.
>
> Stefan



--
Best regards,
Coiby