|
From: | Jens Freimann |
Subject: | Re: [Qemu-devel] [PATCH v2 3/5] libvhost-user: quit when no more data received |
Date: | Thu, 21 Sep 2017 15:31:37 +0200 |
User-agent: | NeoMutt/20170914 (1.9.0) |
On Wed, Sep 20, 2017 at 04:14:33PM +0000, Marc-Andr?? Lureau wrote:
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 pxeWhen 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.
My usecase is that my testcase starts a pxeboot data transfer that goes through vhost-user-bridge. When the transfer is done the socket is disconnected and recvmsg() returns 0. I want vhost-user-bridge to end gracefully, without spitting out an error message. Is thatreasonable?
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).
Ok, that wasn't obvious to me. Thanks for clarifying! So I was "fixing" the wrong part. On a disconnect vubr_panic() in vhost-user-bridge.c is called and is supposed to do the right thing.
Currently it will print an error message "PANIC: Unhandled request: 0" in my use case. I could ignore that or check for exactly this string and suppress the error message. Both seems a bit ugly to me...
Please explain your use case and how you ran into recvmsg() = 0 and what you expect to happen at this point.
Hope this helps! Looks like we can revert this patch. I'll send apatch to do this.
regards,Jens
[Prev in Thread] | Current Thread | [Next in Thread] |