[Qemu-devel] [PATCH 00/10] vhost-user: simple reconnection support

[Qemu-devel] [PATCH 00/10] vhost-user: simple reconnection support

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

In a previous series "RFCv2: vhost-user: shutdown and reconnection", I
proposed to add a new slave request to handle graceful shutdown, for
both qemu configuration, server or client, while keeping the guest
running with link down status.

However, for the simple case where qemu is configured as server, and
the backend processes packets in order and disconnects, it is possible
for the backend to recover after reconnection by discarding the qemu
SET_VRING_BASE value and resuming from the used->index instead. This
simplifies the reconnection support in this particular situation (it
would make sense to have the backend declare this behaviour with an
extra flag)

The guest won't be notified of link status change and queues may not
be processed in a timely manner, also qemu may assert if some
vhost-user commands are happening while the backend is disconnected.
So the reconnection must happen "quickly enough" here. In order to
keep the series relatively small, these further problems will be
addressed later.

These series demonstrates a simple reconnect support for vubr
(configured as client and qemu as server), includes some nice to
have fixes and a simple test.

rfcv3->v1:
- rebased
- added add a dummy vhost_net_get_acked_features()
  implementation for !VHOST_NET
- changed vubr->tests/vhost-user-bridge in summary
- made the test silent by using a subprocess
- added s-o-b/tested-by/r-b tags

Marc-André Lureau (8):
  tests/vhost-user-bridge: add client mode
  tests/vhost-user-bridge: workaround stale vring base
  vhost-user: disconnect on start failure
  vhost-net: do not crash if backend is not present
  vhost-net: save & restore vhost-user acked features
  vhost-net: save & restore vring enable state
  tests: append i386 tests
  test: start vhost-user reconnect test

Tetsuya Mukawa (2):
  vhost-user: add ability to know vhost-user backend disconnection
  qemu-char: add qemu_chr_disconnect to close a fd accepted by listen fd

 hw/net/vhost_net.c        |  45 ++++++++++++++-
 include/net/net.h         |   1 +
 include/net/vhost-user.h  |   1 +
 include/net/vhost_net.h   |   3 +
 include/sysemu/char.h     |   7 +++
 net/vhost-user.c          |  32 ++++++++++-
 qemu-char.c               |   8 +++
 tests/Makefile            |   2 +-
 tests/vhost-user-bridge.c |  45 +++++++++++----
 tests/vhost-user-test.c   | 136 ++++++++++++++++++++++++++++++++++++++++------
 10 files changed, 247 insertions(+), 33 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 01/10] vhost-user: add ability to know vhost-user backend disconnection

[marcandre.lureau]

From: Tetsuya Mukawa <mukawa@igel.co.jp>

Current QEMU cannot detect vhost-user backend disconnection. The
patch adds ability to know it.
To know disconnection, add watcher to detect G_IO_HUP event. When
G_IO_HUP event is detected, the disconnected socket will be read
to cause a CHR_EVENT_CLOSED.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 net/vhost-user.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 1b9e73a..4a7fd5f 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -22,6 +22,7 @@ typedef struct VhostUserState {
     NetClientState nc;
     CharDriverState *chr;
     VHostNetState *vhost_net;
+    int watch;
 } VhostUserState;
 
 typedef struct VhostUserChardevProps {
@@ -167,6 +168,20 @@ static NetClientInfo net_vhost_user_info = {
         .has_ufo = vhost_user_has_ufo,
 };
 
+static gboolean net_vhost_user_watch(GIOChannel *chan, GIOCondition cond,
+                                           void *opaque)
+{
+    VhostUserState *s = opaque;
+    uint8_t buf[1];
+
+    /* We don't actually want to read anything, but CHR_EVENT_CLOSED will be
+     * raised as a side-effect of the read.
+     */
+    qemu_chr_fe_read_all(s->chr, buf, sizeof(buf));
+
+    return FALSE;
+}
+
 static void net_vhost_user_event(void *opaque, int event)
 {
     const char *name = opaque;
@@ -184,6 +199,8 @@ static void net_vhost_user_event(void *opaque, int event)
     trace_vhost_user_event(s->chr->label, event);
     switch (event) {
     case CHR_EVENT_OPENED:
+        s->watch = qemu_chr_fe_add_watch(s->chr, G_IO_HUP,
+                                         net_vhost_user_watch, s);
         if (vhost_user_start(queues, ncs) < 0) {
             exit(1);
         }
@@ -192,6 +209,8 @@ static void net_vhost_user_event(void *opaque, int event)
     case CHR_EVENT_CLOSED:
         qmp_set_link(name, false, &err);
         vhost_user_stop(queues, ncs);
+        g_source_remove(s->watch);
+        s->watch = 0;
         break;
     }
 
-- 
2.7.4

[Qemu-devel] [PATCH 02/10] tests/vhost-user-bridge: add client mode

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

If -c is specified, vubr will try to connect to the socket instead of
listening for connections.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 tests/vhost-user-bridge.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index 0779ba2..f907ce7 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -1204,12 +1204,13 @@ vubr_accept_cb(int sock, void *ctx)
 }
 
 static VubrDev *
-vubr_new(const char *path)
+vubr_new(const char *path, bool client)
 {
     VubrDev *dev = (VubrDev *) calloc(1, sizeof(VubrDev));
     dev->nregions = 0;
     int i;
     struct sockaddr_un un;
+    CallbackFunc cb;
     size_t len;
 
     for (i = 0; i < MAX_NR_VIRTQUEUE; i++) {
@@ -1238,19 +1239,27 @@ vubr_new(const char *path)
     un.sun_family = AF_UNIX;
     strcpy(un.sun_path, path);
     len = sizeof(un.sun_family) + strlen(path);
-    unlink(path);
 
-    if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
-        vubr_die("bind");
-    }
+    if (!client) {
+        unlink(path);
+
+        if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
+            vubr_die("bind");
+        }
 
-    if (listen(dev->sock, 1) == -1) {
-        vubr_die("listen");
+        if (listen(dev->sock, 1) == -1) {
+            vubr_die("listen");
+        }
+        cb = vubr_accept_cb;
+    } else {
+        if (connect(dev->sock, (struct sockaddr *)&un, len) == -1) {
+            vubr_die("connect");
+        }
+        cb = vubr_receive_cb;
     }
 
     dispatcher_init(&dev->dispatcher);
-    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev,
-                   vubr_accept_cb);
+    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev, cb);
 
     DPRINT("Waiting for connections on UNIX socket %s ...\n", path);
     return dev;
