Re: [Qemu-devel] [PATCH v2 3/5] libvhost-user: quit when no more data received

[Jens Freimann]

On Tue, Sep 19, 2017 at 04:46:24PM +0000, Marc-Andr?? Lureau wrote:
> Hi
>
> On Tue, Aug 8, 2017 at 10:52 PM Jens Freimann <address@hidden> wrote:
> > From: Jens Freimann <address@hidden>
> >
> > End processing of messages when VHOST_USER_NONE
> > is received.
> >
> > Without this we run into a vubr_panic() call and get
> > "PANIC: Unhandled request: 0"
> >
> > Signed-off-by: Jens Freimann <address@hidden>
> > ---
> >  contrib/libvhost-user/libvhost-user.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/libvhost-user/libvhost-user.c 
> > b/contrib/libvhost-user/libvhost-user.c
> > index 9efb9dac0e..35fa0c5e56 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -161,7 +161,7 @@ vu_message_read(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
> >          rc = recvmsg(conn_fd, &msg, 0);
> >      } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> >
> > -    if (rc <= 0) {
> > +    if (rc < 0) {
> >          vu_panic(dev, "Error while recvmsg: %s", strerror(errno));
> >          return false;
> >      }
> > @@ -806,6 +806,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
> >          return vu_get_queue_num_exec(dev, vmsg);
> >      case VHOST_USER_SET_VRING_ENABLE:
> >          return vu_set_vring_enable_exec(dev, vmsg);
> > +    case VHOST_USER_NONE:
> > +        break;
>
>
> I am afraid this isn't working. vu_message_read() returns
> true/success, vu_process_message() returns false/no-reply, so
> vu_dispatch() will return success, and the caller has no clear way to
> know that the socket got disconnected. For me the vu_panic() was quite
> more appropriate here.
>
> What problem did this patch exactly solve?

The problem was that a VhostUserMsg of size 0 is considered an
error. But recvmsg() can return 0. When I ran my pxe
testcase using vhost-user-bridge I ran into vu_panic() because of this. 

This worked because VHOST_USER_NONE is defined as 0. Instead of
doing this we could just allow a vmsg size of zero and not tread it
as an error?


regards,
Jens