[Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Tetsuya Mukawa]

Hi guys,

Here are patches to add feature to start QEMU without vhost-user backend.
Currently, if we want to use vhost-user backend, the backend must start before
QEMU. Also, if QEMU or the backend is closed unexpectedly, there is no way to
recover without restarting both applications. Practically, it's not useful.

This patch series adds following features.
 - QEMU can start before the backend.
 - QEMU or the backend can restart anytime.
   connectivity will be recovered automatically, when app starts again.
   (if QEMU is server, QEMU just wait reconnection)
   while lost connection, link status of virtio-net device is down,
   so virtio-net driver on the guest can know it

To work like above, the patch introduces flags to specify features vhost-user
backend will support.

Here are examples.
('backend_features' is the new flags. Each bit of the flag represents
VIRTIO_NET_F_* in linux/virtio_net.h)

    * QEMU is configured as vhost-user client.
     -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
     -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
     -device virtio-net-pci,netdev=net0 \

    * QEMU is configured as vhost-user server.
     -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
     -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
     -device virtio-net-pci,netdev=net0 \

When virtio-net device is configured by virtio-net driver, QEMU should know
vhost-user backend features. But if QEMU starts without the backend, QEMU cannot
know it. So above the feature values specified by user will be used as features
the backend will support.

When connection between QEMU and the backend is established, QEMU checkes feature
values of the backend to make sure the expected features are provided.
If it doesn't, the connection will be closed by QEMU.

Regards,
Tetsuya

----------
Changes
----------
- v2 changes from v1 patch
   - Rebase to latest master.
   - Change user interface to be able to specify each feature by UINT64.
   - Replace backend_* to backend-* in qapi schema.
   - Use close(2) interface for opended socket instead of using shutdown(2)
     interface.
   - Split 2nd patch of v1 into 2nd and 3rd patch of v2.
   - Fix commit title and body.
   - Add comment, and fix indent.
   - Use {} even for single statement if bodies.
   - Use PRIx64 instead of %lx.


- v1 changes from RFC patch
  The last patch of this series was changed like below.
   - Rebase to latest master.
   - Remove needless has_backend_feature variable.
   - Change user interface to be able to specify each feature by name.
   - Add (Since 2.4) to schema file.
   - Fix commit title and body.

Tetsuya Mukawa (5):
  vhost-user: Add ability to know vhost-user backend disconnection
  qemu-char: Add qemu_chr_disconnect to close a fd accepted by listen fd
  vhost-user: Shutdown vhost-user connection when wrong messages are
    passed
  vhost-user: Enable 'nowait' and 'reconnect' option
  vhost-user: Add new option to specify vhost-user backend supports

 hw/net/vhost_net.c             |  6 +++++-
 hw/net/virtio-net.c            | 15 +++++++++++++++
 hw/scsi/vhost-scsi.c           |  2 +-
 hw/virtio/vhost-user.c         | 24 ++++++++++++++++-------
 hw/virtio/vhost.c              |  3 ++-
 include/hw/virtio/vhost.h      |  3 ++-
 include/hw/virtio/virtio-net.h |  1 +
 include/net/net.h              |  3 +++
 include/net/vhost_net.h        |  1 +
 include/sysemu/char.h          |  7 +++++++
 net/net.c                      |  9 +++++++++
 net/tap.c                      |  1 +
 net/vhost-user.c               | 43 ++++++++++++++++++++++++++++++++++++++++--
 qapi-schema.json               | 12 ++++++++++--
 qemu-char.c                    |  8 ++++++++
 qemu-options.hx                |  3 ++-
 16 files changed, 125 insertions(+), 16 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v2 1/5] vhost-user: Add ability to know vhost-user backend disconnection

[Tetsuya Mukawa]

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>
---
 net/vhost-user.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index b51bc04..8b7749a 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -19,6 +19,7 @@ typedef struct VhostUserState {
     NetClientState nc;
     CharDriverState *chr;
     VHostNetState *vhost_net;
+    int watch;
 } VhostUserState;
 
 typedef struct VhostUserChardevProps {
@@ -112,12 +113,27 @@ static void net_vhost_link_down(VhostUserState *s, bool link_down)
     }
 }
 
+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)
 {
     VhostUserState *s = opaque;
 
     switch (event) {
     case CHR_EVENT_OPENED:
+        s->watch = qemu_chr_fe_add_watch(s->chr, G_IO_HUP, net_vhost_user_watch, s);
         vhost_user_start(s);
         net_vhost_link_down(s, false);
         error_report("chardev \"%s\" went up", s->nc.info_str);
@@ -125,6 +141,8 @@ static void net_vhost_user_event(void *opaque, int event)
     case CHR_EVENT_CLOSED:
         net_vhost_link_down(s, true);
         vhost_user_stop(s);
+        g_source_remove(s->watch);
+        s->watch = 0;
         error_report("chardev \"%s\" went down", s->nc.info_str);
         break;
     }
-- 
2.1.4

[Qemu-devel] [PATCH v2 2/5] qemu-char: Add qemu_chr_disconnect to close a fd accepted by listen fd

[Tetsuya Mukawa]

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>
---
 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 832b7fe..141edbd 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -70,6 +70,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);
@@ -124,6 +125,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
  */
 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_delete:
diff --git a/qemu-char.c b/qemu-char.c
index d0c1564..122632b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3836,6 +3836,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);
+    }
+}
+
 void qemu_chr_delete(CharDriverState *chr)
 {
     QTAILQ_REMOVE(&chardevs, chr, next);
@@ -4154,6 +4161,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
     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.1.4

[Qemu-devel] [PATCH v2 3/5] vhost-user: Shutdown vhost-user connection when wrong messages are passed

[Tetsuya Mukawa]

When wrong vhost-user message are passed, the connection should be shutdown.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 hw/virtio/vhost-user.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index d6f2163..2215c39 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -183,6 +183,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
 static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         void *arg)
 {
+    CharDriverState *chr = dev->opaque;
     VhostUserMsg msg;
     VhostUserRequest msg_request;
     struct vhost_vring_file *file = 0;
@@ -242,7 +243,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         if (!fd_num) {
             error_report("Failed initializing vhost-user memory map, "
                     "consider using -object memory-backend-file share=on");
-            return -1;
+            goto close;
         }
 
         msg.size = sizeof(m.memory.nregions);
@@ -289,7 +290,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
     default:
         error_report("vhost-user trying to send unhandled ioctl");
-        return -1;
+        goto close;
         break;
     }
 
@@ -305,33 +306,36 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         if (msg_request != msg.request) {
             error_report("Received unexpected msg type."
                     " Expected %d received %d", msg_request, msg.request);
-            return -1;
+            goto close;
         }
 
         switch (msg_request) {
         case VHOST_USER_GET_FEATURES:
             if (msg.size != sizeof(m.u64)) {
                 error_report("Received bad msg size.");
-                return -1;
+                goto close;
             }
             *((__u64 *) arg) = msg.u64;
             break;
         case VHOST_USER_GET_VRING_BASE:
             if (msg.size != sizeof(m.state)) {
                 error_report("Received bad msg size.");
-                return -1;
+                goto close;
             }
             msg.state.index -= dev->vq_index;
             memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
             break;
         default:
             error_report("Received unexpected msg type.");
-            return -1;
-            break;
+            goto close;
         }
     }
 
     return 0;
+
+close:
+    qemu_chr_disconnect(chr);
+    return -1;
 }
 
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
-- 
2.1.4

[Qemu-devel] [PATCH v2 4/5] vhost-user: Enable 'nowait' and 'reconnect' option

[Tetsuya Mukawa]

The patch enables 'nowait' option for server mode, and 'reconnect'
option for client mode.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 net/vhost-user.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 8b7749a..58cd5dc 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -26,6 +26,8 @@ typedef struct VhostUserChardevProps {
     bool is_socket;
     bool is_unix;
     bool is_server;
+    bool is_nowait;
+    bool is_reconnect;
 } VhostUserChardevProps;
 
 VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