@@ -1369,8 +1378,9 @@ main(int argc, char *argv[])
 {
     VubrDev *dev;
     int opt;
+    bool client = false;
 
-    while ((opt = getopt(argc, argv, "l:r:u:")) != -1) {
+    while ((opt = getopt(argc, argv, "l:r:u:c")) != -1) {
 
         switch (opt) {
         case 'l':
@@ -1386,16 +1396,20 @@ main(int argc, char *argv[])
         case 'u':
             ud_socket_path = strdup(optarg);
             break;
+        case 'c':
+            client = true;
+            break;
         default:
             goto out;
         }
     }
 
-    DPRINT("ud socket: %s\n", ud_socket_path);
+    DPRINT("ud socket: %s (%s)\n", ud_socket_path,
+           client ? "client" : "server");
     DPRINT("local:     %s:%s\n", lhost, lport);
     DPRINT("remote:    %s:%s\n", rhost, rport);
 
-    dev = vubr_new(ud_socket_path);
+    dev = vubr_new(ud_socket_path, client);
     if (!dev) {
         return 1;
     }
-- 
2.7.4

[Qemu-devel] [PATCH 03/10] tests/vhost-user-bridge: workaround stale vring base

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This patch is a similar solution to what Yuanhan Liu/Huawei Xie have
suggested for DPDK. When vubr quits (killed or crashed), a restart of
vubr would get stale vring base from QEMU. That would break the kernel
virtio net completely, making it non-work any more, unless a driver
reset is done.

So, instead of getting the stale vring base from QEMU, Huawei suggested
we could get a proper one from used->idx. This works because the queues
packets are processed in order.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 tests/vhost-user-bridge.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index f907ce7..38963e4 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -946,6 +946,13 @@ vubr_set_vring_addr_exec(VubrDev *dev, VhostUserMsg *vmsg)
     DPRINT("    vring_avail at %p\n", vq->avail);
 
     vq->last_used_index = vq->used->idx;
+
+    if (vq->last_avail_index != vq->used->idx) {
+        DPRINT("Last avail index != used index: %d != %d, resuming",
+               vq->last_avail_index, vq->used->idx);
+        vq->last_avail_index = vq->used->idx;
+    }
+
     return 0;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH 04/10] qemu-char: add qemu_chr_disconnect to close a fd accepted by listen fd

[marcandre.lureau]

From: Tetsuya Mukawa <mukawa@igel.co.jp>

The patch introduces qemu_chr_disconnect(). The function is used for
closing a fd accepted by listen fd. Though we already have qemu_chr_delete(),
but it closes not only accepted fd but also listen fd. This new function
is used when we still want to keep listen fd.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 include/sysemu/char.h | 7 +++++++
 qemu-char.c           | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 307fd8f..2c39987 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -75,6 +75,7 @@ struct CharDriverState {
     IOReadHandler *chr_read;
     void *handler_opaque;
     void (*chr_close)(struct CharDriverState *chr);
+    void (*chr_disconnect)(struct CharDriverState *chr);
     void (*chr_accept_input)(struct CharDriverState *chr);
     void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
     void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
@@ -143,6 +144,12 @@ void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend);
  */
 CharDriverState *qemu_chr_new(const char *label, const char *filename,
                               void (*init)(struct CharDriverState *s));
+/**
+ * @qemu_chr_disconnect:
+ *
+ * Close a fd accpeted by character backend.
+ */
+void qemu_chr_disconnect(CharDriverState *chr);
 
 /**
  * @qemu_chr_new_noreplay:
diff --git a/qemu-char.c b/qemu-char.c
index b597ee1..6efbc0d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4012,6 +4012,13 @@ void qemu_chr_fe_release(CharDriverState *s)
     s->avail_connections++;
 }
 
+void qemu_chr_disconnect(CharDriverState *chr)
+{
+    if (chr->chr_disconnect) {
+        chr->chr_disconnect(chr);
+    }
+}
+
 static void qemu_chr_free_common(CharDriverState *chr)
 {
     g_free(chr->filename);
@@ -4405,6 +4412,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
     chr->chr_write = tcp_chr_write;
     chr->chr_sync_read = tcp_chr_sync_read;
     chr->chr_close = tcp_chr_close;
+    chr->chr_disconnect = tcp_chr_disconnect;
     chr->get_msgfds = tcp_get_msgfds;
     chr->set_msgfds = tcp_set_msgfds;
     chr->chr_add_client = tcp_chr_add_client;
-- 
2.7.4

[Qemu-devel] [PATCH 05/10] vhost-user: disconnect on start failure

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

If the backend failed to start (for example feature negociation failed),
do not exit, but disconnect the char device instead. Slightly more
robust for reconnect case.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 net/vhost-user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 4a7fd5f..41ddb4b 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -202,7 +202,8 @@ static void net_vhost_user_event(void *opaque, int event)
         s->watch = qemu_chr_fe_add_watch(s->chr, G_IO_HUP,
                                          net_vhost_user_watch, s);
         if (vhost_user_start(queues, ncs) < 0) {
-            exit(1);
+            qemu_chr_disconnect(s->chr);
+            return;
         }
         qmp_set_link(name, true, &err);
         break;
-- 
2.7.4

[Qemu-devel] [PATCH 06/10] vhost-net: do not crash if backend is not present

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Do not crash when backend is not present while enabling the ring. A
following patch will save the enabled state so it can be restored once
the backend is started.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 hw/net/vhost_net.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 6e1032f..805a0df 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -401,8 +401,13 @@ 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 = net->dev.vhost_ops;
+    const VhostOps *vhost_ops;
+
+    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.7.4

[Qemu-devel] [PATCH 07/10] vhost-net: save & restore vhost-user acked features

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The initial vhost-user connection sets the features to be negotiated
with the driver. Renegotiation isn't possible without device reset.

To handle reconnection of vhost-user backend, ensure the same set of
features are provided, and reuse already acked features.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 hw/net/vhost_net.c       | 27 ++++++++++++++++++++++++++-
 include/net/vhost-user.h |  1 +
 include/net/vhost_net.h  |  3 +++
 net/vhost-user.c         | 10 ++++++++++
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 805a0df..b28881f 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -120,6 +120,11 @@ uint64_t vhost_net_get_max_queues(VHostNetState *net)
     return net->dev.max_queues;
 }
 
+uint64_t vhost_net_get_acked_features(VHostNetState *net)
+{
+    return net->dev.acked_features;
+}
+
 static int vhost_net_get_fd(NetClientState *backend)
 {
     switch (backend->info->type) {
@@ -136,6 +141,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);
+    uint64_t features = 0;
 
     if (!options->net_backend) {
         fprintf(stderr, "vhost-net requires net backend to be setup\n");
@@ -183,8 +189,21 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
             goto fail;
         }
     }
+
     /* Set sane init value. Override when guest acks. */
-    vhost_net_ack_features(net, 0);
+    if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+        features = vhost_user_get_acked_features(net->nc);
+        if (~net->dev.features & features) {
+            fprintf(stderr, "vhost lacks feature mask %" PRIu64
+                    " for backend\n",
+                    (uint64_t)(~net->dev.features & features));
+            vhost_dev_cleanup(&net->dev);
+            goto fail;
+        }
+    }
+
+    vhost_net_ack_features(net, features);
+
     return net;
 fail:
     g_free(net);
