Hi Michael, > On 16 Jan 2017, at 10:22, Michael S. Tsirkin <m...@redhat.com> wrote: > > On Thu, Jan 12, 2017 at 05:14:07PM -0800, Felipe Franciosi wrote: >> Currently, VQs are started as soon as a SET_VRING_KICK is received. That >> is too early in the VQ setup process, as the backend might not yet have >> a callfd to notify in case it received a kick and fully processed the >> request/command. This patch only starts a VQ when a SET_VRING_CALL is >> received. >> >> Signed-off-by: Felipe Franciosi <fel...@nutanix.com> > > Doesn't look right to me. > It should happen when the fd supplies becomes readable.
Apologies, but I'm confused now. What should happen when which fd becomes readable? I thought we were discussing this further down in the thread. My last comments are here: http://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg02755.html I pointed out what appears to be a race and am looking forward to your comments. There is definitely a problem with the current code, so it's a matter of finding out exactly what's happening and what's the correct (and spec-compliant) way to fix it. Felipe > >> --- >> contrib/libvhost-user/libvhost-user.c | 26 +++++++++++++------------- >> 1 file changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/contrib/libvhost-user/libvhost-user.c >> b/contrib/libvhost-user/libvhost-user.c >> index af4faad..a46ef90 100644 >> --- a/contrib/libvhost-user/libvhost-user.c >> +++ b/contrib/libvhost-user/libvhost-user.c >> @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg *vmsg) >> DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index); >> } >> >> - dev->vq[index].started = true; >> - if (dev->iface->queue_set_started) { >> - dev->iface->queue_set_started(dev, index, true); >> - } >> - >> - if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) { >> - dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN, >> - vu_kick_cb, (void *)(long)index); >> - >> - DPRINT("Waiting for kicks on fd: %d for vq: %d\n", >> - dev->vq[index].kick_fd, index); >> - } >> - >> return false; >> } >> >> @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg) >> >> DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index); >> >> + dev->vq[index].started = true; >> + if (dev->iface->queue_set_started) { >> + dev->iface->queue_set_started(dev, index, true); >> + } >> + >> + if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) { >> + dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN, >> + vu_kick_cb, (void *)(long)index); >> + >> + DPRINT("Waiting for kicks on fd: %d for vq: %d\n", >> + dev->vq[index].kick_fd, index); >> + } >> + >> return false; >> } >> >> -- >> 1.9.4