* [Qemu-devel] [PATCH v2 01/23] misc: indentation
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 14:19 ` Eric Blake
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 02/23] vhost-user: minor simplification marcandre.lureau
` (21 subsequent siblings)
22 siblings, 1 reply; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/net/vhost_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 50f4dcd..3f1fecc 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -313,7 +313,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
* properly.
*/
if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
- dev->use_guest_notifier_mask = false;
+ dev->use_guest_notifier_mask = false;
}
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/23] misc: indentation
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 01/23] misc: indentation marcandre.lureau
@ 2016-06-24 14:19 ` Eric Blake
0 siblings, 0 replies; 28+ messages in thread
From: Eric Blake @ 2016-06-24 14:19 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: yuanhan.liu, victork, mst, jonshin, mukawa
[-- Attachment #1: Type: text/plain, Size: 932 bytes --]
On 06/24/2016 07:50 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/net/vhost_net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 50f4dcd..3f1fecc 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -313,7 +313,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
> * properly.
> */
> if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
> - dev->use_guest_notifier_mask = false;
> + dev->use_guest_notifier_mask = false;
> }
> }
>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 02/23] vhost-user: minor simplification
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 01/23] misc: indentation marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 03/23] vhost: don't assume opaque is a fd, use backend cleanup marcandre.lureau
` (20 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
net/vhost-user.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/vhost-user.c b/net/vhost-user.c
index d72ce9b..392f982 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -310,7 +310,6 @@ static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
{
const char *name = opaque;
const char *driver, *netdev;
- const char virtio_name[] = "virtio-net-";
driver = qemu_opt_get(opts, "driver");
netdev = qemu_opt_get(opts, "netdev");
@@ -320,7 +319,7 @@ static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
}
if (strcmp(netdev, name) == 0 &&
- strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
+ !g_str_has_prefix(driver, "virtio-net-")) {
error_setg(errp, "vhost-user requires frontend driver virtio-net-*");
return -1;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 03/23] vhost: don't assume opaque is a fd, use backend cleanup
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 01/23] misc: indentation marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 02/23] vhost-user: minor simplification marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 04/23] vhost: make vhost_log_put() idempotent marcandre.lureau
` (19 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 81cc5b0..dfcd256 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -997,21 +997,19 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
hdev->migration_blocker = NULL;
- if (vhost_set_backend_type(hdev, backend_type) < 0) {
- close((uintptr_t)opaque);
- return -1;
- }
+ r = vhost_set_backend_type(hdev, backend_type);
+ assert(r >= 0);
- if (hdev->vhost_ops->vhost_backend_init(hdev, opaque) < 0) {
- close((uintptr_t)opaque);
- return -errno;
+ r = hdev->vhost_ops->vhost_backend_init(hdev, opaque);
+ if (r < 0) {
+ goto fail;
}
if (used_memslots > hdev->vhost_ops->vhost_backend_memslots_limit(hdev)) {
fprintf(stderr, "vhost backend memory slots limit is less"
" than current number of present memory slots\n");
- close((uintptr_t)opaque);
- return -1;
+ r = -1;
+ goto fail;
}
QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 04/23] vhost: make vhost_log_put() idempotent
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (2 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 03/23] vhost: don't assume opaque is a fd, use backend cleanup marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 05/23] vhost: call vhost_log_put() on cleanup marcandre.lureau
` (18 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Clear dev->log* when calling vhost_log_put()
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index dfcd256..82290a3 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -362,6 +362,8 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync)
if (!log) {
return;
}
+ dev->log = NULL;
+ dev->log_size = 0;
--log->refcnt;
if (log->refcnt == 0) {
@@ -710,8 +712,6 @@ static int vhost_migration_log(MemoryListener *listener, int enable)
return r;
}
vhost_log_put(dev, false);
- dev->log = NULL;
- dev->log_size = 0;
} else {
vhost_dev_log_resize(dev, vhost_get_log_size(dev));
r = vhost_dev_set_log(dev, true);
@@ -1289,7 +1289,4 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
vhost_log_put(hdev, true);
hdev->started = false;
- hdev->log = NULL;
- hdev->log_size = 0;
}
-
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 05/23] vhost: call vhost_log_put() on cleanup
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (3 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 04/23] vhost: make vhost_log_put() idempotent marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 06/23] vhost: add vhost device only after all success marcandre.lureau
` (17 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Make sure the log is being released on cleanup and the structure
cleared.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 82290a3..ce930cb 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1096,6 +1096,7 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
g_free(hdev->mem);
g_free(hdev->mem_sections);
hdev->vhost_ops->vhost_backend_cleanup(hdev);
+ vhost_log_put(hdev, false);
QLIST_REMOVE(hdev, entry);
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 06/23] vhost: add vhost device only after all success
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (4 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 05/23] vhost: call vhost_log_put() on cleanup marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 07/23] vhost: make vhost_dev_cleanup() idempotent marcandre.lureau
` (16 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This helps following vhost_dev_cleanup() patch to check if the device
was added before removing it from the list.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ce930cb..9c8c9f8 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1011,7 +1011,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
r = -1;
goto fail;
}
- QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
r = hdev->vhost_ops->vhost_set_owner(hdev);
if (r < 0) {
@@ -1070,6 +1069,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
hdev->started = false;
hdev->memory_changed = false;
memory_listener_register(&hdev->memory_listener, &address_space_memory);
+ QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
return 0;
fail_vq:
while (--i >= 0) {
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 07/23] vhost: make vhost_dev_cleanup() idempotent
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (5 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 06/23] vhost: add vhost device only after all success marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 08/23] vhost-net: always call vhost_dev_cleanup() on failure marcandre.lureau
` (15 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
May be called on multiple code path, so it's easier to make it safe in
the first place.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 9c8c9f8..1fdf109 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1085,19 +1085,27 @@ fail:
void vhost_dev_cleanup(struct vhost_dev *hdev)
{
int i;
+
for (i = 0; i < hdev->nvqs; ++i) {
vhost_virtqueue_cleanup(hdev->vqs + i);
}
- memory_listener_unregister(&hdev->memory_listener);
+ if (hdev->mem) {
+ /* those are only safe after successful init */
+ memory_listener_unregister(&hdev->memory_listener);
+ QLIST_REMOVE(hdev, entry);
+ }
if (hdev->migration_blocker) {
migrate_del_blocker(hdev->migration_blocker);
error_free(hdev->migration_blocker);
}
g_free(hdev->mem);
g_free(hdev->mem_sections);
- hdev->vhost_ops->vhost_backend_cleanup(hdev);
+ if (hdev->vhost_ops) {
+ hdev->vhost_ops->vhost_backend_cleanup(hdev);
+ }
vhost_log_put(hdev, false);
- QLIST_REMOVE(hdev, entry);
+
+ memset(hdev, 0, sizeof(struct vhost_dev));
}
/* Stop processing guest IO notifications in qemu.
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 08/23] vhost-net: always call vhost_dev_cleanup() on failure
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (6 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 07/23] vhost: make vhost_dev_cleanup() idempotent marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 09/23] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init() marcandre.lureau
` (14 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
vhost_dev_init() may need to be cleaned up on failure too. Just call
vhost_dev_cleanup() in all cases. But first zero-alloc the struct to
avoid the garbage.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/net/vhost_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 3f1fecc..1da05d0 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -140,7 +140,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
{
int r;
bool backend_kernel = options->backend_type == VHOST_BACKEND_TYPE_KERNEL;
- struct vhost_net *net = g_malloc(sizeof *net);
+ struct vhost_net *net = g_new0(struct vhost_net, 1);
uint64_t features = 0;
if (!options->net_backend) {
@@ -185,7 +185,6 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
fprintf(stderr, "vhost lacks feature mask %" PRIu64
" for backend\n",
(uint64_t)(~net->dev.features & net->dev.backend_features));
- vhost_dev_cleanup(&net->dev);
goto fail;
}
}
@@ -197,7 +196,6 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
fprintf(stderr, "vhost lacks feature mask %" PRIu64
" for backend\n",
(uint64_t)(~net->dev.features & features));
- vhost_dev_cleanup(&net->dev);
goto fail;
}
}
@@ -205,7 +203,9 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
vhost_net_ack_features(net, features);
return net;
+
fail:
+ vhost_dev_cleanup(&net->dev);
g_free(net);
return NULL;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 09/23] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init()
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (7 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 08/23] vhost-net: always call vhost_dev_cleanup() on failure marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 10/23] vhost: change some assert() for error_report() or silent fail marcandre.lureau
` (13 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
vhost_net_init() calls vhost_dev_init() and in case of failure, calls
vhost_dev_cleanup() directly. However, the structure is already
partially cleaned on error. Calling vhost_dev_cleanup() again will call
vhost_virtqueue_cleanup() on already clean queues, and causing potential
double-close. Instead, adjust dev->nvqs and simplify vhost_dev_init()
code to call vhost_virtqueue_cleanup().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1fdf109..314e0e7 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1025,7 +1025,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
for (i = 0; i < hdev->nvqs; ++i) {
r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i);
if (r < 0) {
- goto fail_vq;
+ hdev->nvqs = i;
+ goto fail;
}
}
hdev->features = features;
@@ -1071,14 +1072,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
memory_listener_register(&hdev->memory_listener, &address_space_memory);
QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
return 0;
-fail_vq:
- while (--i >= 0) {
- vhost_virtqueue_cleanup(hdev->vqs + i);
- }
+
fail:
- r = -errno;
- hdev->vhost_ops->vhost_backend_cleanup(hdev);
- QLIST_REMOVE(hdev, entry);
+ vhost_dev_cleanup(hdev);
return r;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 10/23] vhost: change some assert() for error_report() or silent fail
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (8 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 09/23] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init() marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 11/23] vhost: use error_report() instead of fprintf(stderr, ...) marcandre.lureau
` (12 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Calling a vhost operation may fail, especially with disconnectable
backends. Treat some that look harmless as recoverable errors (print
error, or ignore on error code path).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 314e0e7..832fab9 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -400,7 +400,10 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
/* inform backend of log switching, this must be done before
releasing the current log, to ensure no logging is lost */
r = dev->vhost_ops->vhost_set_log_base(dev, log_base, log);
- assert(r >= 0);
+ if (r < 0) {
+ error_report("Failed to change backend log");
+ }
+
vhost_log_put(dev, true);
dev->log = log;
dev->log_size = size;
@@ -567,7 +570,9 @@ static void vhost_commit(MemoryListener *listener)
if (!dev->log_enabled) {
r = dev->vhost_ops->vhost_set_mem_table(dev, dev->mem);
- assert(r >= 0);
+ if (r < 0) {
+ error_report("Failed to set mem table");
+ }
dev->memory_changed = false;
return;
}
@@ -580,7 +585,9 @@ static void vhost_commit(MemoryListener *listener)
vhost_dev_log_resize(dev, log_size + VHOST_LOG_BUFFER);
}
r = dev->vhost_ops->vhost_set_mem_table(dev, dev->mem);
- assert(r >= 0);
+ if (r < 0) {
+ error_report("Failed to set mem table");
+ }
/* To log less, can only decrease log size after table update. */
if (dev->log_size > log_size + VHOST_LOG_BUFFER) {
vhost_dev_log_resize(dev, log_size);
@@ -649,6 +656,7 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
};
int r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
if (r < 0) {
+ error_report("Failed to set vring addr");
return -errno;
}
return 0;
@@ -662,12 +670,15 @@ static int vhost_dev_set_features(struct vhost_dev *dev, bool enable_log)
features |= 0x1ULL << VHOST_F_LOG_ALL;
}
r = dev->vhost_ops->vhost_set_features(dev, features);
+ if (r < 0) {
+ error_report("Failed to set features");
+ }
return r < 0 ? -errno : 0;
}
static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
{
- int r, t, i, idx;
+ int r, i, idx;
r = vhost_dev_set_features(dev, enable_log);
if (r < 0) {
goto err_features;
@@ -684,12 +695,10 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
err_vq:
for (; i >= 0; --i) {
idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
- t = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
- dev->log_enabled);
- assert(t >= 0);
+ vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
+ dev->log_enabled);
}
- t = vhost_dev_set_features(dev, dev->log_enabled);
- assert(t >= 0);
+ vhost_dev_set_features(dev, dev->log_enabled);
err_features:
return r;
}
@@ -937,7 +946,6 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
}
}
- assert (r >= 0);
cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx),
0, virtio_queue_get_ring_size(vdev, idx));
cpu_physical_memory_unmap(vq->used, virtio_queue_get_used_size(vdev, idx),
@@ -1190,7 +1198,9 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
file.index = hdev->vhost_ops->vhost_get_vq_index(hdev, n);
r = hdev->vhost_ops->vhost_set_vring_call(hdev, &file);
- assert(r >= 0);
+ if (r < 0) {
+ error_report("Failed to set vring call");
+ }
}
uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 11/23] vhost: use error_report() instead of fprintf(stderr, ...)
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (9 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 10/23] vhost: change some assert() for error_report() or silent fail marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 12/23] vhost-user: check qemu_chr_fe_set_msgfds() return value marcandre.lureau
` (11 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Let's use qemu proper error reporting API.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 832fab9..e4eb97e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -427,11 +427,11 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
l = vq->ring_size;
p = cpu_physical_memory_map(vq->ring_phys, &l, 1);
if (!p || l != vq->ring_size) {
- fprintf(stderr, "Unable to map ring buffer for ring %d\n", i);
+ error_report("Unable to map ring buffer for ring %d", i);
r = -ENOMEM;
}
if (p != vq->ring) {
- fprintf(stderr, "Ring buffer relocated for ring %d\n", i);
+ error_report("Ring buffer relocated for ring %d", i);
r = -EBUSY;
}
cpu_physical_memory_unmap(p, l, 0, 0);
@@ -928,8 +928,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
if (r < 0) {
- fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
- fflush(stderr);
+ error_report("vhost VQ %d ring restore failed: %d", idx, r);
}
virtio_queue_set_last_avail_idx(vdev, idx, state.num);
virtio_queue_invalidate_signalled_used(vdev, idx);
@@ -1014,8 +1013,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
}
if (used_memslots > hdev->vhost_ops->vhost_backend_memslots_limit(hdev)) {
- fprintf(stderr, "vhost backend memory slots limit is less"
- " than current number of present memory slots\n");
+ error_report("vhost backend memory slots limit is less"
+ " than current number of present memory slots");
r = -1;
goto fail;
}
@@ -1122,7 +1121,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i, r, e;
if (!k->set_host_notifier) {
- fprintf(stderr, "binding does not support host notifiers\n");
+ error_report("binding does not support host notifiers");
r = -ENOSYS;
goto fail;
}
@@ -1130,7 +1129,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
for (i = 0; i < hdev->nvqs; ++i) {
r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
if (r < 0) {
- fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
+ error_report("vhost VQ %d notifier binding failed: %d", i, -r);
goto fail_vq;
}
}
@@ -1140,8 +1139,7 @@ fail_vq:
while (--i >= 0) {
e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
if (e < 0) {
- fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
- fflush(stderr);
+ error_report("vhost VQ %d notifier cleanup error: %d", i, -r);
}
assert (e >= 0);
}
@@ -1164,8 +1162,7 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
for (i = 0; i < hdev->nvqs; ++i) {
r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
if (r < 0) {
- fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
- fflush(stderr);
+ error_report("vhost VQ %d notifier cleanup failed: %d", i, -r);
}
assert (r >= 0);
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 12/23] vhost-user: check qemu_chr_fe_set_msgfds() return value
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (10 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 11/23] vhost: use error_report() instead of fprintf(stderr, ...) marcandre.lureau
@ 2016-06-24 13:50 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 13/23] vhost-user: check vhost_user_{read, write}() " marcandre.lureau
` (10 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:50 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Check return value, and drop the unnecessary 'if' check for fd_num.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost-user.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 495e09f..5dae496 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -187,8 +187,8 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
return 0;
}
- if (fd_num) {
- qemu_chr_fe_set_msgfds(chr, fds, fd_num);
+ if (qemu_chr_fe_set_msgfds(chr, fds, fd_num) < 0) {
+ return -1;
}
return qemu_chr_fe_write_all(chr, (const uint8_t *) msg, size) == size ?
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 13/23] vhost-user: check vhost_user_{read, write}() return value
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (11 preceding siblings ...)
2016-06-24 13:50 ` [Qemu-devel] [PATCH v2 12/23] vhost-user: check qemu_chr_fe_set_msgfds() return value marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 14/23] qemu-char: check socket is actually connected marcandre.lureau
` (9 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
More error checking to break code flow and report appropriate errors.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost-user.c | 50 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 5dae496..819481d 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -214,12 +214,14 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
fds[fd_num++] = log->fd;
}
- vhost_user_write(dev, &msg, fds, fd_num);
+ if (vhost_user_write(dev, &msg, fds, fd_num) < 0) {
+ return -1;
+ }
if (shmfd) {
msg.size = 0;
if (vhost_user_read(dev, &msg) < 0) {
- return 0;
+ return -1;
}
if (msg.request != VHOST_USER_SET_LOG_BASE) {
@@ -275,7 +277,9 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
msg.size += sizeof(msg.payload.memory.padding);
msg.size += fd_num * sizeof(VhostUserMemoryRegion);
- vhost_user_write(dev, &msg, fds, fd_num);
+ if (vhost_user_write(dev, &msg, fds, fd_num) < 0) {
+ return -1;
+ }
return 0;
}
@@ -290,7 +294,9 @@ static int vhost_user_set_vring_addr(struct vhost_dev *dev,
.size = sizeof(msg.payload.addr),
};
- vhost_user_write(dev, &msg, NULL, 0);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
return 0;
}
@@ -313,7 +319,9 @@ static int vhost_set_vring(struct vhost_dev *dev,
.size = sizeof(msg.payload.state),
};
- vhost_user_write(dev, &msg, NULL, 0);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
return 0;
}
@@ -360,10 +368,12 @@ static int vhost_user_get_vring_base(struct vhost_dev *dev,
.size = sizeof(msg.payload.state),
};
- vhost_user_write(dev, &msg, NULL, 0);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
if (vhost_user_read(dev, &msg) < 0) {
- return 0;
+ return -1;
}
if (msg.request != VHOST_USER_GET_VRING_BASE) {
@@ -401,7 +411,9 @@ static int vhost_set_vring_file(struct vhost_dev *dev,
msg.payload.u64 |= VHOST_USER_VRING_NOFD_MASK;
}
- vhost_user_write(dev, &msg, fds, fd_num);
+ if (vhost_user_write(dev, &msg, fds, fd_num) < 0) {
+ return -1;
+ }
return 0;
}
@@ -427,7 +439,9 @@ static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t u64)
.size = sizeof(msg.payload.u64),
};
- vhost_user_write(dev, &msg, NULL, 0);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
return 0;
}
@@ -455,10 +469,12 @@ static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
return 0;
}
- vhost_user_write(dev, &msg, NULL, 0);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
if (vhost_user_read(dev, &msg) < 0) {
- return 0;
+ return -1;
}
if (msg.request != request) {
@@ -489,7 +505,9 @@ static int vhost_user_set_owner(struct vhost_dev *dev)
.flags = VHOST_USER_VERSION,
};
- vhost_user_write(dev, &msg, NULL, 0);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
return 0;
}
@@ -501,7 +519,9 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
.flags = VHOST_USER_VERSION,
};
- vhost_user_write(dev, &msg, NULL, 0);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
return 0;
}
@@ -588,7 +608,6 @@ static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
{
VhostUserMsg msg = { 0 };
- int err;
assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
@@ -605,8 +624,7 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
memcpy((char *)&msg.payload.u64, mac_addr, 6);
msg.size = sizeof(msg.payload.u64);
- err = vhost_user_write(dev, &msg, NULL, 0);
- return err;
+ return vhost_user_write(dev, &msg, NULL, 0);
}
return -1;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 14/23] qemu-char: check socket is actually connected
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (12 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 13/23] vhost-user: check vhost_user_{read, write}() " marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 15/23] vhost-user: keep vhost_net after a disconnection marcandre.lureau
` (8 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Calling qemu_chr_fe_set_msgfds() on unconnected socket can lead to crash
if s->ioc is NULL.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qemu-char.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 84f49ac..06eaba3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2761,14 +2761,16 @@ static int tcp_set_msgfds(CharDriverState *chr, int *fds, int num)
{
TCPCharDriver *s = chr->opaque;
- if (!qio_channel_has_feature(s->ioc,
- QIO_CHANNEL_FEATURE_FD_PASS)) {
- return -1;
- }
/* clear old pending fd array */
g_free(s->write_msgfds);
s->write_msgfds = NULL;
+ if (!s->connected ||
+ !qio_channel_has_feature(s->ioc,
+ QIO_CHANNEL_FEATURE_FD_PASS)) {
+ return -1;
+ }
+
if (num) {
s->write_msgfds = g_new(int, num);
memcpy(s->write_msgfds, fds, num * sizeof(int));
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 15/23] vhost-user: keep vhost_net after a disconnection
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (13 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 14/23] qemu-char: check socket is actually connected marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 16/23] Revert "vhost-net: do not crash if backend is not present" marcandre.lureau
` (7 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Lot of code path assumes get_vhost_net() return non-null. Keep it
after a successful vhost_net_init(). vhost_net_cleanup() will clear the
vhost-dev connection-related data after vhost_user_stop().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/net/vhost_net.c | 1 -
net/tap.c | 1 +
net/vhost-user.c | 21 ++++++++-------------
3 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 1da05d0..08c7d1e 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -378,7 +378,6 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
void vhost_net_cleanup(struct vhost_net *net)
{
vhost_dev_cleanup(&net->dev);
- g_free(net);
}
int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
diff --git a/net/tap.c b/net/tap.c
index 49817c7..31726e7 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -299,6 +299,7 @@ static void tap_cleanup(NetClientState *nc)
if (s->vhost_net) {
vhost_net_cleanup(s->vhost_net);
+ g_free(s->vhost_net);
s->vhost_net = NULL;
}
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 392f982..4c7702f 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -45,11 +45,6 @@ uint64_t vhost_user_get_acked_features(NetClientState *nc)
return s->acked_features;
}
-static int vhost_user_running(VhostUserState *s)
-{
- return (s->vhost_net) ? 1 : 0;
-}
-
static void vhost_user_stop(int queues, NetClientState *ncs[])
{
VhostUserState *s;
@@ -59,15 +54,11 @@ static void vhost_user_stop(int queues, NetClientState *ncs[])
assert (ncs[i]->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
s = DO_UPCAST(VhostUserState, nc, ncs[i]);
- if (!vhost_user_running(s)) {
- continue;
- }
if (s->vhost_net) {
/* save acked features */
- s->acked_features = vhost_net_get_acked_features(s->vhost_net);
+ s->acked_features |= vhost_net_get_acked_features(s->vhost_net);
vhost_net_cleanup(s->vhost_net);
- s->vhost_net = NULL;
}
}
}
@@ -85,12 +76,15 @@ static int vhost_user_start(int queues, NetClientState *ncs[])
assert (ncs[i]->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
s = DO_UPCAST(VhostUserState, nc, ncs[i]);
- if (vhost_user_running(s)) {
- continue;
- }
options.net_backend = ncs[i];
options.opaque = s->chr;
+
+ if (s->vhost_net) {
+ vhost_net_cleanup(s->vhost_net);
+ g_free(s->vhost_net);
+ }
+
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
error_report("failed to init vhost_net for queue %d", i);
@@ -149,6 +143,7 @@ static void vhost_user_cleanup(NetClientState *nc)
if (s->vhost_net) {
vhost_net_cleanup(s->vhost_net);
+ g_free(s->vhost_net);
s->vhost_net = NULL;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 16/23] Revert "vhost-net: do not crash if backend is not present"
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (14 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 15/23] vhost-user: keep vhost_net after a disconnection marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 17/23] get_vhost_net() should be != null after vhost_user_init marcandre.lureau
` (6 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Now that get_vhost_net() returns non-null after a successful
vhost_net_init(), we no longer need to check this case.
This reverts commit ecd34898596c60f79886061618dd7e01001113ad.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/net/vhost_net.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 08c7d1e..f1dd367 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -428,15 +428,10 @@ VHostNetState *get_vhost_net(NetClientState *nc)
int vhost_set_vring_enable(NetClientState *nc, int enable)
{
VHostNetState *net = get_vhost_net(nc);
- const VhostOps *vhost_ops;
+ const VhostOps *vhost_ops = net->dev.vhost_ops;
nc->vring_enable = enable;
- if (!net) {
- return 0;
- }
-
- vhost_ops = net->dev.vhost_ops;
if (vhost_ops->vhost_set_vring_enable) {
return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 17/23] get_vhost_net() should be != null after vhost_user_init
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (15 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 16/23] Revert "vhost-net: do not crash if backend is not present" marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 18/23] vhost-net: success if backend has no ops->vhost_migration_done marcandre.lureau
` (5 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/net/vhost_net.c | 1 +
net/vhost-user.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f1dd367..22ea653 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -417,6 +417,7 @@ VHostNetState *get_vhost_net(NetClientState *nc)
break;
case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
vhost_net = vhost_user_get_vhost_net(nc);
+ assert(vhost_net != NULL);
break;
default:
break;
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 4c7702f..95ed2d2 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -250,6 +250,8 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
qemu_chr_add_handlers(chr, NULL, NULL, net_vhost_user_event, nc[0].name);
+ assert(s->vhost_net != NULL);
+
return 0;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 18/23] vhost-net: success if backend has no ops->vhost_migration_done
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (16 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 17/23] get_vhost_net() should be != null after vhost_user_init marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 19/23] vhost: add assert() to check runtime behaviour marcandre.lureau
` (4 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/net/vhost_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 22ea653..cc2f68a 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -383,7 +383,7 @@ void vhost_net_cleanup(struct vhost_net *net)
int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
{
const VhostOps *vhost_ops = net->dev.vhost_ops;
- int r = -1;
+ int r = 0;
if (vhost_ops->vhost_migration_done) {
r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 19/23] vhost: add assert() to check runtime behaviour
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (17 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 18/23] vhost-net: success if backend has no ops->vhost_migration_done marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 20/23] char: add chr_wait_connected callback marcandre.lureau
` (3 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
All these functions must be called only after the backend is connected.
They are called from virtio-net.c, after either virtio or link status
change.
The check for nc->peer->link_down should ensure vhost_net_{start,stop}()
are always called between vhost_user_{start,stop}().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/virtio/vhost.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e4eb97e..e34bd39 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1186,6 +1186,9 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
int r, index = n - hdev->vq_index;
struct vhost_vring_file file;
+ /* should only be called after backend is connected */
+ assert(hdev->vhost_ops != NULL);
+
if (mask) {
assert(vdev->use_guest_notifier_mask);
file.fd = event_notifier_get_fd(&hdev->vqs[index].masked_notifier);
@@ -1232,6 +1235,9 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
{
int i, r;
+ /* should only be called after backend is connected */
+ assert(hdev->vhost_ops != NULL);
+
hdev->started = true;
r = vhost_dev_set_features(hdev, hdev->log_enabled);
@@ -1292,6 +1298,9 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
{
int i;
+ /* should only be called after backend is connected */
+ assert(hdev->vhost_ops != NULL);
+
for (i = 0; i < hdev->nvqs; ++i) {
vhost_virtqueue_stop(hdev,
vdev,
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 20/23] char: add chr_wait_connected callback
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (18 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 19/23] vhost: add assert() to check runtime behaviour marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 21/23] char: add and use tcp_chr_wait_connected marcandre.lureau
` (2 subsequent siblings)
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
A function to wait on the backend to be connected, to be used in the
following patches.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/sysemu/char.h | 8 ++++++++
qemu-char.c | 9 +++++++++
2 files changed, 17 insertions(+)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 1eb2d0f..0e3c35f 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -70,6 +70,7 @@ struct CharDriverState {
int (*get_msgfds)(struct CharDriverState *s, int* fds, int num);
int (*set_msgfds)(struct CharDriverState *s, int *fds, int num);
int (*chr_add_client)(struct CharDriverState *chr, int fd);
+ int (*chr_wait_connected)(struct CharDriverState *chr, Error **errp);
IOEventHandler *chr_event;
IOCanReadHandler *chr_can_read;
IOReadHandler *chr_read;
@@ -151,6 +152,13 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename,
*/
void qemu_chr_disconnect(CharDriverState *chr);
+/**
+ * @qemu_chr_wait_connected:
+ *
+ * Wait for characted backend to be connected.
+ */
+int qemu_chr_wait_connected(CharDriverState *chr, Error **errp);
+
/**
* @qemu_chr_new_noreplay:
*
diff --git a/qemu-char.c b/qemu-char.c
index 06eaba3..a0e7736 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3140,6 +3140,15 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
return TRUE;
}
+int qemu_chr_wait_connected(CharDriverState *chr, Error **errp)
+{
+ if (chr->chr_wait_connected) {
+ return chr->chr_wait_connected(chr, errp);
+ }
+
+ return 0;
+}
+
static void tcp_chr_close(CharDriverState *chr)
{
TCPCharDriver *s = chr->opaque;
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 21/23] char: add and use tcp_chr_wait_connected
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (19 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 20/23] char: add chr_wait_connected callback marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 22/23] vhost-user: wait until backend init is completed marcandre.lureau
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 23/23] tests: add /vhost-user/connect-fail test marcandre.lureau
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add a chr_wait_connected for the tcp backend, and use it in the
open_socket() function.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qemu-char.c | 63 ++++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 19 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index a0e7736..dca859d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3140,6 +3140,32 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
return TRUE;
}
+static int tcp_chr_wait_connected(CharDriverState *chr, Error **errp)
+{
+ TCPCharDriver *s = chr->opaque;
+ QIOChannelSocket *sioc;
+
+ while (!s->connected) {
+ if (s->is_listen) {
+ fprintf(stderr, "QEMU waiting for connection on: %s\n",
+ chr->filename);
+ qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL);
+ tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr);
+ qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), false, NULL);
+ } else {
+ sioc = qio_channel_socket_new();
+ if (qio_channel_socket_connect_sync(sioc, s->addr, errp) < 0) {
+ object_unref(OBJECT(sioc));
+ return -1;
+ }
+ tcp_chr_new_client(chr, sioc);
+ object_unref(OBJECT(sioc));
+ }
+ }
+
+ return 0;
+}
+
int qemu_chr_wait_connected(CharDriverState *chr, Error **errp)
{
if (chr->chr_wait_connected) {
@@ -4403,6 +4429,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
qapi_copy_SocketAddress(&s->addr, sock->addr);
chr->opaque = s;
+ chr->chr_wait_connected = tcp_chr_wait_connected;
chr->chr_write = tcp_chr_write;
chr->chr_sync_read = tcp_chr_sync_read;
chr->chr_close = tcp_chr_close;
@@ -4426,32 +4453,30 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
s->reconnect_time = reconnect;
}
- sioc = qio_channel_socket_new();
if (s->reconnect_time) {
+ sioc = qio_channel_socket_new();
qio_channel_socket_connect_async(sioc, s->addr,
qemu_chr_socket_connected,
chr, NULL);
- } else if (s->is_listen) {
- if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) {
- goto error;
- }
- s->listen_ioc = sioc;
- if (is_waitconnect) {
- fprintf(stderr, "QEMU waiting for connection on: %s\n",
- chr->filename);
- tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr);
- }
- qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), false, NULL);
- if (!s->ioc) {
- s->listen_tag = qio_channel_add_watch(
- QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
- }
} else {
- if (qio_channel_socket_connect_sync(sioc, s->addr, errp) < 0) {
+ if (s->is_listen) {
+ sioc = qio_channel_socket_new();
+ if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) {
+ goto error;
+ }
+ s->listen_ioc = sioc;
+ if (is_waitconnect &&
+ qemu_chr_wait_connected(chr, errp) < 0) {
+ goto error;
+ }
+ if (!s->ioc) {
+ s->listen_tag = qio_channel_add_watch(
+ QIO_CHANNEL(s->listen_ioc), G_IO_IN,
+ tcp_chr_accept, chr, NULL);
+ }
+ } else if (qemu_chr_wait_connected(chr, errp) < 0) {
goto error;
}
- tcp_chr_new_client(chr, sioc);
- object_unref(OBJECT(sioc));
}
return chr;
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 22/23] vhost-user: wait until backend init is completed
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (20 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 21/23] char: add and use tcp_chr_wait_connected marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
2016-06-30 6:38 ` Yuanhan Liu
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 23/23] tests: add /vhost-user/connect-fail test marcandre.lureau
22 siblings, 1 reply; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The chardev waits for an initial connection before starting qemu,
vhost-user wants the backend negotiation to be completed. vhost-user is
started in the net_vhost_user_event callback, which is synchronously
called after the socket is connected.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
net/vhost-user.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 95ed2d2..4badd9e 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -24,6 +24,7 @@ typedef struct VhostUserState {
VHostNetState *vhost_net;
int watch;
uint64_t acked_features;
+ bool started;
} VhostUserState;
typedef struct VhostUserChardevProps {
@@ -211,6 +212,7 @@ static void net_vhost_user_event(void *opaque, int event)
return;
}
qmp_set_link(name, true, &err);
+ s->started = true;
break;
case CHR_EVENT_CLOSED:
qmp_set_link(name, false, &err);
@@ -248,7 +250,15 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
s->chr = chr;
}
- qemu_chr_add_handlers(chr, NULL, NULL, net_vhost_user_event, nc[0].name);
+ do {
+ Error *err = NULL;
+ if (qemu_chr_wait_connected(chr, &err) < 0) {
+ error_report_err(err);
+ return -1;
+ }
+ qemu_chr_add_handlers(chr, NULL, NULL,
+ net_vhost_user_event, nc[0].name);
+ } while (!s->started);
assert(s->vhost_net != NULL);
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH v2 22/23] vhost-user: wait until backend init is completed
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 22/23] vhost-user: wait until backend init is completed marcandre.lureau
@ 2016-06-30 6:38 ` Yuanhan Liu
2016-06-30 9:22 ` Marc-André Lureau
0 siblings, 1 reply; 28+ messages in thread
From: Yuanhan Liu @ 2016-06-30 6:38 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, mukawa, victork, jonshin, mst
On Fri, Jun 24, 2016 at 03:51:09PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The chardev waits for an initial connection before starting qemu,
> vhost-user wants the backend negotiation to be completed. vhost-user is
> started in the net_vhost_user_event callback, which is synchronously
> called after the socket is connected.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> net/vhost-user.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 95ed2d2..4badd9e 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -24,6 +24,7 @@ typedef struct VhostUserState {
> VHostNetState *vhost_net;
> int watch;
> uint64_t acked_features;
> + bool started;
> } VhostUserState;
>
> typedef struct VhostUserChardevProps {
> @@ -211,6 +212,7 @@ static void net_vhost_user_event(void *opaque, int event)
> return;
> }
> qmp_set_link(name, true, &err);
> + s->started = true;
> break;
> case CHR_EVENT_CLOSED:
> qmp_set_link(name, false, &err);
> @@ -248,7 +250,15 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
> s->chr = chr;
> }
>
> - qemu_chr_add_handlers(chr, NULL, NULL, net_vhost_user_event, nc[0].name);
> + do {
> + Error *err = NULL;
> + if (qemu_chr_wait_connected(chr, &err) < 0) {
> + error_report_err(err);
> + return -1;
> + }
> + qemu_chr_add_handlers(chr, NULL, NULL,
> + net_vhost_user_event, nc[0].name);
> + } while (!s->started);
I haven't looked at your patchset carefully yet, but I did a quick test,
and showed that above code piece just breaks vhost-user: it's a dead loop
that vhost-user net will be initiated again and again.
The dead loop is due to, we check "s->started" corresponding to last nc,
while we set "s->started" corresponding to the first nc.
--yliu
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH v2 22/23] vhost-user: wait until backend init is completed
2016-06-30 6:38 ` Yuanhan Liu
@ 2016-06-30 9:22 ` Marc-André Lureau
2016-06-30 13:02 ` Yuanhan Liu
0 siblings, 1 reply; 28+ messages in thread
From: Marc-André Lureau @ 2016-06-30 9:22 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: marcandre lureau, qemu-devel, mukawa, victork, jonshin, mst
Hi
----- Original Message -----
> On Fri, Jun 24, 2016 at 03:51:09PM +0200, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The chardev waits for an initial connection before starting qemu,
> > vhost-user wants the backend negotiation to be completed. vhost-user is
> > started in the net_vhost_user_event callback, which is synchronously
> > called after the socket is connected.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > net/vhost-user.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 95ed2d2..4badd9e 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -24,6 +24,7 @@ typedef struct VhostUserState {
> > VHostNetState *vhost_net;
> > int watch;
> > uint64_t acked_features;
> > + bool started;
> > } VhostUserState;
> >
> > typedef struct VhostUserChardevProps {
> > @@ -211,6 +212,7 @@ static void net_vhost_user_event(void *opaque, int
> > event)
> > return;
> > }
> > qmp_set_link(name, true, &err);
> > + s->started = true;
> > break;
> > case CHR_EVENT_CLOSED:
> > qmp_set_link(name, false, &err);
> > @@ -248,7 +250,15 @@ static int net_vhost_user_init(NetClientState *peer,
> > const char *device,
> > s->chr = chr;
> > }
> >
> > - qemu_chr_add_handlers(chr, NULL, NULL, net_vhost_user_event,
> > nc[0].name);
> > + do {
> > + Error *err = NULL;
> > + if (qemu_chr_wait_connected(chr, &err) < 0) {
> > + error_report_err(err);
> > + return -1;
> > + }
> > + qemu_chr_add_handlers(chr, NULL, NULL,
> > + net_vhost_user_event, nc[0].name);
> > + } while (!s->started);
>
>
> I haven't looked at your patchset carefully yet, but I did a quick test,
> and showed that above code piece just breaks vhost-user: it's a dead loop
> that vhost-user net will be initiated again and again.
Interesting, thanks a lot for testing!
The vhost-user-test works just fine, as well as vhost-user-bridge.
> The dead loop is due to, we check "s->started" corresponding to last nc,
> while we set "s->started" corresponding to the first nc.
I see, that makes sense with multiqueue, I'll update the code. It would be really nice to have a basic multiqueue test in vhost-user-test.
thanks
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH v2 22/23] vhost-user: wait until backend init is completed
2016-06-30 9:22 ` Marc-André Lureau
@ 2016-06-30 13:02 ` Yuanhan Liu
0 siblings, 0 replies; 28+ messages in thread
From: Yuanhan Liu @ 2016-06-30 13:02 UTC (permalink / raw)
To: Marc-André Lureau
Cc: marcandre lureau, qemu-devel, mukawa, victork, jonshin, mst
On Thu, Jun 30, 2016 at 05:22:43AM -0400, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
> > On Fri, Jun 24, 2016 at 03:51:09PM +0200, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > The chardev waits for an initial connection before starting qemu,
> > > vhost-user wants the backend negotiation to be completed. vhost-user is
> > > started in the net_vhost_user_event callback, which is synchronously
> > > called after the socket is connected.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > net/vhost-user.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > > index 95ed2d2..4badd9e 100644
> > > --- a/net/vhost-user.c
> > > +++ b/net/vhost-user.c
> > > @@ -24,6 +24,7 @@ typedef struct VhostUserState {
> > > VHostNetState *vhost_net;
> > > int watch;
> > > uint64_t acked_features;
> > > + bool started;
> > > } VhostUserState;
> > >
> > > typedef struct VhostUserChardevProps {
> > > @@ -211,6 +212,7 @@ static void net_vhost_user_event(void *opaque, int
> > > event)
> > > return;
> > > }
> > > qmp_set_link(name, true, &err);
> > > + s->started = true;
> > > break;
> > > case CHR_EVENT_CLOSED:
> > > qmp_set_link(name, false, &err);
> > > @@ -248,7 +250,15 @@ static int net_vhost_user_init(NetClientState *peer,
> > > const char *device,
> > > s->chr = chr;
> > > }
> > >
> > > - qemu_chr_add_handlers(chr, NULL, NULL, net_vhost_user_event,
> > > nc[0].name);
> > > + do {
> > > + Error *err = NULL;
> > > + if (qemu_chr_wait_connected(chr, &err) < 0) {
> > > + error_report_err(err);
> > > + return -1;
> > > + }
> > > + qemu_chr_add_handlers(chr, NULL, NULL,
> > > + net_vhost_user_event, nc[0].name);
> > > + } while (!s->started);
> >
> >
> > I haven't looked at your patchset carefully yet, but I did a quick test,
> > and showed that above code piece just breaks vhost-user: it's a dead loop
> > that vhost-user net will be initiated again and again.
>
> Interesting, thanks a lot for testing!
>
> The vhost-user-test works just fine, as well as vhost-user-bridge.
>
> > The dead loop is due to, we check "s->started" corresponding to last nc,
> > while we set "s->started" corresponding to the first nc.
>
> I see, that makes sense with multiqueue, I'll update the code. It would be really nice to have a basic multiqueue test in vhost-user-test.
My bad. It was actually my task to add the multiqueue test: it was
postponed, or even forgotten :(. If you don't mind, feel free to
add it. If you have not time for that, I will spend some time to
fix what I left.
--yliu
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v2 23/23] tests: add /vhost-user/connect-fail test
2016-06-24 13:50 [Qemu-devel] [PATCH v2 00/23] vhost-user reconnect fixes marcandre.lureau
` (21 preceding siblings ...)
2016-06-24 13:51 ` [Qemu-devel] [PATCH v2 22/23] vhost-user: wait until backend init is completed marcandre.lureau
@ 2016-06-24 13:51 ` marcandre.lureau
22 siblings, 0 replies; 28+ messages in thread
From: marcandre.lureau @ 2016-06-24 13:51 UTC (permalink / raw)
To: qemu-devel
Cc: mukawa, yuanhan.liu, victork, jonshin, mst, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Check early connection failure and resume.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/vhost-user-test.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 8b2164b..7fe12e6 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -131,6 +131,7 @@ typedef struct TestServer {
GCond data_cond;
int log_fd;
uint64_t rings;
+ bool test_fail;
} TestServer;
#if !GLIB_CHECK_VERSION(2, 32, 0)
@@ -234,6 +235,12 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
uint8_t *p = (uint8_t *) &msg;
int fd;
+ if (s->test_fail) {
+ qemu_chr_disconnect(chr);
+ /* now switch to non-failure */
+ s->test_fail = false;
+ }
+
if (size != VHOST_USER_HDR_SIZE) {
g_test_message("Wrong message size received %d\n", size);
return;
@@ -697,6 +704,33 @@ static void test_reconnect(void)
g_test_trap_subprocess(path, 0, 0);
g_test_trap_assert_passed();
}
+
+static void test_connect_fail_subprocess(void)
+{
+ TestServer *s = test_server_new("reconnect");
+ char *cmd;
+
+ s->test_fail = true;
+ g_thread_new("connect", connect_thread, s);
+ cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+ qtest_start(cmd);
+ g_free(cmd);
+
+ wait_for_fds(s);
+ wait_for_rings_started(s, 2);
+
+ qtest_end();
+ test_server_free(s);
+}
+
+static void test_connect_fail(void)
+{
+ gchar *path = g_strdup_printf("/%s/vhost-user/connect-fail/subprocess",
+ qtest_get_arch());
+ g_test_trap_subprocess(path, 0, 0);
+ g_test_trap_assert_passed();
+}
+
#endif
int main(int argc, char **argv)
@@ -747,6 +781,9 @@ int main(int argc, char **argv)
qtest_add_func("/vhost-user/reconnect/subprocess",
test_reconnect_subprocess);
qtest_add_func("/vhost-user/reconnect", test_reconnect);
+ qtest_add_func("/vhost-user/connect-fail/subprocess",
+ test_connect_fail_subprocess);
+ qtest_add_func("/vhost-user/connect-fail", test_connect_fail);
#endif
ret = g_test_run();
--
2.9.0
^ permalink raw reply related [flat|nested] 28+ messages in thread