@@ -447,10 +466,16 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
 {
     return features;
 }
+
 void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
 {
 }
 
+uint64_t vhost_net_get_acked_features(VHostNetState *net)
+{
+    return 0;
+}
+
 bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
 {
     return false;
diff --git a/include/net/vhost-user.h b/include/net/vhost-user.h
index 85109f6..efae35d 100644
--- a/include/net/vhost-user.h
+++ b/include/net/vhost-user.h
@@ -13,5 +13,6 @@
 
 struct vhost_net;
 struct vhost_net *vhost_user_get_vhost_net(NetClientState *nc);
+uint64_t vhost_user_get_acked_features(NetClientState *nc);
 
 #endif /* VHOST_USER_H_ */
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 3389b41..0bd4877 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -31,4 +31,7 @@ int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
 VHostNetState *get_vhost_net(NetClientState *nc);
 
 int vhost_set_vring_enable(NetClientState * nc, int enable);
+
+uint64_t vhost_net_get_acked_features(VHostNetState *net);
+
 #endif
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 41ddb4b..d72ce9b 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -23,6 +23,7 @@ typedef struct VhostUserState {
     CharDriverState *chr;
     VHostNetState *vhost_net;
     int watch;
+    uint64_t acked_features;
 } VhostUserState;
 
 typedef struct VhostUserChardevProps {
@@ -37,6 +38,13 @@ VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
     return s->vhost_net;
 }
 
+uint64_t vhost_user_get_acked_features(NetClientState *nc)
+{
+    VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+    return s->acked_features;
+}
+
 static int vhost_user_running(VhostUserState *s)
 {
     return (s->vhost_net) ? 1 : 0;
@@ -56,6 +64,8 @@ static void vhost_user_stop(int queues, NetClientState *ncs[])
         }
 
         if (s->vhost_net) {
+            /* save acked features */
+            s->acked_features = vhost_net_get_acked_features(s->vhost_net);
             vhost_net_cleanup(s->vhost_net);
             s->vhost_net = NULL;
         }
-- 
2.7.4

[Qemu-devel] [PATCH 08/10] vhost-net: save & restore vring enable state

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

A driver may change the vring enable state at run time but vhost-user
backend may not be present (a contrived example is when the backend is
disconnected and the device is reconfigured after driver rebinding)

Restore the vring state when the vhost-user backend is started, so it
can process the ring.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 hw/net/vhost_net.c | 11 +++++++++++
 include/net/net.h  |  1 +
 2 files changed, 12 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index b28881f..50f4dcd 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -329,6 +329,15 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         if (r < 0) {
             goto err_start;
         }
+
+        if (ncs[i].peer->vring_enable) {
+            /* restore vring enable state */
+            r = vhost_set_vring_enable(ncs[i].peer, ncs[i].peer->vring_enable);
+
+            if (r < 0) {
+                goto err_start;
+            }
+        }
     }
 
     return 0;
