* [Qemu-devel] [PATCH] virtio: fix vhost handling
@ 2012-08-06 13:26 Paolo Bonzini
2012-08-06 13:39 ` Stefan Hajnoczi
2012-08-06 20:37 ` Anthony Liguori
0 siblings, 2 replies; 10+ messages in thread
From: Paolo Bonzini @ 2012-08-06 13:26 UTC (permalink / raw)
To: qemu-devel; +Cc: avi, Stefan Hajnoczi
Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
because it always registers the virtio_pci_host_notifier_read() handler
function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
The result is both QEMU and vhost_net.ko polling on the same eventfd
and the virtio_net.ko guest driver seeing inconsistent results:
# ifconfig eth0 192.168.0.1 netmask 255.255.255.0
virtio_net virtio0: output:id 0 is not a head!
To fix this, proceed the same as we do for irqfd: add a parameter to
virtio_queue_set_host_notifier_fd_handler and in that case only set
the notifier, not the handler.
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/virtio-pci.c | 14 +++++++-------
hw/virtio.c | 7 +++++--
hw/virtio.h | 3 ++-
3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 3ab9747..6133626 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -160,7 +160,7 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
}
static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
- int n, bool assign)
+ int n, bool assign, bool set_handler)
{
VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
@@ -173,13 +173,13 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
__func__, r);
return r;
}
- virtio_queue_set_host_notifier_fd_handler(vq, true);
+ virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
true, n, notifier);
} else {
memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
true, n, notifier);
- virtio_queue_set_host_notifier_fd_handler(vq, false);
+ virtio_queue_set_host_notifier_fd_handler(vq, false, false);
event_notifier_cleanup(notifier);
}
return r;
@@ -200,7 +200,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
continue;
}
- r = virtio_pci_set_host_notifier_internal(proxy, n, true);
+ r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
if (r < 0) {
goto assign_error;
}
@@ -214,7 +214,7 @@ assign_error:
continue;
}
- r = virtio_pci_set_host_notifier_internal(proxy, n, false);
+ r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
proxy->ioeventfd_started = false;
@@ -235,7 +235,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
continue;
}
- r = virtio_pci_set_host_notifier_internal(proxy, n, false);
+ r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
proxy->ioeventfd_started = false;
@@ -683,7 +683,7 @@ static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign)
* currently only stops on status change away from ok,
* reset, vmstop and such. If we do add code to start here,
* need to check vmstate, device state etc. */
- return virtio_pci_set_host_notifier_internal(proxy, n, assign);
+ return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
}
static void virtio_pci_vmstate_change(void *opaque, bool running)
diff --git a/hw/virtio.c b/hw/virtio.c
index d146f86..89e6d6f 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -1021,13 +1021,16 @@ static void virtio_queue_host_notifier_read(EventNotifier *n)
}
}
-void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign)
+void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
+ bool set_handler)
{
- if (assign) {
+ if (assign && set_handler) {
event_notifier_set_handler(&vq->host_notifier,
virtio_queue_host_notifier_read);
} else {
event_notifier_set_handler(&vq->host_notifier, NULL);
+ }
+ if (!assign) {
/* Test and clear notifier before after disabling event,
* in case poll callback didn't have time to run. */
virtio_queue_host_notifier_read(&vq->host_notifier);
diff --git a/hw/virtio.h b/hw/virtio.h
index f8b5535..d6a8ea3 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -233,7 +233,8 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
bool with_irqfd);
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
-void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign);
+void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
+ bool set_handler);
void virtio_queue_notify_vq(VirtQueue *vq);
void virtio_irq(VirtQueue *vq);
#endif
--
1.7.11.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-06 13:26 Paolo Bonzini
@ 2012-08-06 13:39 ` Stefan Hajnoczi
2012-08-06 13:51 ` Paolo Bonzini
2012-08-06 20:37 ` Anthony Liguori
1 sibling, 1 reply; 10+ messages in thread
From: Stefan Hajnoczi @ 2012-08-06 13:39 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Stefan Hajnoczi, avi
On Mon, Aug 6, 2012 at 2:26 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
> because it always registers the virtio_pci_host_notifier_read() handler
> function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
> The result is both QEMU and vhost_net.ko polling on the same eventfd
> and the virtio_net.ko guest driver seeing inconsistent results:
>
> # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
> virtio_net virtio0: output:id 0 is not a head!
>
> To fix this, proceed the same as we do for irqfd: add a parameter to
> virtio_queue_set_host_notifier_fd_handler and in that case only set
> the notifier, not the handler.
>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/virtio-pci.c | 14 +++++++-------
> hw/virtio.c | 7 +++++--
> hw/virtio.h | 3 ++-
> 3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
Internationalized diff stat, I never noticed... :)
Tested-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-06 13:39 ` Stefan Hajnoczi
@ 2012-08-06 13:51 ` Paolo Bonzini
0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2012-08-06 13:51 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Stefan Hajnoczi, avi
Il 06/08/2012 15:39, Stefan Hajnoczi ha scritto:
>> > hw/virtio-pci.c | 14 +++++++-------
>> > hw/virtio.c | 7 +++++--
>> > hw/virtio.h | 3 ++-
>> > 3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
> Internationalized diff stat, I never noticed... :)
Must be new with the latest update, and a bug IMHO (ok for git log, but
not for git format-patch).
> Tested-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] virtio: fix vhost handling
@ 2012-08-06 14:56 Paolo Bonzini
2012-08-06 15:47 ` Stefan Hajnoczi
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Paolo Bonzini @ 2012-08-06 14:56 UTC (permalink / raw)
To: qemu-devel; +Cc: avi, Stefan Hajnoczi
Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
because it always registers the virtio_pci_host_notifier_read() handler
function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
The result is both QEMU and vhost_net.ko polling on the same eventfd
and the virtio_net.ko guest driver seeing inconsistent results:
# ifconfig eth0 192.168.0.1 netmask 255.255.255.0
virtio_net virtio0: output:id 0 is not a head!
To fix this, proceed the same as we do for irqfd: add a parameter to
virtio_queue_set_host_notifier_fd_handler and in that case only set
the notifier, not the handler.
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
The patch is a bit different from what I posted before,
so I didn't add Stefan's *-by tags.
hw/virtio-pci.c | 14 +++++++-------
hw/virtio.c | 7 +++++--
hw/virtio.h | 3 ++-
3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 3ab9747..6133626 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -160,7 +160,7 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
}
static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
- int n, bool assign)
+ int n, bool assign, bool set_handler)
{
VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
@@ -173,13 +173,13 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
__func__, r);
return r;
}
- virtio_queue_set_host_notifier_fd_handler(vq, true);
+ virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
true, n, notifier);
} else {
memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
true, n, notifier);
- virtio_queue_set_host_notifier_fd_handler(vq, false);
+ virtio_queue_set_host_notifier_fd_handler(vq, false, false);
event_notifier_cleanup(notifier);
}
return r;
@@ -200,7 +200,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
continue;
}
- r = virtio_pci_set_host_notifier_internal(proxy, n, true);
+ r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
if (r < 0) {
goto assign_error;
}
@@ -214,7 +214,7 @@ assign_error:
continue;
}
- r = virtio_pci_set_host_notifier_internal(proxy, n, false);
+ r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
proxy->ioeventfd_started = false;
@@ -235,7 +235,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
continue;
}
- r = virtio_pci_set_host_notifier_internal(proxy, n, false);
+ r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
assert(r >= 0);
}
proxy->ioeventfd_started = false;
@@ -683,7 +683,7 @@ static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign)
* currently only stops on status change away from ok,
* reset, vmstop and such. If we do add code to start here,
* need to check vmstate, device state etc. */
- return virtio_pci_set_host_notifier_internal(proxy, n, assign);
+ return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
}
static void virtio_pci_vmstate_change(void *opaque, bool running)
diff --git a/hw/virtio.c b/hw/virtio.c
index d146f86..89e6d6f 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -1021,13 +1021,16 @@ static void virtio_queue_host_notifier_read(EventNotifier *n)
}
}
-void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign)
+void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
+ bool set_handler)
{
- if (assign) {
+ if (assign && set_handler) {
event_notifier_set_handler(&vq->host_notifier,
virtio_queue_host_notifier_read);
} else {
event_notifier_set_handler(&vq->host_notifier, NULL);
+ }
+ if (!assign) {
/* Test and clear notifier before after disabling event,
* in case poll callback didn't have time to run. */
virtio_queue_host_notifier_read(&vq->host_notifier);
diff --git a/hw/virtio.h b/hw/virtio.h
index f8b5535..d6a8ea3 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -233,7 +233,8 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
bool with_irqfd);
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
-void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign);
+void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
+ bool set_handler);
void virtio_queue_notify_vq(VirtQueue *vq);
void virtio_irq(VirtQueue *vq);
#endif
--
1.7.11.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-06 14:56 [Qemu-devel] [PATCH] virtio: fix vhost handling Paolo Bonzini
@ 2012-08-06 15:47 ` Stefan Hajnoczi
2012-08-12 8:58 ` Michael S. Tsirkin
2012-08-12 10:08 ` Michael Tokarev
2 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2012-08-06 15:47 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Stefan Hajnoczi, avi
On Mon, Aug 6, 2012 at 3:56 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
> because it always registers the virtio_pci_host_notifier_read() handler
> function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
> The result is both QEMU and vhost_net.ko polling on the same eventfd
> and the virtio_net.ko guest driver seeing inconsistent results:
>
> # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
> virtio_net virtio0: output:id 0 is not a head!
>
> To fix this, proceed the same as we do for irqfd: add a parameter to
> virtio_queue_set_host_notifier_fd_handler and in that case only set
> the notifier, not the handler.
>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> The patch is a bit different from what I posted before,
> so I didn't add Stefan's *-by tags.
>
> hw/virtio-pci.c | 14 +++++++-------
> hw/virtio.c | 7 +++++--
> hw/virtio.h | 3 ++-
> 3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
Tested-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-06 13:26 Paolo Bonzini
2012-08-06 13:39 ` Stefan Hajnoczi
@ 2012-08-06 20:37 ` Anthony Liguori
1 sibling, 0 replies; 10+ messages in thread
From: Anthony Liguori @ 2012-08-06 20:37 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: avi, Stefan Hajnoczi
Paolo Bonzini <pbonzini@redhat.com> writes:
> Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
> because it always registers the virtio_pci_host_notifier_read() handler
> function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
> The result is both QEMU and vhost_net.ko polling on the same eventfd
> and the virtio_net.ko guest driver seeing inconsistent results:
>
> # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
> virtio_net virtio0: output:id 0 is not a head!
>
> To fix this, proceed the same as we do for irqfd: add a parameter to
> virtio_queue_set_host_notifier_fd_handler and in that case only set
> the notifier, not the handler.
>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Applied. Thanks.
Regards,
Anthony Liguori
> ---
> hw/virtio-pci.c | 14 +++++++-------
> hw/virtio.c | 7 +++++--
> hw/virtio.h | 3 ++-
> 3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 3ab9747..6133626 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -160,7 +160,7 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
> }
>
> static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> - int n, bool assign)
> + int n, bool assign, bool set_handler)
> {
> VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
> EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> @@ -173,13 +173,13 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> __func__, r);
> return r;
> }
> - virtio_queue_set_host_notifier_fd_handler(vq, true);
> + virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
> true, n, notifier);
> } else {
> memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
> true, n, notifier);
> - virtio_queue_set_host_notifier_fd_handler(vq, false);
> + virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> event_notifier_cleanup(notifier);
> }
> return r;
> @@ -200,7 +200,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, true);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
> if (r < 0) {
> goto assign_error;
> }
> @@ -214,7 +214,7 @@ assign_error:
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -235,7 +235,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -683,7 +683,7 @@ static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign)
> * currently only stops on status change away from ok,
> * reset, vmstop and such. If we do add code to start here,
> * need to check vmstate, device state etc. */
> - return virtio_pci_set_host_notifier_internal(proxy, n, assign);
> + return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
> }
>
> static void virtio_pci_vmstate_change(void *opaque, bool running)
> diff --git a/hw/virtio.c b/hw/virtio.c
> index d146f86..89e6d6f 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -1021,13 +1021,16 @@ static void virtio_queue_host_notifier_read(EventNotifier *n)
> }
> }
>
> -void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign)
> +void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> + bool set_handler)
> {
> - if (assign) {
> + if (assign && set_handler) {
> event_notifier_set_handler(&vq->host_notifier,
> virtio_queue_host_notifier_read);
> } else {
> event_notifier_set_handler(&vq->host_notifier, NULL);
> + }
> + if (!assign) {
> /* Test and clear notifier before after disabling event,
> * in case poll callback didn't have time to run. */
> virtio_queue_host_notifier_read(&vq->host_notifier);
> diff --git a/hw/virtio.h b/hw/virtio.h
> index f8b5535..d6a8ea3 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -233,7 +233,8 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
> void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> bool with_irqfd);
> EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> -void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign);
> +void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> + bool set_handler);
> void virtio_queue_notify_vq(VirtQueue *vq);
> void virtio_irq(VirtQueue *vq);
> #endif
> --
> 1.7.11.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-06 14:56 [Qemu-devel] [PATCH] virtio: fix vhost handling Paolo Bonzini
2012-08-06 15:47 ` Stefan Hajnoczi
@ 2012-08-12 8:58 ` Michael S. Tsirkin
2012-08-12 10:08 ` Michael Tokarev
2 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-08-12 8:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Stefan Hajnoczi, avi
On Mon, Aug 06, 2012 at 04:56:10PM +0200, Paolo Bonzini wrote:
> Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
> because it always registers the virtio_pci_host_notifier_read() handler
> function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
> The result is both QEMU and vhost_net.ko polling on the same eventfd
> and the virtio_net.ko guest driver seeing inconsistent results:
>
> # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
> virtio_net virtio0: output:id 0 is not a head!
>
> To fix this, proceed the same as we do for irqfd: add a parameter to
> virtio_queue_set_host_notifier_fd_handler and in that case only set
> the notifier, not the handler.
>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Anthony, could you pick this bugfix up please?
> ---
> The patch is a bit different from what I posted before,
> so I didn't add Stefan's *-by tags.
>
> hw/virtio-pci.c | 14 +++++++-------
> hw/virtio.c | 7 +++++--
> hw/virtio.h | 3 ++-
> 3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 3ab9747..6133626 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -160,7 +160,7 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
> }
>
> static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> - int n, bool assign)
> + int n, bool assign, bool set_handler)
> {
> VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
> EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> @@ -173,13 +173,13 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> __func__, r);
> return r;
> }
> - virtio_queue_set_host_notifier_fd_handler(vq, true);
> + virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
> true, n, notifier);
> } else {
> memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
> true, n, notifier);
> - virtio_queue_set_host_notifier_fd_handler(vq, false);
> + virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> event_notifier_cleanup(notifier);
> }
> return r;
> @@ -200,7 +200,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, true);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
> if (r < 0) {
> goto assign_error;
> }
> @@ -214,7 +214,7 @@ assign_error:
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -235,7 +235,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -683,7 +683,7 @@ static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign)
> * currently only stops on status change away from ok,
> * reset, vmstop and such. If we do add code to start here,
> * need to check vmstate, device state etc. */
> - return virtio_pci_set_host_notifier_internal(proxy, n, assign);
> + return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
> }
>
> static void virtio_pci_vmstate_change(void *opaque, bool running)
> diff --git a/hw/virtio.c b/hw/virtio.c
> index d146f86..89e6d6f 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -1021,13 +1021,16 @@ static void virtio_queue_host_notifier_read(EventNotifier *n)
> }
> }
>
> -void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign)
> +void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> + bool set_handler)
> {
> - if (assign) {
> + if (assign && set_handler) {
> event_notifier_set_handler(&vq->host_notifier,
> virtio_queue_host_notifier_read);
> } else {
> event_notifier_set_handler(&vq->host_notifier, NULL);
> + }
> + if (!assign) {
> /* Test and clear notifier before after disabling event,
> * in case poll callback didn't have time to run. */
> virtio_queue_host_notifier_read(&vq->host_notifier);
> diff --git a/hw/virtio.h b/hw/virtio.h
> index f8b5535..d6a8ea3 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -233,7 +233,8 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
> void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> bool with_irqfd);
> EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> -void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign);
> +void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> + bool set_handler);
> void virtio_queue_notify_vq(VirtQueue *vq);
> void virtio_irq(VirtQueue *vq);
> #endif
> --
> 1.7.11.2
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-06 14:56 [Qemu-devel] [PATCH] virtio: fix vhost handling Paolo Bonzini
2012-08-06 15:47 ` Stefan Hajnoczi
2012-08-12 8:58 ` Michael S. Tsirkin
@ 2012-08-12 10:08 ` Michael Tokarev
2012-08-18 18:56 ` Paolo Bonzini
2 siblings, 1 reply; 10+ messages in thread
From: Michael Tokarev @ 2012-08-12 10:08 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Stefan Hajnoczi, avi
On 06.08.2012 18:56, Paolo Bonzini wrote:
> Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
> because it always registers the virtio_pci_host_notifier_read() handler
> function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
> The result is both QEMU and vhost_net.ko polling on the same eventfd
> and the virtio_net.ko guest driver seeing inconsistent results:
>
> # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
> virtio_net virtio0: output:id 0 is not a head!
>
> To fix this, proceed the same as we do for irqfd: add a parameter to
> virtio_queue_set_host_notifier_fd_handler and in that case only set
> the notifier, not the handler.
Stable-1.1 material? The mentioned commit is included into 1.1 release.
Thanks,
/mjt
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> The patch is a bit different from what I posted before,
> so I didn't add Stefan's *-by tags.
>
> hw/virtio-pci.c | 14 +++++++-------
> hw/virtio.c | 7 +++++--
> hw/virtio.h | 3 ++-
> 3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 3ab9747..6133626 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -160,7 +160,7 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
> }
>
> static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> - int n, bool assign)
> + int n, bool assign, bool set_handler)
> {
> VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
> EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> @@ -173,13 +173,13 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> __func__, r);
> return r;
> }
> - virtio_queue_set_host_notifier_fd_handler(vq, true);
> + virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
> true, n, notifier);
> } else {
> memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
> true, n, notifier);
> - virtio_queue_set_host_notifier_fd_handler(vq, false);
> + virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> event_notifier_cleanup(notifier);
> }
> return r;
> @@ -200,7 +200,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, true);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
> if (r < 0) {
> goto assign_error;
> }
> @@ -214,7 +214,7 @@ assign_error:
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -235,7 +235,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
> continue;
> }
>
> - r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> + r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -683,7 +683,7 @@ static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign)
> * currently only stops on status change away from ok,
> * reset, vmstop and such. If we do add code to start here,
> * need to check vmstate, device state etc. */
> - return virtio_pci_set_host_notifier_internal(proxy, n, assign);
> + return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
> }
>
> static void virtio_pci_vmstate_change(void *opaque, bool running)
> diff --git a/hw/virtio.c b/hw/virtio.c
> index d146f86..89e6d6f 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -1021,13 +1021,16 @@ static void virtio_queue_host_notifier_read(EventNotifier *n)
> }
> }
>
> -void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign)
> +void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> + bool set_handler)
> {
> - if (assign) {
> + if (assign && set_handler) {
> event_notifier_set_handler(&vq->host_notifier,
> virtio_queue_host_notifier_read);
> } else {
> event_notifier_set_handler(&vq->host_notifier, NULL);
> + }
> + if (!assign) {
> /* Test and clear notifier before after disabling event,
> * in case poll callback didn't have time to run. */
> virtio_queue_host_notifier_read(&vq->host_notifier);
> diff --git a/hw/virtio.h b/hw/virtio.h
> index f8b5535..d6a8ea3 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -233,7 +233,8 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
> void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> bool with_irqfd);
> EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> -void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign);
> +void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> + bool set_handler);
> void virtio_queue_notify_vq(VirtQueue *vq);
> void virtio_irq(VirtQueue *vq);
> #endif
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-12 10:08 ` Michael Tokarev
@ 2012-08-18 18:56 ` Paolo Bonzini
2012-08-18 18:58 ` Michael Tokarev
0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2012-08-18 18:56 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, Stefan Hajnoczi, avi
Il 12/08/2012 12:08, Michael Tokarev ha scritto:
>> > Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
>> > because it always registers the virtio_pci_host_notifier_read() handler
>> > function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
>> > The result is both QEMU and vhost_net.ko polling on the same eventfd
>> > and the virtio_net.ko guest driver seeing inconsistent results:
>> >
>> > # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
>> > virtio_net virtio0: output:id 0 is not a head!
>> >
>> > To fix this, proceed the same as we do for irqfd: add a parameter to
>> > virtio_queue_set_host_notifier_fd_handler and in that case only set
>> > the notifier, not the handler.
> Stable-1.1 material? The mentioned commit is included into 1.1 release.
Are you sure?
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
2012-08-18 18:56 ` Paolo Bonzini
@ 2012-08-18 18:58 ` Michael Tokarev
0 siblings, 0 replies; 10+ messages in thread
From: Michael Tokarev @ 2012-08-18 18:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: avi, qemu-devel, Stefan Hajnoczi
On 18.08.2012 22:56, Paolo Bonzini wrote:
> Il 12/08/2012 12:08, Michael Tokarev ha scritto:
>>>> Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
>>>> because it always registers the virtio_pci_host_notifier_read() handler
>>>> function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
>>>> The result is both QEMU and vhost_net.ko polling on the same eventfd
>>>> and the virtio_net.ko guest driver seeing inconsistent results:
>>>>
>>>> # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
>>>> virtio_net virtio0: output:id 0 is not a head!
>>>>
>>>> To fix this, proceed the same as we do for irqfd: add a parameter to
>>>> virtio_queue_set_host_notifier_fd_handler and in that case only set
>>>> the notifier, not the handler.
>> Stable-1.1 material? The mentioned commit is included into 1.1 release.
>
> Are you sure?
Oh. Indeed. It is 625 commits after 1.1.0 :) Sorry for the noize.
/mjt
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-08-18 18:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-06 14:56 [Qemu-devel] [PATCH] virtio: fix vhost handling Paolo Bonzini
2012-08-06 15:47 ` Stefan Hajnoczi
2012-08-12 8:58 ` Michael S. Tsirkin
2012-08-12 10:08 ` Michael Tokarev
2012-08-18 18:56 ` Paolo Bonzini
2012-08-18 18:58 ` Michael Tokarev
-- strict thread matches above, loose matches on Subject: below --
2012-08-06 13:26 Paolo Bonzini
2012-08-06 13:39 ` Stefan Hajnoczi
2012-08-06 13:51 ` Paolo Bonzini
2012-08-06 20:37 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).