[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 3/3] libvhost-user: quit when no more data r
From: |
Jens Freimann |
Subject: |
Re: [Qemu-devel] [PATCH RFC 3/3] libvhost-user: quit when no more data received |
Date: |
Fri, 14 Jul 2017 12:12:07 +0200 |
User-agent: |
NeoMutt/20170609 (1.8.3) |
On Thu, Jul 13, 2017 at 04:01:34PM +0000, Marc-André Lureau wrote:
Hi
On Wed, Jul 12, 2017 at 1:47 PM Jens Freimann <address@hidden> wrote:
From: Jens Freimann <address@hidden>
When recvmsg() returns a message size of zero and
errno is ENOENT end processing of vhost-user messages.
The man page says that zero-length messages indicate a orderly
shutdown for stream sockets, but for datagram sockets they are
permitted as normal messages. So technically quitting when receiving
a zero-length packet is not correct here.
I found I can replace this entire patch with this hunk:
@@ -812,6 +812,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:
+ return true;
default:
vmsg_close_fds(vmsg);
vu_panic(dev, "Unhandled
request: %d",
vmsg->request);
This works, because vmsg is initialzed to all zeros. When we read a
message of length zero, vmsg->request is 0, which is VHOST_USER_NONE.
Would this be acceptable?
Without this we run into a vubr_panic() call and get
PANIC: Error while recvmsg: No such file or directory
Error while dispatching.
How do you get ENOENT on recvmsg()?
I don't. I looked at errno when the return code of recvmsg() was 0,
but errno is only set in case of an error. So it is has no meaning
regarding recvmsg().
Add a switch "quit" to the vhost user device and set true to stop
processing messages.
Signed-off-by: Jens Freimann <address@hidden>
---
contrib/libvhost-user/libvhost-user.c | 12 +++++++++++-
contrib/libvhost-user/libvhost-user.h | 1 +
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/contrib/libvhost-user/libvhost-user.c
b/contrib/libvhost-user/libvhost-user.c
index 9efb9da..5538859 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -161,7 +161,10 @@ 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 && (errno == ENOENT)) {
+ vmsg->size = 0;
+ dev->quit = true;
+ } else if (rc < 0) {
vu_panic(dev, "Error while recvmsg: %s", strerror(errno));
return false;
}
@@ -755,6 +758,10 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("Flags: 0x%x\n", vmsg->flags);
DPRINT("Size: %d\n", vmsg->size);
+ if (dev->quit) {
+ return true;
+ }
Make it false, as no reply is expected then
ok
+
if (vmsg->fd_num) {
int i;
DPRINT("Fds:");
@@ -822,6 +829,9 @@ vu_dispatch(VuDev *dev)
bool success = false;
if (!vu_message_read(dev, dev->sock, &vmsg)) {
+ if (vmsg.size == 0) {
+ success = true;
+ }
There might be better ways to indicate a disconnection than modifying and
checking vmsg.size. Perhaps dev->quit alone is enough?
yes, it is enough. I'll change it.
goto end;
}
diff --git a/contrib/libvhost-user/libvhost-user.h
b/contrib/libvhost-user/libvhost-user.h
index 53ef222..c02215a 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -217,6 +217,7 @@ struct VuDev {
uint64_t features;
uint64_t protocol_features;
bool broken;
+ bool quit;
I also think you could re-use broken in this case.
I could use broken, it just felt wrong because in this case the device
is not broken, we just want to quit receiving messages because there is
no more data to be received. But if this is not enough to justify a
new variable, I'm fine with using broken.
Thanks for the review!
regards,
Jens
/* @set_watch: add or update the given fd to the watch set,
* call cb when condition is met */
--
2.9.4
--
Marc-André Lureau
- [Qemu-devel] [PATCH RFC 0/3] tests/pxe-testt: add testcase using vhost-user-bridge, Jens Freimann, 2017/07/12
- [Qemu-devel] [PATCH RFC 1/3] tests/vhost-user-bridge: disable debug output by default, Jens Freimann, 2017/07/12
- [Qemu-devel] [PATCH RFC 2/3] tests/pxe-test: add testcase using vhost-user-bridge, Jens Freimann, 2017/07/12
- [Qemu-devel] [PATCH RFC 3/3] libvhost-user: quit when no more data received, Jens Freimann, 2017/07/12
- Re: [Qemu-devel] [PATCH RFC 0/3] tests/pxe-testt: add testcase using vhost-user-bridge, no-reply, 2017/07/12
- Re: [Qemu-devel] [PATCH RFC 0/3] tests/pxe-testt: add testcase using vhost-user-bridge, no-reply, 2017/07/12
- Re: [Qemu-devel] [PATCH RFC 0/3] tests/pxe-testt: add testcase using vhost-user-bridge, Michael S. Tsirkin, 2017/07/12