@@ -422,6 +431,8 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
     VHostNetState *net = get_vhost_net(nc);
     const VhostOps *vhost_ops;
 
+    nc->vring_enable = enable;
+
     if (!net) {
         return 0;
     }
diff --git a/include/net/net.h b/include/net/net.h
index a69e382..a5c5095 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -99,6 +99,7 @@ struct NetClientState {
     NetClientDestructor *destructor;
     unsigned int queue_index;
     unsigned rxfilter_notify_enabled:1;
+    int vring_enable;
     QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
 };
 
-- 
2.7.4

[Qemu-devel] [PATCH 09/10] tests: append i386 tests

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Do not overwrite x86-64 tests, re-enable vhost-user-test.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index a3e20e3..932ad2a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -228,7 +228,7 @@ endif
 check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
 check-qtest-i386-y += tests/test-filter-redirector$(EXESUF)
-check-qtest-x86_64-y = $(check-qtest-i386-y)
+check-qtest-x86_64-y += $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
-- 
2.7.4

[Qemu-devel] [PATCH 10/10] test: start vhost-user reconnect test

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This is a simple reconnect test, that simply checks if vhost-user
reconnection is possible and restore the state. A more complete test
would actually manipulate and check the ring contents (such extended
testing would benefit from the libvhost-user proposed in QEMU list to
avoid duplication of ring manipulations)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/vhost-user-test.c | 136 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 119 insertions(+), 17 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 6961596..75ac930 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -34,7 +34,7 @@
 #define QEMU_CMD_ACCEL  " -machine accel=tcg"
 #define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM,"\
                         "mem-path=%s,share=on -numa node,memdev=mem"
-#define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s"
+#define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0,romfile=./pc-bios/pxe-virtio.rom"
 
