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

[Marc-André Lureau]

On Wed, Sep 20, 2017 at 5:09 PM, Jens Freimann <address@hidden> wrote:
> 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

When did recvmsg() return 0? It should only be called after a poll
IN/ERR, in case of data it should always return != 0, and if
disconnected, it returns 0.

> 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?

We want to treat disconnect as a panic condition imho, that the
library user is free to implement in different way (abort() clean
exit, reconnect etc).

Please explain your use case and how you ran into recvmsg() = 0 and
what you expect to happen at this point.

-- 
Marc-André Lureau