@@ -186,6 +188,10 @@ static int net_vhost_chardev_opts(void *opaque,
         props->is_unix = true;
     } else if (strcmp(name, "server") == 0) {
         props->is_server = true;
+    } else if ((strcmp(name, "wait") == 0) && (strcmp(value, "off")) == 0) {
+        props->is_nowait = true;
+    } else if (strcmp(name, "reconnect") == 0) {
+        props->is_reconnect = true;
     } else {
         error_setg(errp,
                    "vhost-user does not support a chardev with option %s=%s",
-- 
2.1.4

[Qemu-devel] [PATCH v2 5/5] vhost-user: Add new option to specify vhost-user backend supports

[Tetsuya Mukawa]

This patch adds 'backend_features' option for vhost-user backends.
If this option is specified, QEMU assumes vhost-user backends support
the features specified by user, and QEMU can start without vhost-user
backend.

Here are examples.
* QEMU is configured as vhost-user client.
 -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
 -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
 -device virtio-net-pci,netdev=net0 \

* QEMU is configured as vhost-user server.
 -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
 -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
 -device virtio-net-pci,netdev=net0 \

To know vhost-user backend features that the backend expects, please
specify 0xffffffff as backend-features, then invoke QEMU and check error log
like below.

  Lack of backend features. Expected 0xffffffff, but receives 0x68000

Above log indicates the backend features QEMU should be passed.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 hw/net/vhost_net.c             |  6 +++++-
 hw/net/virtio-net.c            | 15 +++++++++++++++
 hw/scsi/vhost-scsi.c           |  2 +-
 hw/virtio/vhost-user.c         |  6 ++++++
 hw/virtio/vhost.c              |  3 ++-
 include/hw/virtio/vhost.h      |  3 ++-
 include/hw/virtio/virtio-net.h |  1 +
 include/net/net.h              |  3 +++
 include/net/vhost_net.h        |  1 +
 net/net.c                      |  9 +++++++++
 net/tap.c                      |  1 +
 net/vhost-user.c               | 19 +++++++++++++++++--
 qapi-schema.json               | 12 ++++++++++--
 qemu-options.hx                |  3 ++-
 14 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 9bd360b..b9425ea 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -162,8 +162,12 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
     net->dev.vqs = net->vqs;
     net->dev.vq_index = net->nc->queue_index;
 
+    if (options->backend_features) {
+        net->dev.backend_features = options->backend_features;
+    }
+
     r = vhost_dev_init(&net->dev, options->opaque,
-                       options->backend_type);
+                       options->backend_type, options->backend_features);
     if (r < 0) {
         goto fail;
     }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d728233..7138f4e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -363,6 +363,18 @@ static int peer_has_ufo(VirtIONet *n)
     return n->has_ufo;
 }
 
+static uint64_t peer_backend_features(VirtIONet *n)
+{
+    if (!peer_has_vnet_hdr(n)) {
+        return 0;
+    }
+
+    n->backend_features =
+            qemu_backend_features(qemu_get_queue(n->nic)->peer);
+
+    return n->backend_features;
+}
+
 static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs,
                                        int version_1)
 {
@@ -467,6 +479,9 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features)
 
     if (!get_vhost_net(nc->peer)) {
         virtio_add_feature(&features, VIRTIO_F_VERSION_1);
+        if (peer_backend_features(n)) {
+            features = peer_backend_features(n);
+        }
         return features;
     }
     return vhost_net_get_features(get_vhost_net(nc->peer), features);
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 1c389c4..1d7957c 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -246,7 +246,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     s->dev.backend_features = 0;
 
     ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)vhostfd,
-                         VHOST_BACKEND_TYPE_KERNEL);
+                         VHOST_BACKEND_TYPE_KERNEL, 0);
     if (ret < 0) {
         error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
                    strerror(-ret));
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 2215c39..3caa1a0 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -315,6 +315,12 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
                 error_report("Received bad msg size.");
                 goto close;
             }
+            if (dev->backend_features != (dev->backend_features & msg.u64)) {
+                error_report("Lack of backend features. "
+                            "Expected 0x%llx, but receives 0x%" PRIx64,
+                            dev->backend_features, msg.u64);
+                goto close;
+            }
             *((__u64 *) arg) = msg.u64;
             break;
         case VHOST_USER_GET_VRING_BASE:
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index a6dcc79..bbfc336 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -901,7 +901,8 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
 }
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-                   VhostBackendType backend_type)
+                   VhostBackendType backend_type,
+                   unsigned long long backend_features)
 {
     uint64_t features;
     int i, r;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index dd51050..074b36b 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -60,7 +60,8 @@ struct vhost_dev {
 };
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-                   VhostBackendType backend_type);
+                   VhostBackendType backend_type,
+                   unsigned long long backend_features);
 void vhost_dev_cleanup(struct vhost_dev *hdev);
 bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev);
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 280dacf..871dc3c 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -70,6 +70,7 @@ typedef struct VirtIONet {
     size_t guest_hdr_len;
     uint32_t host_features;
     uint8_t has_ufo;
+    uint64_t backend_features;
     int mergeable_rx_bufs;
     uint8_t promisc;
     uint8_t allmulti;
diff --git a/include/net/net.h b/include/net/net.h
index 4306252..d4379d1 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -57,6 +57,7 @@ typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
 typedef void (SetVnetHdrLen)(NetClientState *, int);
 typedef int (SetVnetLE)(NetClientState *, bool);
 typedef int (SetVnetBE)(NetClientState *, bool);
+typedef unsigned long long (BackendFeatures)(NetClientState *);
 
 typedef struct NetClientInfo {
     NetClientOptionsKind type;
@@ -77,6 +78,7 @@ typedef struct NetClientInfo {
     SetVnetHdrLen *set_vnet_hdr_len;
     SetVnetLE *set_vnet_le;
     SetVnetBE *set_vnet_be;
+    BackendFeatures *backend_features;
 } NetClientInfo;
 
 struct NetClientState {
@@ -140,6 +142,7 @@ bool qemu_has_ufo(NetClientState *nc);
 bool qemu_has_vnet_hdr(NetClientState *nc);
 bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
 void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
+unsigned long long qemu_backend_features(NetClientState *nc);
 void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
                       int ecn, int ufo);
 void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 840d4b1..0b19b4a 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -11,6 +11,7 @@ typedef struct VhostNetOptions {
     VhostBackendType backend_type;
     NetClientState *net_backend;
     void *opaque;
+    unsigned long long backend_features;
 } VhostNetOptions;
 
 struct vhost_net *vhost_net_init(VhostNetOptions *options);
diff --git a/net/net.c b/net/net.c
index 6dbd61a..4ad7a69 100644
--- a/net/net.c
+++ b/net/net.c
@@ -528,6 +528,15 @@ int qemu_set_vnet_be(NetClientState *nc, bool is_be)
     return nc->info->set_vnet_be(nc, is_be);
 }
 
+unsigned long long qemu_backend_features(NetClientState *nc)
+{
+    if (!nc || !nc->info->backend_features) {
+        return false;
+    }
+
+    return nc->info->backend_features(nc);
+}
+
 int qemu_can_send_packet(NetClientState *sender)
 {
     int vm_running = runstate_is_running();
diff --git a/net/tap.c b/net/tap.c
index bd01590..743c168 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -662,6 +662,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
 
         options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
         options.net_backend = &s->nc;
+        options.backend_features = tap->backend_features;
 
         if (tap->has_vhostfd || tap->has_vhostfds) {
             vhostfd = monitor_fd_param(cur_mon, vhostfdname, &err);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 58cd5dc..fd46e32 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -19,6 +19,7 @@ typedef struct VhostUserState {
     NetClientState nc;
     CharDriverState *chr;
     VHostNetState *vhost_net;
+    unsigned long long backend_features;
     int watch;
 } VhostUserState;
 
@@ -53,6 +54,7 @@ static int vhost_user_start(VhostUserState *s)
     options.backend_type = VHOST_BACKEND_TYPE_USER;
     options.net_backend = &s->nc;
     options.opaque = s->chr;
+    options.backend_features = s->backend_features;
 
     s->vhost_net = vhost_net_init(&options);
 
@@ -90,12 +92,22 @@ static bool vhost_user_has_ufo(NetClientState *nc)
     return true;
 }
 
+static unsigned long long vhost_user_backend_features(NetClientState *nc)
+{
+    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+
+    VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+
+    return s->backend_features;
+}
+
 static NetClientInfo net_vhost_user_info = {
         .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
         .size = sizeof(VhostUserState),
         .cleanup = vhost_user_cleanup,
         .has_vnet_hdr = vhost_user_has_vnet_hdr,
         .has_ufo = vhost_user_has_ufo,
+        .backend_features = vhost_user_backend_features,
 };
 
 static void net_vhost_link_down(VhostUserState *s, bool link_down)
@@ -152,7 +164,8 @@ static void net_vhost_user_event(void *opaque, int event)
 
 static int net_vhost_user_init(NetClientState *peer, const char *device,
                                const char *name, CharDriverState *chr,
-                               uint32_t queues)
+                               uint32_t queues,
+                               unsigned long long backend_features)
 {
     NetClientState *nc;
     VhostUserState *s;
@@ -170,6 +183,7 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
         s->nc.receive_disabled = 1;
         s->chr = chr;
         s->nc.queue_index = i;
+        s->backend_features = backend_features;
 
         qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
     }
@@ -279,5 +293,6 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
         queues = 1;
     }
 