@@ -246,7 +246,12 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
 
     if (msg.size) {
         p += VHOST_USER_HDR_SIZE;
-        g_assert_cmpint(qemu_chr_fe_read_all(chr, p, msg.size), ==, msg.size);
+        size = qemu_chr_fe_read_all(chr, p, msg.size);
+        if (size != msg.size) {
+            g_test_message("Wrong message size received %d != %d\n",
+                           size, msg.size);
+            return;
+        }
     }
 
     switch (msg.request) {
@@ -363,17 +368,10 @@ static const char *init_hugepagefs(const char *path)
 static TestServer *test_server_new(const gchar *name)
 {
     TestServer *server = g_new0(TestServer, 1);
-    gchar *chr_path;
 
     server->socket_path = g_strdup_printf("%s/%s.sock", tmpfs, name);
     server->mig_path = g_strdup_printf("%s/%s.mig", tmpfs, name);
-
-    chr_path = g_strdup_printf("unix:%s,server,nowait", server->socket_path);
     server->chr_name = g_strdup_printf("chr-%s", name);
-    server->chr = qemu_chr_new(server->chr_name, chr_path, NULL);
-    g_free(chr_path);
-
-    qemu_chr_add_handlers(server->chr, chr_can_read, chr_read, NULL, server);
 
     g_mutex_init(&server->data_mutex);
     g_cond_init(&server->data_cond);
@@ -383,13 +381,34 @@ static TestServer *test_server_new(const gchar *name)
     return server;
 }
 
-#define GET_QEMU_CMD(s)                                                        \
-    g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,                 \
-                    (s)->socket_path, (s)->chr_name)
+static void test_server_create_chr(TestServer *server, const gchar *opt)
+{
+    gchar *chr_path;
 
-#define GET_QEMU_CMDE(s, mem, extra, ...)                                      \
-    g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name,       \
-                    (s)->socket_path, (s)->chr_name, ##__VA_ARGS__)
+    chr_path = g_strdup_printf("unix:%s%s", server->socket_path, opt);
+    server->chr = qemu_chr_new(server->chr_name, chr_path, NULL);
+    g_free(chr_path);
+
+    qemu_chr_add_handlers(server->chr, chr_can_read, chr_read, NULL, server);
+}
+
+static void test_server_listen(TestServer *server)
+{
+    test_server_create_chr(server, ",server,nowait");
+}
+
+static void test_server_connect(TestServer *server)
+{
+    test_server_create_chr(server, ",reconnect=1");
+}
+
+#define GET_QEMU_CMD(s)                                         \
+    g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
+                    (s)->socket_path, "", (s)->chr_name)
+
+#define GET_QEMU_CMDE(s, mem, chr_opts, extra, ...)                     \
+    g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name, \
+                    (s)->socket_path, (chr_opts), (s)->chr_name, ##__VA_ARGS__)
 
 static gboolean _test_server_free(TestServer *server)
 {
@@ -537,7 +556,10 @@ static void test_migrate(void)
     guint8 *log;
     guint64 size;
 
-    cmd = GET_QEMU_CMDE(s, 2, "");
+    test_server_listen(s);
+    test_server_listen(dest);
+
+    cmd = GET_QEMU_CMDE(s, 2, "", "");
     from = qtest_start(cmd);
     g_free(cmd);
 
@@ -545,7 +567,7 @@ static void test_migrate(void)
     size = get_log_size(s);
     g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
 
-    cmd = GET_QEMU_CMDE(dest, 2, " -incoming %s", uri);
+    cmd = GET_QEMU_CMDE(dest, 2, "", " -incoming %s", uri);
     to = qtest_init(cmd);
     g_free(cmd);
 
@@ -605,6 +627,80 @@ static void test_migrate(void)
     global_qtest = global;
 }
 
+#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+static void wait_for_rings_started(TestServer *s, size_t count)
+{
+    gint64 end_time;
+
+    g_mutex_lock(&s->data_mutex);
+    end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
+    while (ctpop64(s->rings) != count) {
+        if (!g_cond_wait_until(&s->data_cond, &s->data_mutex, end_time)) {
+            /* timeout has passed */
+            g_assert_cmpint(ctpop64(s->rings), ==, count);
+            break;
+        }
+    }
+
+    g_mutex_unlock(&s->data_mutex);
+}
+
+static gboolean
+reconnect_cb(gpointer user_data)
+{
+    TestServer *s = user_data;
+
+    qemu_chr_disconnect(s->chr);
+
+    return FALSE;
+}
+
+static gpointer
+connect_thread(gpointer data)
+{
+    TestServer *s = data;
+
+    /* wait for qemu to start before first try, to avoid extra warnings */
+    g_usleep(G_USEC_PER_SEC);
+    test_server_connect(s);
+
+    return NULL;
+}
+
+static void test_reconnect_subprocess(void)
+{
+    TestServer *s = test_server_new("reconnect");
+    char *cmd;
+
+    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);
+
+    /* reconnect */
+    s->fds_num = 0;
+    s->rings = 0;
+    g_idle_add(reconnect_cb, s);
+    wait_for_fds(s);
+    wait_for_rings_started(s, 2);
+
+    qtest_end();
+    test_server_free(s);
+    return;
+}
+
+static void test_reconnect(void)
+{
+    gchar *path = g_strdup_printf("/%s/vhost-user/reconnect/subprocess",
+                                  qtest_get_arch());
+    g_test_trap_subprocess(path, 0, 0);
+    g_test_trap_assert_passed();
+}
+#endif
+
 int main(int argc, char **argv)
 {
     QTestState *s = NULL;
@@ -636,6 +732,7 @@ int main(int argc, char **argv)
     }
 
     server = test_server_new("test");
