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

[Kevin Wolf]

Am 27.02.2020 um 12:07 hat Marc-André Lureau geschrieben:
> On Thu, Feb 27, 2020 at 11:55 AM Kevin Wolf <address@hidden> wrote:
> > Am 27.02.2020 um 11:28 hat Coiby Xu geschrieben:
> > > > > 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.
> > >
> > > > In any case, please make sure that we use the QIOChannel functions
> > > > called from a coroutine in QEMU so that it will never block, but the
> > > > coroutine can just yield while it's waiting for more bytes.
> > >
> > > But if I am not wrong, libvhost-user is supposed to be indepdent from
> > > the main QEMU code. So it can't use the QIOChannel functions if we
> > > simply modify exiting vu_message_read to address the short read issue.
> > > In v3 & v4, I extended libvhost-user to allow vu_message_read to be
> > > replaced by one which will depend on the main QEMU code. I'm not sure
> > > which way is better.
> >
> > The way your latest patches have it, with a separate read function,
> > works for me.
> 
> Done right, I am not against it, fwiw
> 
> > You could probably change libvhost-user to reimplement the same
> > functionality, and it might be an improvement for other users of the
> > library, but it's also code duplication and doesn't provide more value
> > in the context of the vhost-user export in QEMU.
> >
> > The point that's really important to me is just that we never block when
> > we run inside QEMU because that would actually stall the guest. This
> > means busy waiting in a tight loop until read() returns enough bytes is
> > not acceptable in QEMU.
> 
> In the context of vhost-user, local unix sockets with short messages
> (do we have >1k messages?), I am not sure if this is really a problem.

I'm not sure how much of a problem it is in practice, and whether we
can consider the vhost-user client trusted. But using QIOChannel from
within a coroutine just avoids the problem completely, so it feels like
a natural choice to just do that.

> And isn't it possible to run libvhost-user in its own thread for this
> series?

You need to run the actual block I/O requests in the iothread where the
block device happens to run. So if you move the message processing to a
separate thread, you would have to communicate between threads for the
actual request processing. Possible, but probably slower than necessary
and certainly more complex.

Kevin