-    return net_vhost_user_init(peer, "vhost_user", name, chr, queues);
+    return net_vhost_user_init(peer, "vhost_user", name, chr, queues,
+                                vhost_user_opts->backend_features);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index 106008c..88760e5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2262,6 +2262,9 @@
 #
 # @queues: #optional number of queues to be created for multiqueue capable tap
 #
+# @backend-features: #optional feature flag to support vhost user backend
+#                    (since 2.4)
+#
 # Since 1.2
 ##
 { 'struct': 'NetdevTapOptions',
@@ -2278,7 +2281,8 @@
     '*vhostfd':    'str',
     '*vhostfds':   'str',
     '*vhostforce': 'bool',
-    '*queues':     'uint32'} }
+    '*queues':     'uint32',
+    '*backend-features':'uint64'} }
 
 ##
 # @NetdevSocketOptions
@@ -2466,13 +2470,17 @@
 # @queues: #optional number of queues to be created for multiqueue vhost-user
 #          (default: 1) (Since 2.4)
 #
+# @backend-features: #optional feature flag to support vhost user backend
+#                    (default: 0) (since 2.4)
+#
 # Since 2.1
 ##
 { 'struct': 'NetdevVhostUserOptions',
   'data': {
     'chardev':        'str',
     '*vhostforce':    'bool',
-    '*queues':        'uint32' } }
+    '*queues':        'uint32',
+    '*backend-features':    'uint64' } }
 
 ##
 # @NetClientOptions
diff --git a/qemu-options.hx b/qemu-options.hx
index 7959dd0..69f13c2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1473,7 +1473,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
 #else
     "-netdev tap,id=str[,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile]\n"
     "         [,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
-    "         [,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
+    "         [,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n][,backend-features=n]\n"
     "                configure a host TAP network backend with ID 'str'\n"
     "                use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
     "                to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
@@ -1493,6 +1493,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "                use 'vhostfd=h' to connect to an already opened vhost net device\n"
     "                use 'vhostfds=x:y:...:z to connect to multiple already opened vhost net devices\n"
     "                use 'queues=n' to specify the number of queues to be created for multiqueue TAP\n"
+    "                use 'backend-features=n' to specify the features that vhost backend supported\n"
     "-netdev bridge,id=str[,br=bridge][,helper=helper]\n"
     "                configure a host TAP network backend with ID 'str' that is\n"
     "                connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE ")\n"
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 3/5] vhost-user: Shutdown vhost-user connection when wrong messages are passed

[Michael S. Tsirkin]

On Mon, Jun 22, 2015 at 12:50:46PM +0900, Tetsuya Mukawa wrote:
> When wrong vhost-user message are passed, the connection should be shutdown.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

This silently changes the protocol semantics: previously
unknown messages were ignored. We can't do this.
See email titled "vhost-user: protocol extensions" which relies on this
to detect protocol version.


> ---
>  hw/virtio/vhost-user.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index d6f2163..2215c39 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -183,6 +183,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>  static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
>          void *arg)
>  {
> +    CharDriverState *chr = dev->opaque;
>      VhostUserMsg msg;
>      VhostUserRequest msg_request;
>      struct vhost_vring_file *file = 0;
> @@ -242,7 +243,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
>          if (!fd_num) {
>              error_report("Failed initializing vhost-user memory map, "
>                      "consider using -object memory-backend-file share=on");
> -            return -1;
> +            goto close;
>          }
>  
>          msg.size = sizeof(m.memory.nregions);
> @@ -289,7 +290,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
>          break;
>      default:
>          error_report("vhost-user trying to send unhandled ioctl");
> -        return -1;
> +        goto close;
>          break;
>      }
>  
> @@ -305,33 +306,36 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
>          if (msg_request != msg.request) {
>              error_report("Received unexpected msg type."
>                      " Expected %d received %d", msg_request, msg.request);
> -            return -1;
> +            goto close;
>          }
>  
>          switch (msg_request) {
>          case VHOST_USER_GET_FEATURES:
>              if (msg.size != sizeof(m.u64)) {
>                  error_report("Received bad msg size.");
> -                return -1;
> +                goto close;
>              }
>              *((__u64 *) arg) = msg.u64;
>              break;
>          case VHOST_USER_GET_VRING_BASE:
>              if (msg.size != sizeof(m.state)) {
>                  error_report("Received bad msg size.");
> -                return -1;
> +                goto close;
>              }
>              msg.state.index -= dev->vq_index;
>              memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
>              break;
>          default:
>              error_report("Received unexpected msg type.");
> -            return -1;
> -            break;
> +            goto close;
>          }
>      }
>  
>      return 0;
> +
> +close:
> +    qemu_chr_disconnect(chr);
> +    return -1;
>  }
>  
>  static int vhost_user_init(struct vhost_dev *dev, void *opaque)
> -- 
> 2.1.4

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Michael S. Tsirkin]

On Mon, Jun 22, 2015 at 12:50:43PM +0900, Tetsuya Mukawa wrote:
> Hi guys,
> 
> Here are patches to add feature to start QEMU without vhost-user backend.
> Currently, if we want to use vhost-user backend, the backend must start before
> QEMU.

Aren't you looking for something like xinetd?

> Also, if QEMU or the backend is closed unexpectedly, there is no way to
> recover without restarting both applications.

This was previously discussed:
https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00585.html
It doesn't look like any of the issues have been addressed.

In my opinion, the right way to handle this requires
guest cooperation.  For example, we can extend virtio 1 to allow a
single queue to reset.