+    test_server_listen(server);
 
     loop = g_main_loop_new(NULL, FALSE);
     /* run the main loop thread so the chardev may operate */
@@ -648,6 +745,11 @@ int main(int argc, char **argv)
 
     qtest_add_data_func("/vhost-user/read-guest-mem", server, read_guest_mem);
     qtest_add_func("/vhost-user/migrate", test_migrate);
+#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+    qtest_add_func("/vhost-user/reconnect/subprocess",
+                   test_reconnect_subprocess);
+    qtest_add_func("/vhost-user/reconnect", test_reconnect);
+#endif
 
     ret = g_test_run();
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 02/10] tests/vhost-user-bridge: add client mode

[Victor Kaplansky]

On Mon, Jun 06, 2016 at 06:45:00PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> If -c is specified, vubr will try to connect to the socket instead of
> listening for connections.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>  tests/vhost-user-bridge.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
> index 0779ba2..f907ce7 100644
> --- a/tests/vhost-user-bridge.c
> +++ b/tests/vhost-user-bridge.c
> @@ -1204,12 +1204,13 @@ vubr_accept_cb(int sock, void *ctx)
>  }
>  
>  static VubrDev *
> -vubr_new(const char *path)
> +vubr_new(const char *path, bool client)
>  {
>      VubrDev *dev = (VubrDev *) calloc(1, sizeof(VubrDev));
>      dev->nregions = 0;
>      int i;
>      struct sockaddr_un un;
> +    CallbackFunc cb;
>      size_t len;
>  
>      for (i = 0; i < MAX_NR_VIRTQUEUE; i++) {
> @@ -1238,19 +1239,27 @@ vubr_new(const char *path)
>      un.sun_family = AF_UNIX;
>      strcpy(un.sun_path, path);
>      len = sizeof(un.sun_family) + strlen(path);
> -    unlink(path);
>  
> -    if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
> -        vubr_die("bind");
> -    }
> +    if (!client) {
> +        unlink(path);
> +
> +        if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
> +            vubr_die("bind");
> +        }
>  
> -    if (listen(dev->sock, 1) == -1) {
> -        vubr_die("listen");
> +        if (listen(dev->sock, 1) == -1) {
> +            vubr_die("listen");
> +        }
> +        cb = vubr_accept_cb;
> +    } else {
> +        if (connect(dev->sock, (struct sockaddr *)&un, len) == -1) {
> +            vubr_die("connect");
> +        }
> +        cb = vubr_receive_cb;
>      }
>  
>      dispatcher_init(&dev->dispatcher);
> -    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev,
> -                   vubr_accept_cb);
> +    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev, cb);
>  
>      DPRINT("Waiting for connections on UNIX socket %s ...\n", path);

I think this message should be issued only for server mode.

>      return dev;
> @@ -1369,8 +1378,9 @@ main(int argc, char *argv[])
>  {
>      VubrDev *dev;
>      int opt;
> +    bool client = false;
>  
> -    while ((opt = getopt(argc, argv, "l:r:u:")) != -1) {
> +    while ((opt = getopt(argc, argv, "l:r:u:c")) != -1) {
>  
>          switch (opt) {
>          case 'l':
> @@ -1386,16 +1396,20 @@ main(int argc, char *argv[])
>          case 'u':
>              ud_socket_path = strdup(optarg);
>              break;
> +        case 'c':
> +            client = true;
> +            break;
>          default:
>              goto out;
>          }
>      }
>  
> -    DPRINT("ud socket: %s\n", ud_socket_path);
> +    DPRINT("ud socket: %s (%s)\n", ud_socket_path,
> +           client ? "client" : "server");
>      DPRINT("local:     %s:%s\n", lhost, lport);
>      DPRINT("remote:    %s:%s\n", rhost, rport);
>  
> -    dev = vubr_new(ud_socket_path);
> +    dev = vubr_new(ud_socket_path, client);
>      if (!dev) {
>          return 1;
>      }
> -- 
> 2.7.4
> 

Re: [Qemu-devel] [PATCH 03/10] tests/vhost-user-bridge: workaround stale vring base

[Victor Kaplansky]

On Mon, Jun 06, 2016 at 06:45:01PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This patch is a similar solution to what Yuanhan Liu/Huawei Xie have
> suggested for DPDK. When vubr quits (killed or crashed), a restart of
> vubr would get stale vring base from QEMU. That would break the kernel
> virtio net completely, making it non-work any more, unless a driver
> reset is done.
> 
> So, instead of getting the stale vring base from QEMU, Huawei suggested
> we could get a proper one from used->idx. This works because the queues
> packets are processed in order.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>  tests/vhost-user-bridge.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
> index f907ce7..38963e4 100644
> --- a/tests/vhost-user-bridge.c
> +++ b/tests/vhost-user-bridge.c
> @@ -946,6 +946,13 @@ vubr_set_vring_addr_exec(VubrDev *dev, VhostUserMsg *vmsg)
>      DPRINT("    vring_avail at %p\n", vq->avail);
>  
>      vq->last_used_index = vq->used->idx;
> +
> +    if (vq->last_avail_index != vq->used->idx) {
> +        DPRINT("Last avail index != used index: %d != %d, resuming",
> +               vq->last_avail_index, vq->used->idx);
> +        vq->last_avail_index = vq->used->idx;
> +    }
> +

What if set_vring_base is called after set_vring_addr?
Maybe it is worth to add the fixup to the set_vring_base as well?

>      return 0;
>  }
>  
> -- 
> 2.7.4
> 
> 

Re: [Qemu-devel] [PATCH 02/10] tests/vhost-user-bridge: add client mode

[Marc-André Lureau]

Hi

On Thu, Jun 9, 2016 at 11:53 AM, Victor Kaplansky <victork@redhat.com> wrote:
> On Mon, Jun 06, 2016 at 06:45:00PM +0200, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> If -c is specified, vubr will try to connect to the socket instead of
>> listening for connections.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> ---
>>  tests/vhost-user-bridge.c | 38 ++++++++++++++++++++++++++------------
>>  1 file changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
>> index 0779ba2..f907ce7 100644
>> --- a/tests/vhost-user-bridge.c
>> +++ b/tests/vhost-user-bridge.c
>> @@ -1204,12 +1204,13 @@ vubr_accept_cb(int sock, void *ctx)
>>  }
>>
>>  static VubrDev *
>> -vubr_new(const char *path)
>> +vubr_new(const char *path, bool client)
>>  {
>>      VubrDev *dev = (VubrDev *) calloc(1, sizeof(VubrDev));
>>      dev->nregions = 0;
>>      int i;
>>      struct sockaddr_un un;
>> +    CallbackFunc cb;
>>      size_t len;
>>
>>      for (i = 0; i < MAX_NR_VIRTQUEUE; i++) {
>> @@ -1238,19 +1239,27 @@ vubr_new(const char *path)
>>      un.sun_family = AF_UNIX;
>>      strcpy(un.sun_path, path);
>>      len = sizeof(un.sun_family) + strlen(path);
>> -    unlink(path);
>>
>> -    if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
>> -        vubr_die("bind");
>> -    }
>> +    if (!client) {
>> +        unlink(path);
>> +
>> +        if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
>> +            vubr_die("bind");
>> +        }
>>
>> -    if (listen(dev->sock, 1) == -1) {
>> -        vubr_die("listen");
>> +        if (listen(dev->sock, 1) == -1) {
>> +            vubr_die("listen");
>> +        }
>> +        cb = vubr_accept_cb;
>> +    } else {
>> +        if (connect(dev->sock, (struct sockaddr *)&un, len) == -1) {
>> +            vubr_die("connect");
>> +        }
>> +        cb = vubr_receive_cb;
>>      }
>>
>>      dispatcher_init(&dev->dispatcher);
>> -    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev,
>> -                   vubr_accept_cb);
>> +    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev, cb);
>>
>>      DPRINT("Waiting for connections on UNIX socket %s ...\n", path);
>
> I think this message should be issued only for server mode.

That should improve this patch:

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index 38963e4..36b3cd8 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -1258,6 +1258,8 @@ vubr_new(const char *path, bool client)
             vubr_die("listen");
         }
         cb = vubr_accept_cb;
+
+        DPRINT("Waiting for connections on UNIX socket %s ...\n", path);
     } else {
         if (connect(dev->sock, (struct sockaddr *)&un, len) == -1) {
             vubr_die("connect");
@@ -1268,7 +1270,6 @@ vubr_new(const char *path, bool client)
     dispatcher_init(&dev->dispatcher);
     dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev, cb);

-    DPRINT("Waiting for connections on UNIX socket %s ...\n", path);
     return dev;
 }

@@ -1427,13 +1428,14 @@ main(int argc, char *argv[])

 out:
     fprintf(stderr, "Usage: %s ", argv[0]);
-    fprintf(stderr, "[-u ud_socket_path] [-l lhost:lport] [-r rhost:rport]\n");
+    fprintf(stderr, "[-c] [-u ud_socket_path] [-l lhost:lport] [-r
rhost:rport]\n");
     fprintf(stderr, "\t-u path to unix doman socket. default: %s\n",
             DEFAULT_UD_SOCKET);
     fprintf(stderr, "\t-l local host and port. default: %s:%s\n",
             DEFAULT_LHOST, DEFAULT_LPORT);
     fprintf(stderr, "\t-r remote host and port. default: %s:%s\n",
             DEFAULT_RHOST, DEFAULT_RPORT);
+    fprintf(stderr, "\t-c client mode\n");

     return 1;
 }


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/10] tests/vhost-user-bridge: workaround stale vring base

[Marc-André Lureau]

Hi