We then can do something like the following:
- hypervisor detects queue backend disconnect.
- hypervisor sets flag and notifies guest
- guest resets queue, optionally discarding queued packets
- hypervisor detects (or requests if it's a client), backend reconnect
- hypervisor detects guest queue reset
- hypervisor notifies guests about backend reconnect
- hypervisor sends hypervisor device state (e.g. queue addresses and
  indices) to backend

Reconnect isn't robust without such an extension.

> Practically, it's not useful.

One thing we do seem to lack is specifying an action
taken on disconnect. It could be an action similar to r/werror.
One extra useful option could be to exit qemu.

> This patch series adds following features.
>  - QEMU can start before the backend.

I don't see the point of this one.

>  - QEMU or the backend can restart anytime.
>    connectivity will be recovered automatically, when app starts again.
>    (if QEMU is server, QEMU just wait reconnection)
>    while lost connection, link status of virtio-net device is down,
>    so virtio-net driver on the guest can know it
> 
> To work like above, the patch introduces flags to specify features vhost-user
> backend will support.
> 
> Here are examples.
> ('backend_features' is the new flags. Each bit of the flag represents
> VIRTIO_NET_F_* in linux/virtio_net.h)
> 
>     * QEMU is configured as vhost-user client.
>      -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
>      -device virtio-net-pci,netdev=net0 \
> 
>     * QEMU is configured as vhost-user server.
>      -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
>      -device virtio-net-pci,netdev=net0 \

This relies on management knowing the feature bitmap encoding of the
backend, not sure how practical that is.

> 
> When virtio-net device is configured by virtio-net driver, QEMU should know
> vhost-user backend features. But if QEMU starts without the backend, QEMU cannot
> know it. So above the feature values specified by user will be used as features
> the backend will support.
> 
> When connection between QEMU and the backend is established, QEMU checkes feature
> values of the backend to make sure the expected features are provided.
> If it doesn't, the connection will be closed by QEMU.
> 
> Regards,
> Tetsuya
> 
> ----------
> Changes
> ----------
> - v2 changes from v1 patch
>    - Rebase to latest master.
>    - Change user interface to be able to specify each feature by UINT64.
>    - Replace backend_* to backend-* in qapi schema.
>    - Use close(2) interface for opended socket instead of using shutdown(2)
>      interface.
>    - Split 2nd patch of v1 into 2nd and 3rd patch of v2.
>    - Fix commit title and body.
>    - Add comment, and fix indent.
>    - Use {} even for single statement if bodies.
>    - Use PRIx64 instead of %lx.
> 
> 
> - v1 changes from RFC patch
>   The last patch of this series was changed like below.
>    - Rebase to latest master.
>    - Remove needless has_backend_feature variable.
>    - Change user interface to be able to specify each feature by name.
>    - Add (Since 2.4) to schema file.
>    - Fix commit title and body.
> 
> Tetsuya Mukawa (5):
>   vhost-user: Add ability to know vhost-user backend disconnection
>   qemu-char: Add qemu_chr_disconnect to close a fd accepted by listen fd
>   vhost-user: Shutdown vhost-user connection when wrong messages are
>     passed
>   vhost-user: Enable 'nowait' and 'reconnect' option
>   vhost-user: Add new option to specify vhost-user backend supports
> 
>  hw/net/vhost_net.c             |  6 +++++-
>  hw/net/virtio-net.c            | 15 +++++++++++++++
>  hw/scsi/vhost-scsi.c           |  2 +-
>  hw/virtio/vhost-user.c         | 24 ++++++++++++++++-------
>  hw/virtio/vhost.c              |  3 ++-
>  include/hw/virtio/vhost.h      |  3 ++-
>  include/hw/virtio/virtio-net.h |  1 +
>  include/net/net.h              |  3 +++
>  include/net/vhost_net.h        |  1 +
>  include/sysemu/char.h          |  7 +++++++
>  net/net.c                      |  9 +++++++++
>  net/tap.c                      |  1 +
>  net/vhost-user.c               | 43 ++++++++++++++++++++++++++++++++++++++++--
>  qapi-schema.json               | 12 ++++++++++--
>  qemu-char.c                    |  8 ++++++++
>  qemu-options.hx                |  3 ++-
>  16 files changed, 125 insertions(+), 16 deletions(-)
> 
> -- 
> 2.1.4

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Tetsuya Mukawa]

On 2015/06/22 17:14, Michael S. Tsirkin wrote:
> On Mon, Jun 22, 2015 at 12:50:43PM +0900, Tetsuya Mukawa wrote:
>> Hi guys,
>>
>> Here are patches to add feature to start QEMU without vhost-user backend.
>> Currently, if we want to use vhost-user backend, the backend must start before
>> QEMU.
> Aren't you looking for something like xinetd?

It will help for restricting starting order, but not help for
reconnection of vhost-user backend.

>> Also, if QEMU or the backend is closed unexpectedly, there is no way to
>> recover without restarting both applications.
> This was previously discussed:
> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00585.html
> It doesn't look like any of the issues have been addressed.
>
> In my opinion, the right way to handle this requires
> guest cooperation.  For example, we can extend virtio 1 to allow a
> single queue to reset.
>
> We then can do something like the following:
> - hypervisor detects queue backend disconnect.
> - hypervisor sets flag and notifies guest
> - guest resets queue, optionally discarding queued packets
> - hypervisor detects (or requests if it's a client), backend reconnect
> - hypervisor detects guest queue reset
> - hypervisor notifies guests about backend reconnect
> - hypervisor sends hypervisor device state (e.g. queue addresses and
>   indices) to backend
>
> Reconnect isn't robust without such an extension.

I appreciate your all comments. It's truly helpful.
Do you have any plans to support such queue reset features in virtio spec?
Also, I haven't known following behavior.

> some guests crash if we never complete handling buffers,
> or networking breaks, etc ...

Does it mean that device needs to return buffers through used ring as
soon as possible?
If so, when backend switch is under heavy load, some guests could be
crashed?

>> Practically, it's not useful.
> One thing we do seem to lack is specifying an action
> taken on disconnect. It could be an action similar to r/werror.
> One extra useful option could be to exit qemu.

My implementation treats disconnection as link down.
And when backend is connected, link status will be changed.
So here is behavior of my patch.

- hypervisor detects queue backend disconnect.
- hypervisor sets link status down and notifies guest
- optionally, guest stops sending and receiving packets
  => As you mentioned above, this step may crash some guests.
- hypervisor detects (or requests if it's a client), backend reconnect
- hypervisor make sure backend supports features that the former backend
supports.
- hypervisor sends hypervisor device state (e.g. queue addresses and
  indices) to backend
- backend reads virtqueues, and recover correct indices.
  => Some backends needs to have this feature to reconnect correctly.
- hypervisor notifies guests about backend reconnect as link status up

>> This patch series adds following features.
>>  - QEMU can start before the backend.
> I don't see the point of this one.

Sorry for my bad English. Here is correct.
 - Guest can start before the backend.

Currently, guest will not start without vhost-user backend application
when vhost-user is specified.

>>  - QEMU or the backend can restart anytime.
>>    connectivity will be recovered automatically, when app starts again.
>>    (if QEMU is server, QEMU just wait reconnection)
>>    while lost connection, link status of virtio-net device is down,
>>    so virtio-net driver on the guest can know it
>>
>> To work like above, the patch introduces flags to specify features vhost-user
>> backend will support.
>>
>> Here are examples.
>> ('backend_features' is the new flags. Each bit of the flag represents
>> VIRTIO_NET_F_* in linux/virtio_net.h)
>>
>>     * QEMU is configured as vhost-user client.
>>      -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
>>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
>>      -device virtio-net-pci,netdev=net0 \
>>
>>     * QEMU is configured as vhost-user server.
>>      -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
>>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
>>      -device virtio-net-pci,netdev=net0 \
> This relies on management knowing the feature bitmap encoding of the
> backend, not sure how practical that is.

If we can assume which vhost-user backend application is used, we can
specify backend-features value like above.
Probably such an assumption is not so strange in practice.
And above option allow us to start VM before starting backend application.

I guess it's useful, but if you don't think, I could remove this feature.
In the case, still VM needs to start before backend.
And when backend is connected, virtio-net device will keep backend
features for future reconnection.
(When backend reconnects again, device will make sure that backend
supports features that the former backend supports.)

Regards,
Tetsuya

Re: [Qemu-devel] [PATCH v2 3/5] vhost-user: Shutdown vhost-user connection when wrong messages are passed

[Tetsuya Mukawa]

On 2015/06/22 16:54, Michael S. Tsirkin wrote:
> On Mon, Jun 22, 2015 at 12:50:46PM +0900, Tetsuya Mukawa wrote:
>> When wrong vhost-user message are passed, the connection should be shutdown.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> This silently changes the protocol semantics: previously
> unknown messages were ignored. We can't do this.
> See email titled "vhost-user: protocol extensions" which relies on this
> to detect protocol version.
>

Hi Michael,

Thanks for commenting.
Instead of disconnecting the backend, I will add warning message and
leave link status down.

Regards,
Tetsuya

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Michael S. Tsirkin]

On Tue, Jun 23, 2015 at 05:31:06PM +0900, Tetsuya Mukawa wrote:
> On 2015/06/22 17:14, Michael S. Tsirkin wrote:
> > On Mon, Jun 22, 2015 at 12:50:43PM +0900, Tetsuya Mukawa wrote:
> >> Hi guys,
> >>
> >> Here are patches to add feature to start QEMU without vhost-user backend.
> >> Currently, if we want to use vhost-user backend, the backend must start before
> >> QEMU.
> > Aren't you looking for something like xinetd?
> 
> It will help for restricting starting order, but not help for
> reconnection of vhost-user backend.

At this point, I suggest that we always connect at vm start.
With that, you can add an option to reset the VM
on backend disconnect.
So
	- detect backend disconnect
	- reset and stop (notifying management)
	- reconnect or detect backend reconnect
	- proceed with boot

As I tried to explain below, getting the full functionality
will require guest driver changes. They aren't hard to get
done, patches welcome.



> >> Also, if QEMU or the backend is closed unexpectedly, there is no way to
> >> recover without restarting both applications.
> > This was previously discussed:
> > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00585.html
> > It doesn't look like any of the issues have been addressed.
> >
> > In my opinion, the right way to handle this requires
> > guest cooperation.  For example, we can extend virtio 1 to allow a
> > single queue to reset.
> >
> > We then can do something like the following:
> > - hypervisor detects queue backend disconnect.
> > - hypervisor sets flag and notifies guest
> > - guest resets queue, optionally discarding queued packets
> > - hypervisor detects (or requests if it's a client), backend reconnect
> > - hypervisor detects guest queue reset
> > - hypervisor notifies guests about backend reconnect
> > - hypervisor sends hypervisor device state (e.g. queue addresses and
> >   indices) to backend
> >
> > Reconnect isn't robust without such an extension.
> 
> I appreciate your all comments. It's truly helpful.
> Do you have any plans to support such queue reset features in virtio spec?

Maybe, but I don't have the time to work on this :(
You are welcome to contribute this.

> Also, I haven't known following behavior.
> 
> > some guests crash if we never complete handling buffers,
> > or networking breaks, etc ...
> 
> Does it mean that device needs to return buffers through used ring as
> soon as possible?
> If so, when backend switch is under heavy load, some guests could be
> crashed?

Yes. Some guests crash when disk times out, too.

> >> Practically, it's not useful.
> > One thing we do seem to lack is specifying an action
> > taken on disconnect. It could be an action similar to r/werror.
> > One extra useful option could be to exit qemu.
> 
> My implementation treats disconnection as link down.
> And when backend is connected, link status will be changed.
> So here is behavior of my patch.
> 
> - hypervisor detects queue backend disconnect.
> - hypervisor sets link status down and notifies guest
> - optionally, guest stops sending and receiving packets
>   => As you mentioned above, this step may crash some guests.

It's not just that.
Under the virtio protocol, you can not, just by looking at the ring,
detect which buffers have been used.
Imagine:

	- guest adds buffer A
	- guest adds buffer B * 10000 - backend uses buffer B * 10000

buffer A is nowhere in the available ring since it has
since wrapped around many times.
So backend that crashes and re-connects simply can not recover
and complete buffer A.





> - hypervisor detects (or requests if it's a client), backend reconnect
> - hypervisor make sure backend supports features that the former backend
> supports.
> - hypervisor sends hypervisor device state (e.g. queue addresses and
>   indices) to backend
> - backend reads virtqueues, and recover correct indices.
>   => Some backends needs to have this feature to reconnect correctly.
> - hypervisor notifies guests about backend reconnect as link status up
> 
> >> This patch series adds following features.
> >>  - QEMU can start before the backend.
> > I don't see the point of this one.
> 
> Sorry for my bad English. Here is correct.
>  - Guest can start before the backend.
> 
> Currently, guest will not start without vhost-user backend application
> when vhost-user is specified.

E.g. with control VQ (which vhost user doesn't handle, but really
should, and I think it will in the future), guest will loop waiting for
the command to complete.

> >>  - QEMU or the backend can restart anytime.
> >>    connectivity will be recovered automatically, when app starts again.
> >>    (if QEMU is server, QEMU just wait reconnection)
> >>    while lost connection, link status of virtio-net device is down,
> >>    so virtio-net driver on the guest can know it
> >>
> >> To work like above, the patch introduces flags to specify features vhost-user
> >> backend will support.
> >>
> >> Here are examples.
> >> ('backend_features' is the new flags. Each bit of the flag represents
> >> VIRTIO_NET_F_* in linux/virtio_net.h)
> >>
> >>     * QEMU is configured as vhost-user client.
> >>      -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
> >>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
> >>      -device virtio-net-pci,netdev=net0 \
> >>
> >>     * QEMU is configured as vhost-user server.
> >>      -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
> >>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
> >>      -device virtio-net-pci,netdev=net0 \
> > This relies on management knowing the feature bitmap encoding of the
> > backend, not sure how practical that is.
> 
> If we can assume which vhost-user backend application is used, we can
> specify backend-features value like above.
> Probably such an assumption is not so strange in practice.
> And above option allow us to start VM before starting backend application.
> 
> I guess it's useful, but if you don't think, I could remove this feature.

To make it useful, management needs a way to find out what are the
values. We don't want it to deal with the virtio spec, do we?


> In the case, still VM needs to start before backend.
> And when backend is connected, virtio-net device will keep backend
> features for future reconnection.
> (When backend reconnects again, device will make sure that backend
> supports features that the former backend supports.)
> 
> Regards,
> Tetsuya

I think that
- reconnect requires guest support. let's add this to virtio
- when not connected, the cleanest thing is to block VM
  options to exit and reset might also make sense
- if you want to start guest before backend, I think
  we are looking at a spec extension, to tell guest
  not to access device since backend is not ready.
  we also need to solve two problems
	- how does management find out features supported by qemu?
	- how does management find out features supported by backend?

-- 
MST

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Tetsuya Mukawa]

On 2015/06/23 18:41, Michael S. Tsirkin wrote:
> On Tue, Jun 23, 2015 at 05:31:06PM +0900, Tetsuya Mukawa wrote:
>> On 2015/06/22 17:14, Michael S. Tsirkin wrote:
>>> On Mon, Jun 22, 2015 at 12:50:43PM +0900, Tetsuya Mukawa wrote:
>>>> Hi guys,
>>>>
>>>> Here are patches to add feature to start QEMU without vhost-user backend.
>>>> Currently, if we want to use vhost-user backend, the backend must start before
>>>> QEMU.
>>> Aren't you looking for something like xinetd?
>> It will help for restricting starting order, but not help for
>> reconnection of vhost-user backend.
> At this point, I suggest that we always connect at vm start.
> With that, you can add an option to reset the VM
> on backend disconnect.
> So
> 	- detect backend disconnect
> 	- reset and stop (notifying management)
> 	- reconnect or detect backend reconnect
> 	- proceed with boot
>
> As I tried to explain below, getting the full functionality
> will require guest driver changes. They aren't hard to get
> done, patches welcome.
>

Could you please let me know your thinking about using
DEVICE_NEEDS_RESET for vhost-user reconnection?
If it's works, I will try to submit it.

>
>>>> Also, if QEMU or the backend is closed unexpectedly, there is no way to
>>>> recover without restarting both applications.
>>> This was previously discussed:
>>> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00585.html
>>> It doesn't look like any of the issues have been addressed.
>>>
>>> In my opinion, the right way to handle this requires
>>> guest cooperation.  For example, we can extend virtio 1 to allow a
>>> single queue to reset.
>>>
>>> We then can do something like the following:
>>> - hypervisor detects queue backend disconnect.
>>> - hypervisor sets flag and notifies guest
>>> - guest resets queue, optionally discarding queued packets
>>> - hypervisor detects (or requests if it's a client), backend reconnect
>>> - hypervisor detects guest queue reset
>>> - hypervisor notifies guests about backend reconnect
>>> - hypervisor sends hypervisor device state (e.g. queue addresses and
>>>   indices) to backend
>>>
>>> Reconnect isn't robust without such an extension.
>> I appreciate your all comments. It's truly helpful.
>> Do you have any plans to support such queue reset features in virtio spec?
> Maybe, but I don't have the time to work on this :(
> You are welcome to contribute this.

To do that, I need to talk around my boss to join OASIS :'(
Probably it's not so easy, so I will just wait, if we need to change
spec itself.

>> Also, I haven't known following behavior.
>>
>>> some guests crash if we never complete handling buffers,
>>> or networking breaks, etc ...
>> Does it mean that device needs to return buffers through used ring as
>> soon as possible?
>> If so, when backend switch is under heavy load, some guests could be
>> crashed?
> Yes. Some guests crash when disk times out, too.

It's bad news. I haven't assumed the case while writing patches.

>>>> Practically, it's not useful.
>>> One thing we do seem to lack is specifying an action
>>> taken on disconnect. It could be an action similar to r/werror.
>>> One extra useful option could be to exit qemu.
>> My implementation treats disconnection as link down.
>> And when backend is connected, link status will be changed.
>> So here is behavior of my patch.
>>
>> - hypervisor detects queue backend disconnect.
>> - hypervisor sets link status down and notifies guest
>> - optionally, guest stops sending and receiving packets
>>   => As you mentioned above, this step may crash some guests.
> It's not just that.
> Under the virtio protocol, you can not, just by looking at the ring,
> detect which buffers have been used.
> Imagine:
>
> 	- guest adds buffer A
> 	- guest adds buffer B * 10000 - backend uses buffer B * 10000
>
> buffer A is nowhere in the available ring since it has
> since wrapped around many times.
> So backend that crashes and re-connects simply can not recover
> and complete buffer A.
>

Could I make sure that I understand the issue correctly?
I guess that an one of cause of the above issue is that the guest
doesn't check used_ring before filling avail ring to make sure that
buffers overwritten by new filling buffers have already returned through
used_ring.
If the guest checks used_ring, probably performance will be poor, but
there are no buffer loss.

Also I guess the above issue might be happen if backend is too slow,
because the guest will overwrite avail_ring without checking.
Is this correct?

>
>
>
>> - hypervisor detects (or requests if it's a client), backend reconnect
>> - hypervisor make sure backend supports features that the former backend
>> supports.
>> - hypervisor sends hypervisor device state (e.g. queue addresses and
>>   indices) to backend
>> - backend reads virtqueues, and recover correct indices.
>>   => Some backends needs to have this feature to reconnect correctly.
>> - hypervisor notifies guests about backend reconnect as link status up
>>
>>>> This patch series adds following features.
>>>>  - QEMU can start before the backend.
>>> I don't see the point of this one.
>> Sorry for my bad English. Here is correct.
>>  - Guest can start before the backend.
>>
>> Currently, guest will not start without vhost-user backend application
>> when vhost-user is specified.
> E.g. with control VQ (which vhost user doesn't handle, but really
> should, and I think it will in the future), guest will loop waiting for
> the command to complete.

I agree.
I guess control queue should work fine even when link status is down,
but my patches doesn't care about it.

>>>>  - QEMU or the backend can restart anytime.
>>>>    connectivity will be recovered automatically, when app starts again.
>>>>    (if QEMU is server, QEMU just wait reconnection)
>>>>    while lost connection, link status of virtio-net device is down,
>>>>    so virtio-net driver on the guest can know it
>>>>
>>>> To work like above, the patch introduces flags to specify features vhost-user
>>>> backend will support.
>>>>
>>>> Here are examples.
>>>> ('backend_features' is the new flags. Each bit of the flag represents
>>>> VIRTIO_NET_F_* in linux/virtio_net.h)
>>>>
>>>>     * QEMU is configured as vhost-user client.
>>>>      -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
>>>>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
>>>>      -device virtio-net-pci,netdev=net0 \
>>>>
>>>>     * QEMU is configured as vhost-user server.
>>>>      -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
>>>>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
>>>>      -device virtio-net-pci,netdev=net0 \
>>> This relies on management knowing the feature bitmap encoding of the
>>> backend, not sure how practical that is.
>> If we can assume which vhost-user backend application is used, we can
>> specify backend-features value like above.
>> Probably such an assumption is not so strange in practice.
>> And above option allow us to start VM before starting backend application.
>>
>> I guess it's useful, but if you don't think, I could remove this feature.
> To make it useful, management needs a way to find out what are the
> values. We don't want it to deal with the virtio spec, do we?

I agree.

>
>> In the case, still VM needs to start before backend.
>> And when backend is connected, virtio-net device will keep backend
>> features for future reconnection.
>> (When backend reconnects again, device will make sure that backend
>> supports features that the former backend supports.)
>>
>> Regards,
>> Tetsuya
> I think that
> - reconnect requires guest support. let's add this to virtio
> - when not connected, the cleanest thing is to block VM
>   options to exit and reset might also make sense
> - if you want to start guest before backend, I think
>   we are looking at a spec extension, to tell guest
>   not to access device since backend is not ready.
>   we also need to solve two problems
> 	- how does management find out features supported by qemu?
> 	- how does management find out features supported by backend?

Thanks for clarifying. I understand the correct steps.

Regards,
Tetsuya

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Michael S. Tsirkin]

On Wed, Jun 24, 2015 at 02:46:30PM +0900, Tetsuya Mukawa wrote:
> On 2015/06/23 18:41, Michael S. Tsirkin wrote:
> > On Tue, Jun 23, 2015 at 05:31:06PM +0900, Tetsuya Mukawa wrote:
> >> On 2015/06/22 17:14, Michael S. Tsirkin wrote:
> >>> On Mon, Jun 22, 2015 at 12:50:43PM +0900, Tetsuya Mukawa wrote:
> >>>> Hi guys,
> >>>>
> >>>> Here are patches to add feature to start QEMU without vhost-user backend.
> >>>> Currently, if we want to use vhost-user backend, the backend must start before
> >>>> QEMU.
> >>> Aren't you looking for something like xinetd?
> >> It will help for restricting starting order, but not help for
> >> reconnection of vhost-user backend.
> > At this point, I suggest that we always connect at vm start.
> > With that, you can add an option to reset the VM
> > on backend disconnect.
> > So
> > 	- detect backend disconnect
> > 	- reset and stop (notifying management)
> > 	- reconnect or detect backend reconnect
> > 	- proceed with boot
> >
> > As I tried to explain below, getting the full functionality
> > will require guest driver changes. They aren't hard to get
> > done, patches welcome.
> >
> 
> Could you please let me know your thinking about using
> DEVICE_NEEDS_RESET for vhost-user reconnection?
> If it's works, I will try to submit it.

DEVICE_NEEDS_RESET is hard to handle correctly in guest:
you need to reconfigure a bunch of state,
so far no one wrote the necessary support.


> >
> >>>> Also, if QEMU or the backend is closed unexpectedly, there is no way to
> >>>> recover without restarting both applications.
> >>> This was previously discussed:
> >>> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00585.html
> >>> It doesn't look like any of the issues have been addressed.
> >>>
> >>> In my opinion, the right way to handle this requires
> >>> guest cooperation.  For example, we can extend virtio 1 to allow a
> >>> single queue to reset.
> >>>
> >>> We then can do something like the following:
> >>> - hypervisor detects queue backend disconnect.
> >>> - hypervisor sets flag and notifies guest
> >>> - guest resets queue, optionally discarding queued packets
> >>> - hypervisor detects (or requests if it's a client), backend reconnect
> >>> - hypervisor detects guest queue reset
> >>> - hypervisor notifies guests about backend reconnect
> >>> - hypervisor sends hypervisor device state (e.g. queue addresses and
> >>>   indices) to backend
> >>>
> >>> Reconnect isn't robust without such an extension.
> >> I appreciate your all comments. It's truly helpful.
> >> Do you have any plans to support such queue reset features in virtio spec?
> > Maybe, but I don't have the time to work on this :(
> > You are welcome to contribute this.
> 
> To do that, I need to talk around my boss to join OASIS :'(

That's not true at all. Non members can submit feedback.
You do need permission from your employer, but
no need to join and pay fees if you don't want to.
See
https://www.oasis-open.org/committees/comments/index.php?wg_abbrev=virtio
for the details.

> Probably it's not so easy, so I will just wait, if we need to change
> spec itself.

The QEMU patch that allows handling backend disconnect similar to
disk errors can be implemented without guest/spec changes
let me know if you need more explanation.


> >> Also, I haven't known following behavior.
> >>
> >>> some guests crash if we never complete handling buffers,
> >>> or networking breaks, etc ...
> >> Does it mean that device needs to return buffers through used ring as
> >> soon as possible?
> >> If so, when backend switch is under heavy load, some guests could be
> >> crashed?
> > Yes. Some guests crash when disk times out, too.
> 
> It's bad news. I haven't assumed the case while writing patches.
> 
> >>>> Practically, it's not useful.
> >>> One thing we do seem to lack is specifying an action
> >>> taken on disconnect. It could be an action similar to r/werror.
> >>> One extra useful option could be to exit qemu.
> >> My implementation treats disconnection as link down.
> >> And when backend is connected, link status will be changed.
> >> So here is behavior of my patch.
> >>
> >> - hypervisor detects queue backend disconnect.
> >> - hypervisor sets link status down and notifies guest
> >> - optionally, guest stops sending and receiving packets
> >>   => As you mentioned above, this step may crash some guests.
> > It's not just that.
> > Under the virtio protocol, you can not, just by looking at the ring,
> > detect which buffers have been used.
> > Imagine:
> >
> > 	- guest adds buffer A
> > 	- guest adds buffer B * 10000 - backend uses buffer B * 10000
> >
> > buffer A is nowhere in the available ring since it has
> > since wrapped around many times.
> > So backend that crashes and re-connects simply can not recover
> > and complete buffer A.
> >
> 
> Could I make sure that I understand the issue correctly?
> I guess that an one of cause of the above issue is that the guest
> doesn't check used_ring before filling avail ring to make sure that
> buffers overwritten by new filling buffers have already returned through
> used_ring.
> If the guest checks used_ring, probably performance will be poor, but
> there are no buffer loss.
> 
> Also I guess the above issue might be happen if backend is too slow,
> because the guest will overwrite avail_ring without checking.
> Is this correct?

No - it happens with a correct guest too, because buffers
can be consumed out of order.


> >
> >
> >
> >> - hypervisor detects (or requests if it's a client), backend reconnect
> >> - hypervisor make sure backend supports features that the former backend
> >> supports.
> >> - hypervisor sends hypervisor device state (e.g. queue addresses and
> >>   indices) to backend
> >> - backend reads virtqueues, and recover correct indices.
> >>   => Some backends needs to have this feature to reconnect correctly.
> >> - hypervisor notifies guests about backend reconnect as link status up
> >>
> >>>> This patch series adds following features.
> >>>>  - QEMU can start before the backend.
> >>> I don't see the point of this one.
> >> Sorry for my bad English. Here is correct.
> >>  - Guest can start before the backend.
> >>
> >> Currently, guest will not start without vhost-user backend application
> >> when vhost-user is specified.
> > E.g. with control VQ (which vhost user doesn't handle, but really
> > should, and I think it will in the future), guest will loop waiting for
> > the command to complete.
> 
> I agree.
> I guess control queue should work fine even when link status is down,
> but my patches doesn't care about it.
> 
> >>>>  - QEMU or the backend can restart anytime.
> >>>>    connectivity will be recovered automatically, when app starts again.
> >>>>    (if QEMU is server, QEMU just wait reconnection)
> >>>>    while lost connection, link status of virtio-net device is down,
> >>>>    so virtio-net driver on the guest can know it
> >>>>
> >>>> To work like above, the patch introduces flags to specify features vhost-user
> >>>> backend will support.
> >>>>
> >>>> Here are examples.
> >>>> ('backend_features' is the new flags. Each bit of the flag represents
> >>>> VIRTIO_NET_F_* in linux/virtio_net.h)
> >>>>
> >>>>     * QEMU is configured as vhost-user client.
> >>>>      -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
> >>>>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
> >>>>      -device virtio-net-pci,netdev=net0 \
> >>>>
> >>>>     * QEMU is configured as vhost-user server.
> >>>>      -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
> >>>>      -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend-features=0x68000 \
> >>>>      -device virtio-net-pci,netdev=net0 \
> >>> This relies on management knowing the feature bitmap encoding of the
> >>> backend, not sure how practical that is.
> >> If we can assume which vhost-user backend application is used, we can
> >> specify backend-features value like above.
> >> Probably such an assumption is not so strange in practice.
> >> And above option allow us to start VM before starting backend application.
> >>
> >> I guess it's useful, but if you don't think, I could remove this feature.
> > To make it useful, management needs a way to find out what are the
> > values. We don't want it to deal with the virtio spec, do we?
> 
> I agree.
> 
> >
> >> In the case, still VM needs to start before backend.
> >> And when backend is connected, virtio-net device will keep backend
> >> features for future reconnection.
> >> (When backend reconnects again, device will make sure that backend
> >> supports features that the former backend supports.)
> >>
> >> Regards,
> >> Tetsuya
> > I think that
> > - reconnect requires guest support. let's add this to virtio
> > - when not connected, the cleanest thing is to block VM
> >   options to exit and reset might also make sense
> > - if you want to start guest before backend, I think
> >   we are looking at a spec extension, to tell guest
> >   not to access device since backend is not ready.
> >   we also need to solve two problems
> > 	- how does management find out features supported by qemu?
> > 	- how does management find out features supported by backend?
> 
> Thanks for clarifying. I understand the correct steps.
> 
> Regards,
> Tetsuya



-- 
MST

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Tetsuya Mukawa]

On 2015/06/24 14:57, Michael S. Tsirkin wrote:
>>>>>> Also, if QEMU or the backend is closed unexpectedly, there is no way to
>>>>>> recover without restarting both applications.
>>>>> This was previously discussed:
>>>>> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00585.html
>>>>> It doesn't look like any of the issues have been addressed.
>>>>>
>>>>> In my opinion, the right way to handle this requires
>>>>> guest cooperation.  For example, we can extend virtio 1 to allow a
>>>>> single queue to reset.
>>>>>
>>>>> We then can do something like the following:
>>>>> - hypervisor detects queue backend disconnect.
>>>>> - hypervisor sets flag and notifies guest
>>>>> - guest resets queue, optionally discarding queued packets
>>>>> - hypervisor detects (or requests if it's a client), backend reconnect
>>>>> - hypervisor detects guest queue reset
>>>>> - hypervisor notifies guests about backend reconnect
>>>>> - hypervisor sends hypervisor device state (e.g. queue addresses and
>>>>>   indices) to backend
>>>>>
>>>>> Reconnect isn't robust without such an extension.
>>>> I appreciate your all comments. It's truly helpful.
>>>> Do you have any plans to support such queue reset features in virtio spec?
>>> Maybe, but I don't have the time to work on this :(
>>> You are welcome to contribute this.
>> To do that, I need to talk around my boss to join OASIS :'(
> That's not true at all. Non members can submit feedback.
> You do need permission from your employer, but
> no need to join and pay fees if you don't want to.
> See
> https://www.oasis-open.org/committees/comments/index.php?wg_abbrev=virtio
> for the details.

Thanks so much for correcting.

>> Probably it's not so easy, so I will just wait, if we need to change
>> spec itself.
> The QEMU patch that allows handling backend disconnect similar to
> disk errors can be implemented without guest/spec changes
> let me know if you need more explanation.

Could you please describe it more?
I want to try to implement it.

Regards,
Tetsuya

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Michael S. Tsirkin]

On Wed, Jun 24, 2015 at 07:57:45AM +0200, Michael S. Tsirkin wrote:
> On Wed, Jun 24, 2015 at 02:46:30PM +0900, Tetsuya Mukawa wrote:
> > On 2015/06/23 18:41, Michael S. Tsirkin wrote:
> > > On Tue, Jun 23, 2015 at 05:31:06PM +0900, Tetsuya Mukawa wrote:
> > >> On 2015/06/22 17:14, Michael S. Tsirkin wrote:
> > >>> On Mon, Jun 22, 2015 at 12:50:43PM +0900, Tetsuya Mukawa wrote:
> > >>>> Hi guys,
> > >>>>
> > >>>> Here are patches to add feature to start QEMU without vhost-user backend.
> > >>>> Currently, if we want to use vhost-user backend, the backend must start before
> > >>>> QEMU.
> > >>> Aren't you looking for something like xinetd?
> > >> It will help for restricting starting order, but not help for
> > >> reconnection of vhost-user backend.
> > > At this point, I suggest that we always connect at vm start.
> > > With that, you can add an option to reset the VM
> > > on backend disconnect.
> > > So
> > > 	- detect backend disconnect
> > > 	- reset and stop (notifying management)
> > > 	- reconnect or detect backend reconnect
> > > 	- proceed with boot
> > >
> > > As I tried to explain below, getting the full functionality
> > > will require guest driver changes. They aren't hard to get
> > > done, patches welcome.
> > >
> > 
> > Could you please let me know your thinking about using
> > DEVICE_NEEDS_RESET for vhost-user reconnection?
> > If it's works, I will try to submit it.
> 
> DEVICE_NEEDS_RESET is hard to handle correctly in guest:
> you need to reconfigure a bunch of state,
> so far no one wrote the necessary support.


But yes, if you write the guest code, I think
it can work.

We probably want a feature bit so host can know guest can
handle reconnections, but that's a minor detail, it
would be easy to add afterwards.

-- 
MST

Re: [Qemu-devel] [PATCH v2 0/5] Add feature to start QEMU without vhost-user backend

[Tetsuya Mukawa]

On 2015/07/19 21:48, Michael S. Tsirkin wrote:
> On Wed, Jun 24, 2015 at 07:57:45AM +0200, Michael S. Tsirkin wrote:
>> On Wed, Jun 24, 2015 at 02:46:30PM +0900, Tetsuya Mukawa wrote:
>>> On 2015/06/23 18:41, Michael S. Tsirkin wrote:
>>>> On Tue, Jun 23, 2015 at 05:31:06PM +0900, Tetsuya Mukawa wrote:
>>>>> On 2015/06/22 17:14, Michael S. Tsirkin wrote:
>>>>>> On Mon, Jun 22, 2015 at 12:50:43PM +0900, Tetsuya Mukawa wrote:
>>>>>>> Hi guys,
>>>>>>>
>>>>>>> Here are patches to add feature to start QEMU without vhost-user backend.
>>>>>>> Currently, if we want to use vhost-user backend, the backend must start before
>>>>>>> QEMU.
>>>>>> Aren't you looking for something like xinetd?
>>>>> It will help for restricting starting order, but not help for
>>>>> reconnection of vhost-user backend.
>>>> At this point, I suggest that we always connect at vm start.
>>>> With that, you can add an option to reset the VM
>>>> on backend disconnect.
>>>> So
>>>> 	- detect backend disconnect
>>>> 	- reset and stop (notifying management)
>>>> 	- reconnect or detect backend reconnect
>>>> 	- proceed with boot
>>>>
>>>> As I tried to explain below, getting the full functionality
>>>> will require guest driver changes. They aren't hard to get
>>>> done, patches welcome.
>>>>
>>> Could you please let me know your thinking about using
>>> DEVICE_NEEDS_RESET for vhost-user reconnection?
>>> If it's works, I will try to submit it.
>> DEVICE_NEEDS_RESET is hard to handle correctly in guest:
>> you need to reconfigure a bunch of state,
>> so far no one wrote the necessary support.
> But yes, if you write the guest code, I think
> it can work.
>
> We probably want a feature bit so host can know guest can
> handle reconnections, but that's a minor detail, it
> would be easy to add afterwards.
>

Hi Michael,

I appreciate for your suggestion.
I will check the virtio-net driver of linux and the virtio-net device of
QEMU to know how difficult to implement DEVICE_NEEDS_RESET.

Regards,
Tetsuya

Re: [Qemu-devel] [PATCH v2 3/5] vhost-user: Shutdown vhost-user connection when wrong messages are passed

[Marc-André Lureau]

Hi

On Mon, Jun 22, 2015 at 5:50 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> When wrong vhost-user message are passed, the connection should be shutdown.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  hw/virtio/vhost-user.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index d6f2163..2215c39 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -183,6 +183,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>  static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
...
> +close:
> +    qemu_chr_disconnect(chr);
> +    return -1;
>  }

Many vhost_call() are followed by an assert(). You'll want to change
this if you want to make vhost-user robust against disconnect.

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 4/5] vhost-user: Enable 'nowait' and 'reconnect' option

[Marc-André Lureau]

Hi

On Mon, Jun 22, 2015 at 5:50 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> The patch enables 'nowait' option for server mode, and 'reconnect'
> option for client mode.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  net/vhost-user.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 8b7749a..58cd5dc 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -26,6 +26,8 @@ typedef struct VhostUserChardevProps {
>      bool is_socket;
>      bool is_unix;
>      bool is_server;
> +    bool is_nowait;
> +    bool is_reconnect;
>  } VhostUserChardevProps;
>
>  VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
> @@ -186,6 +188,10 @@ static int net_vhost_chardev_opts(void *opaque,
>          props->is_unix = true;
>      } else if (strcmp(name, "server") == 0) {
>          props->is_server = true;
> +    } else if ((strcmp(name, "wait") == 0) && (strcmp(value, "off")) == 0) {
> +        props->is_nowait = true;
> +    } else if (strcmp(name, "reconnect") == 0) {
> +        props->is_reconnect = true;
>      } else {
>          error_setg(errp,
>                     "vhost-user does not support a chardev with option %s=%s",

I don't see the point in saving all those options... is_server could
be removed too, in a separate patch


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 5/5] vhost-user: Add new option to specify vhost-user backend supports

[Marc-André Lureau]

Hi

On Mon, Jun 22, 2015 at 5:50 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index a6dcc79..bbfc336 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -901,7 +901,8 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
>  }
>
>  int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> -                   VhostBackendType backend_type)
> +                   VhostBackendType backend_type,
> +                   unsigned long long backend_features)
>  {
>      uint64_t features;
>      int i, r;

What's the point in passing backend_features here ? It's a leftover?



-- 
Marc-André Lureau