On Thu, Jun 9, 2016 at 12:07 PM, Victor Kaplansky <victork@redhat.com> wrote:
> What if set_vring_base is called after set_vring_addr?
> Maybe it is worth to add the fixup to the set_vring_base as well?

It would need to handle conditions like set_vring_base() being called
while set_vring_addr() is not yet, and thus vq->used isn't set.

Imho it's not necessary, since order is currently fixed in
vhost_virtqueue_start(), but we could specify this in the protocol to
avoid too much possible states.

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 00/10] vhost-user: simple reconnection support

[Victor Kaplansky]

Hi MST, I've reviewed the patches and they are fine.
>From my perspective we can pull them into upstream.


On Mon, Jun 06, 2016 at 06:44:58PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> In a previous series "RFCv2: vhost-user: shutdown and reconnection", I
> proposed to add a new slave request to handle graceful shutdown, for
> both qemu configuration, server or client, while keeping the guest
> running with link down status.
> 
> However, for the simple case where qemu is configured as server, and
> the backend processes packets in order and disconnects, it is possible
> for the backend to recover after reconnection by discarding the qemu
> SET_VRING_BASE value and resuming from the used->index instead. This
> simplifies the reconnection support in this particular situation (it
> would make sense to have the backend declare this behaviour with an
> extra flag)
> 
> The guest won't be notified of link status change and queues may not
> be processed in a timely manner, also qemu may assert if some
> vhost-user commands are happening while the backend is disconnected.
> So the reconnection must happen "quickly enough" here. In order to
> keep the series relatively small, these further problems will be
> addressed later.
> 
> These series demonstrates a simple reconnect support for vubr
> (configured as client and qemu as server), includes some nice to
> have fixes and a simple test.
> 
> rfcv3->v1:
> - rebased
> - added add a dummy vhost_net_get_acked_features()
>   implementation for !VHOST_NET
> - changed vubr->tests/vhost-user-bridge in summary
> - made the test silent by using a subprocess
> - added s-o-b/tested-by/r-b tags
> 
> Marc-André Lureau (8):
>   tests/vhost-user-bridge: add client mode
>   tests/vhost-user-bridge: workaround stale vring base
>   vhost-user: disconnect on start failure
>   vhost-net: do not crash if backend is not present
>   vhost-net: save & restore vhost-user acked features
>   vhost-net: save & restore vring enable state
>   tests: append i386 tests
>   test: start vhost-user reconnect test
> 
> Tetsuya Mukawa (2):
>   vhost-user: add ability to know vhost-user backend disconnection
>   qemu-char: add qemu_chr_disconnect to close a fd accepted by listen fd
> 
>  hw/net/vhost_net.c        |  45 ++++++++++++++-
>  include/net/net.h         |   1 +
>  include/net/vhost-user.h  |   1 +
>  include/net/vhost_net.h   |   3 +
>  include/sysemu/char.h     |   7 +++
>  net/vhost-user.c          |  32 ++++++++++-
>  qemu-char.c               |   8 +++
>  tests/Makefile            |   2 +-
>  tests/vhost-user-bridge.c |  45 +++++++++++----
>  tests/vhost-user-test.c   | 136 ++++++++++++++++++++++++++++++++++++++++------
>  10 files changed, 247 insertions(+), 33 deletions(-)
> 
> -- 
> 2.7.4
> 
> 

Re: [Qemu-devel] [PATCH 00/10] vhost-user: simple reconnection support

[Michael S. Tsirkin]

On Thu, Jun 09, 2016 at 05:14:42PM +0300, Victor Kaplansky wrote:
> Hi MST, I've reviewed the patches and they are fine.
> >From my perspective we can pull them into upstream.

In the future please send:

Reviewed-by: Victor Kaplansky <victork@redhat.com>


Thanks!

-- 
MST

Re: [Qemu-devel] [PATCH 03/10] tests/vhost-user-bridge: workaround stale vring base

[Michael S. Tsirkin]

On Thu, Jun 09, 2016 at 12:25:54PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jun 9, 2016 at 12:07 PM, Victor Kaplansky <victork@redhat.com> wrote:
> > What if set_vring_base is called after set_vring_addr?
> > Maybe it is worth to add the fixup to the set_vring_base as well?
> 
> It would need to handle conditions like set_vring_base() being called
> while set_vring_addr() is not yet, and thus vq->used isn't set.
> 
> Imho it's not necessary, since order is currently fixed in
> vhost_virtqueue_start(), but we could specify this in the protocol to
> avoid too much possible states.

I personally think it's better to just get the used idx
from memory before reconnecting.

Will fix old clients automatically.

Whoever wants to support old QEMU, can do this by a work-around
similar to the supplied one. Maybe add code here to make
sure everything is setup - if not it's a new QEMU so
it does not need the work-around.

I think there was a patch like this suggested at some point -
mind digging it up?

> -- 
> Marc-André Lureau