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

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

[Tetsuya Mukawa]

Hi guys,

Here is RFC patch 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 backend_features flag.
Here are examples.

 -chardev socket,id=chr0,path=/tmp/sock0,reconnect=3 \
 -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_features=0x68000 \
 -device virtio-net-pci,netdev=net0 \
 \
 -chardev socket,id=chr1,path=/tmp/sock1,server,nowait \
 -netdev vhost-user,id=net1,chardev=chr1,vhostforce,backend_features=0x68000 \
 -device virtio-net-pci,netdev=net1 \

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


Tetsuya Mukawa (4):
  vhost-user: Add ability to know vhost-user backend disconnection
  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             |  9 +++++-
 hw/net/virtio-net.c            | 24 ++++++++++++++++
 hw/scsi/vhost-scsi.c           |  2 +-
 hw/virtio/vhost-user.c         | 26 +++++++++++++-----
 hw/virtio/vhost.c              |  8 ++++--
 include/hw/virtio/vhost.h      |  5 +++-
 include/hw/virtio/virtio-net.h |  2 ++
 include/net/net.h              |  6 ++++
 include/net/vhost_net.h        |  2 ++
 include/sysemu/char.h          |  7 +++++
 net/net.c                      | 18 ++++++++++++
 net/tap.c                      |  5 +++-
 net/vhost-user.c               | 62 ++++++++++++++++++++++++++++++++++++++++--
 qapi-schema.json               | 10 +++++--
 qemu-char.c                    | 15 ++++++++++
 qemu-options.hx                |  3 +-
 16 files changed, 186 insertions(+), 18 deletions(-)

-- 
2.1.4

[Qemu-devel] [RFC PATCH 1/4] 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 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 1d86a2b..55c05a5 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 {
@@ -113,12 +114,23 @@ 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];
+
+    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->chr->label);
@@ -126,6 +138,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->chr->label);
         break;
     }
-- 
2.1.4

[Qemu-devel] [RFC PATCH 2/4] 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 | 17 ++++++++++-------
 include/sysemu/char.h  |  7 +++++++
 qemu-char.c            | 15 +++++++++++++++
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e7ab829..4d7e3ba 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;
@@ -237,7 +238,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);
@@ -281,7 +282,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;
     }
 
@@ -297,32 +298,34 @@ 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;
             }
             memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
             break;
         default:
             error_report("Received unexpected msg type.");
-            return -1;
-            break;
         }
     }
 
     return 0;
+
+close:
+    qemu_chr_shutdown(chr, SHUT_RDWR);
+    return -1;
 }
 
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 832b7fe..d944ded 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_shutdown)(struct CharDriverState *chr, int how);
     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_shutdown:
+ *
+ * Shutdown a fd of character backend.
+ */
+void qemu_chr_shutdown(CharDriverState *chr, int how);
 
 /**
  * @qemu_chr_delete:
diff --git a/qemu-char.c b/qemu-char.c
index d0c1564..2b28808 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2839,6 +2839,13 @@ static void tcp_chr_disconnect(CharDriverState *chr)
     }
 }
 
+static void tcp_chr_shutdown(CharDriverState *chr, int how)
+{
+    TCPCharDriver *s = chr->opaque;
+
+    shutdown(s->fd, how);
+}
+
 static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -3836,6 +3843,13 @@ void qemu_chr_fe_release(CharDriverState *s)
     s->avail_connections++;
 }
 
+void qemu_chr_shutdown(CharDriverState *chr, int how)
+{
+    if (chr->chr_shutdown) {
+        chr->chr_shutdown(chr, how);
+    }
+}
+
 void qemu_chr_delete(CharDriverState *chr)
 {
     QTAILQ_REMOVE(&chardevs, chr, next);
@@ -4154,6 +4168,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_shutdown = tcp_chr_shutdown;
     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] [RFC PATCH 3/4] 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 55c05a5..d31fc41 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)
@@ -178,6 +180,10 @@ static int net_vhost_chardev_opts(const char *name, const char *value,
         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_report("vhost-user does not support a chardev"
                      " with the following option:\n %s = %s",
-- 
2.1.4

[Qemu-devel] [RFC PATCH 4/4] 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             |  9 ++++++++-
 hw/net/virtio-net.c            | 24 ++++++++++++++++++++++++
 hw/scsi/vhost-scsi.c           |  2 +-
 hw/virtio/vhost-user.c         |  9 +++++++++
 hw/virtio/vhost.c              |  8 ++++++--
 include/hw/virtio/vhost.h      |  5 ++++-
 include/hw/virtio/virtio-net.h |  2 ++
 include/net/net.h              |  6 ++++++
 include/net/vhost_net.h        |  2 ++
 net/net.c                      | 18 ++++++++++++++++++
 net/tap.c                      |  5 ++++-
 net/vhost-user.c               | 42 ++++++++++++++++++++++++++++++++++++++++--
 qapi-schema.json               | 10 ++++++++--
 qemu-options.hx                |  3 ++-
 14 files changed, 134 insertions(+), 11 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 47f8b89..f974d09 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -158,8 +158,15 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
     net->dev.nvqs = 2;
     net->dev.vqs = net->vqs;
 
+    if (options->has_backend_features) {
+        net->dev.has_backend_features = options->has_backend_features;
+        net->dev.backend_features = options->backend_features;
+    }
+
     r = vhost_dev_init(&net->dev, options->opaque,
-                       options->backend_type, options->force);
+                       options->backend_type, options->force,
+                       options->has_backend_features,
+                       options->backend_features);
     if (r < 0) {
         goto fail;
     }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3af6faf..ebe5422 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -366,6 +366,28 @@ static int peer_has_ufo(VirtIONet *n)
     return n->has_ufo;
 }
 
+static int peer_has_backend_features(VirtIONet *n)
+{
+    if (!peer_has_vnet_hdr(n))
+        return 0;
+
+    n->has_backend_features =
+            qemu_has_backend_features(qemu_get_queue(n->nic)->peer);
+
+    return n->has_backend_features;
+}
+
+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 i;
@@ -463,6 +485,8 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
     }
 
     if (!get_vhost_net(nc->peer)) {
+        if (peer_has_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 335f442..4998a95 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, true);
+                         VHOST_BACKEND_TYPE_KERNEL, true, false, 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 4d7e3ba..f0dcb97 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -307,6 +307,15 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
                 error_report("Received bad msg size.");
                 goto close;
             }
+            if (dev->has_backend_features) {
+                if (dev->backend_features != (dev->backend_features & msg.u64)) {
+                    error_report("Lack of backend features. "
+                                 "Expected 0x%llx, but receives 0x%lx",
+                                 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 54851b7..20cb116 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -811,7 +811,9 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
 }
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-                   VhostBackendType backend_type, bool force)
+                   VhostBackendType backend_type, bool force,
+                   bool has_backend_features,
+                   unsigned long long backend_features)
 {
     uint64_t features;
     int i, r;
@@ -833,7 +835,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 
     r = hdev->vhost_ops->vhost_call(hdev, VHOST_GET_FEATURES, &features);
     if (r < 0) {
-        goto fail;
+        if (!has_backend_features)
+            goto fail;
+        features = backend_features;
     }
 
     for (i = 0; i < hdev->nvqs; ++i) {
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 8f04888..60306cb 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -41,6 +41,7 @@ struct vhost_dev {
     unsigned long long features;
     unsigned long long acked_features;
     unsigned long long backend_features;
+    bool has_backend_features;
     bool started;
     bool log_enabled;
     vhost_log_chunk_t *log;
@@ -55,7 +56,9 @@ struct vhost_dev {
 };
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-                   VhostBackendType backend_type, bool force);
+                   VhostBackendType backend_type, bool force,
+                   bool has_backend_features,
+                   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 e0dbb41..528da28 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -70,6 +70,8 @@ typedef struct VirtIONet {
     size_t guest_hdr_len;
     uint32_t host_features;
     uint8_t has_ufo;
+    uint8_t has_backend_features;
+    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 e66ca03..e2eab3e 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -55,6 +55,8 @@ typedef bool (HasVnetHdrLen)(NetClientState *, int);
 typedef void (UsingVnetHdr)(NetClientState *, bool);
 typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
 typedef void (SetVnetHdrLen)(NetClientState *, int);
+typedef bool (HasBackendFeatures)(NetClientState *);
+typedef unsigned long long (BackendFeatures)(NetClientState *);
 
 typedef struct NetClientInfo {
     NetClientOptionsKind type;
@@ -73,6 +75,8 @@ typedef struct NetClientInfo {
     UsingVnetHdr *using_vnet_hdr;
     SetOffload *set_offload;
     SetVnetHdrLen *set_vnet_hdr_len;
+    HasBackendFeatures *has_backend_features;
+    BackendFeatures *backend_features;
 } NetClientInfo;
 
 struct NetClientState {
@@ -136,6 +140,8 @@ 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);
+bool qemu_has_backend_features(NetClientState *nc);
+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 b1c18a3..b4ca8e8 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -12,6 +12,8 @@ typedef struct VhostNetOptions {
     NetClientState *net_backend;
     void *opaque;
     bool force;
+    bool has_backend_features;
+    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 7427f6a..85e41d0 100644
--- a/net/net.c
+++ b/net/net.c
@@ -459,6 +459,24 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
     nc->info->set_vnet_hdr_len(nc, len);
 }
 
+bool qemu_has_backend_features(NetClientState *nc)
+{
+    if (!nc || !nc->info->has_backend_features) {
+        return false;
+    }
+
+    return nc->info->has_backend_features(nc);
+}
+
+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 968df46..6d25170 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -636,12 +636,15 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
     }
 
     if (tap->has_vhost ? tap->vhost :
-        vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
+        vhostfdname || tap->has_backend_features ||
+                    (tap->has_vhostforce && tap->vhostforce)) {
         VhostNetOptions options;
 
         options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
         options.net_backend = &s->nc;
         options.force = tap->has_vhostforce && tap->vhostforce;
+        options.has_backend_features = tap->has_backend_features;
+        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 d31fc41..2705393 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -19,6 +19,8 @@ typedef struct VhostUserState {
     NetClientState nc;
     CharDriverState *chr;
     VHostNetState *vhost_net;
+    bool has_backend_features;
+    unsigned long long backend_features;
     int watch;
 } VhostUserState;
 
@@ -54,6 +56,7 @@ static int vhost_user_start(VhostUserState *s)
     options.net_backend = &s->nc;
     options.opaque = s->chr;
     options.force = true;
+    options.backend_features = s->backend_features;
 
     s->vhost_net = vhost_net_init(&options);
 
@@ -91,12 +94,32 @@ static bool vhost_user_has_ufo(NetClientState *nc)
     return true;
 }
 
+static bool vhost_user_has_backend_features(NetClientState *nc)
+{
+    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+
+    VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+
+    return s->has_backend_features;
+}
+
+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,
+        .has_backend_features = vhost_user_has_backend_features,
+        .backend_features = vhost_user_backend_features,
 };
 
 static void net_vhost_link_down(VhostUserState *s, bool link_down)
@@ -148,7 +171,9 @@ 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)
+                               const char *name, CharDriverState *chr,
+                               bool has_backend_features,
+                               unsigned long long backend_features)
 {
     NetClientState *nc;
     VhostUserState *s;
@@ -163,6 +188,8 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
     /* We don't provide a receive callback */
     s->nc.receive_disabled = 1;
     s->chr = chr;
+    s->has_backend_features = has_backend_features;
+    s->backend_features = backend_features;
 
     qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
 
@@ -247,6 +274,8 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
 {
     const NetdevVhostUserOptions *vhost_user_opts;
     CharDriverState *chr;
+    bool has_backend_features;;
+    unsigned long long backend_features;
 
     assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
     vhost_user_opts = opts->vhost_user;
@@ -263,6 +292,15 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
         return -1;
     }
 
+    /* backend features */
+    if (vhost_user_opts->has_backend_features) {
+        has_backend_features = true;
+        backend_features = vhost_user_opts->backend_features;
+    } else {
+        has_backend_features = false;
+        backend_features = 0;
+    }
 
-    return net_vhost_user_init(peer, "vhost_user", name, chr);
+    return net_vhost_user_init(peer, "vhost_user", name, chr,
+                                has_backend_features, backend_features);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index f97ffa1..bab4a74 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2243,6 +2243,8 @@
 #
 # @queues: #optional number of queues to be created for multiqueue capable tap
 #
+# @backend_features: #optional feature flag to support vhost user backend
+#
 # Since 1.2
 ##
 { 'struct': 'NetdevTapOptions',
@@ -2259,7 +2261,8 @@
     '*vhostfd':    'str',
     '*vhostfds':   'str',
     '*vhostforce': 'bool',
-    '*queues':     'uint32'} }
+    '*queues':     'uint32',
+    '*backend_features':'uint64'} }
 
 ##
 # @NetdevSocketOptions
@@ -2444,12 +2447,15 @@
 #
 # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
 #
+# @backend_features: #optional feature flag to support vhost user backend (default: 0).
+#
 # Since 2.1
 ##
 { 'struct': 'NetdevVhostUserOptions',
   'data': {
     'chardev':        'str',
-    '*vhostforce':    'bool' } }
+    '*vhostforce':    'bool',
+    '*backend_features':    'uint64' } }
 
 ##
 # @NetClientOptions
diff --git a/qemu-options.hx b/qemu-options.hx
index ec356f6..3ad3486 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1466,7 +1466,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "-net tap[,vlan=n][,name=str],ifname=name\n"
     "                connect the host TAP network interface to VLAN 'n'\n"
 #else
-    "-net tap[,vlan=n][,name=str][,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
+    "-net tap[,vlan=n][,name=str][,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n][,backend_features=n]\n"
     "                connect the host TAP network interface to VLAN 'n'\n"
     "                use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
     "                to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
@@ -1486,6 +1486,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "                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"
     "-net bridge[,vlan=n][,name=str][,br=bridge][,helper=helper]\n"
     "                connects a host TAP network interface to a host bridge device 'br'\n"
     "                (default=" DEFAULT_BRIDGE_INTERFACE ") using the program 'helper'\n"
-- 
2.1.4

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2849 bytes --]

On 05/25/2015 01:28 AM, Tetsuya Mukawa wrote:
> 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>
> ---

> +++ b/qapi-schema.json
> @@ -2243,6 +2243,8 @@
>  #
>  # @queues: #optional number of queues to be created for multiqueue capable tap
>  #
> +# @backend_features: #optional feature flag to support vhost user backend

Missing a (since 2.4) designation.

New interfaces should prefer - over _, but I see that you are consistent
with vnet_hder already in this struct so it is okay.

> +#
>  # Since 1.2
>  ##
>  { 'struct': 'NetdevTapOptions',
> @@ -2259,7 +2261,8 @@
>      '*vhostfd':    'str',
>      '*vhostfds':   'str',
>      '*vhostforce': 'bool',
> -    '*queues':     'uint32'} }
> +    '*queues':     'uint32',
> +    '*backend_features':'uint64'} }

Ewww. Making users figure out what integers to pass is NOT user
friendly.  Better would be an enum type, and make the parameter an
optional array of enum values.

>  
>  ##
>  # @NetdevSocketOptions
> @@ -2444,12 +2447,15 @@
>  #
>  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
>  #
> +# @backend_features: #optional feature flag to support vhost user backend (default: 0).
> +#

Long line. Please wrap to fit within 80 columns.  Missing (since 2.4)
deisgnation.  Again, I don't like making users know a raw integer; an
enum type would be better.

>  # Since 2.1
>  ##
>  { 'struct': 'NetdevVhostUserOptions',
>    'data': {
>      'chardev':        'str',
> -    '*vhostforce':    'bool' } }
> +    '*vhostforce':    'bool',
> +    '*backend_features':    'uint64' } }

This struct has no pre-existing _, so the name 'backend-features' is nicer.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Tetsuya Mukawa]

Hi Eric,

On 月, 2015-05-25 at 16:11 -0600, Eric Blake wrote:
> On 05/25/2015 01:28 AM, Tetsuya Mukawa wrote:
> > 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>
> > ---
> 
> > +++ b/qapi-schema.json
> > @@ -2243,6 +2243,8 @@
> >  #
> >  # @queues: #optional number of queues to be created for multiqueue capable tap
> >  #
> > +# @backend_features: #optional feature flag to support vhost user backend
> 
> Missing a (since 2.4) designation.
> 
> New interfaces should prefer - over _, but I see that you are consistent
> with vnet_hder already in this struct so it is okay.
> 
> > +#
> >  # Since 1.2
> >  ##
> >  { 'struct': 'NetdevTapOptions',
> > @@ -2259,7 +2261,8 @@
> >      '*vhostfd':    'str',
> >      '*vhostfds':   'str',
> >      '*vhostforce': 'bool',
> > -    '*queues':     'uint32'} }
> > +    '*queues':     'uint32',
> > +    '*backend_features':'uint64'} }
> 
> Ewww. Making users figure out what integers to pass is NOT user
> friendly.  Better would be an enum type, and make the parameter an
> optional array of enum values.

Thanks for your comments. 
I guess below may be good example. Is this same as your suggestion?

 virtio-net-pci,netdev=hostnet3,id=net3,gso=off,guest_tso4=off,guest_tso6=off

So I will improve 'backend-features' like below.

 backend-features=gso=off,guest_tso4=off,guest_tso6=off

Also I will fix 'qapi-schema.json' to work like above.

> 
> >  
> >  ##
> >  # @NetdevSocketOptions
> > @@ -2444,12 +2447,15 @@
> >  #
> >  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
> >  #
> > +# @backend_features: #optional feature flag to support vhost user backend (default: 0).
> > +#
> 
> Long line. Please wrap to fit within 80 columns.  Missing (since 2.4)
> deisgnation.  

I will fix it in v2 patch.

> Again, I don't like making users know a raw integer; an
> enum type would be better.
> 
> >  # Since 2.1
> >  ##
> >  { 'struct': 'NetdevVhostUserOptions',
> >    'data': {
> >      'chardev':        'str',
> > -    '*vhostforce':    'bool' } }
> > +    '*vhostforce':    'bool',
> > +    '*backend_features':    'uint64' } }
> 
> This struct has no pre-existing _, so the name 'backend-features' is nicer.
> 

Also I will fix it.

Regards,
Tetsuya

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

[Tetsuya Mukawa]

On 2015/05/26 11:46, Tetsuya Mukawa wrote:
> Hi Eric,
>
> On 月, 2015-05-25 at 16:11 -0600, Eric Blake wrote:
>> On 05/25/2015 01:28 AM, Tetsuya Mukawa wrote:
>>> 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>
>>> ---
>>> +++ b/qapi-schema.json
>>> @@ -2243,6 +2243,8 @@
>>>  #
>>>  # @queues: #optional number of queues to be created for multiqueue capable tap
>>>  #
>>> +# @backend_features: #optional feature flag to support vhost user backend
>> Missing a (since 2.4) designation.
>>
>> New interfaces should prefer - over _, but I see that you are consistent
>> with vnet_hder already in this struct so it is okay.
>>
>>> +#
>>>  # Since 1.2
>>>  ##
>>>  { 'struct': 'NetdevTapOptions',
>>> @@ -2259,7 +2261,8 @@
>>>      '*vhostfd':    'str',
>>>      '*vhostfds':   'str',
>>>      '*vhostforce': 'bool',
>>> -    '*queues':     'uint32'} }
>>> +    '*queues':     'uint32',
>>> +    '*backend_features':'uint64'} }
>> Ewww. Making users figure out what integers to pass is NOT user
>> friendly.  Better would be an enum type, and make the parameter an
>> optional array of enum values.
> Thanks for your comments. 
> I guess below may be good example. Is this same as your suggestion?
>
>  virtio-net-pci,netdev=hostnet3,id=net3,gso=off,guest_tso4=off,guest_tso6=off
>
> So I will improve 'backend-features' like below.
>
>  backend-features=gso=off,guest_tso4=off,guest_tso6=off
>
> Also I will fix 'qapi-schema.json' to work like above.

I seems it's impossible to implement like above.
I may need to implement like below.

virtio-net-pci,netdev=hostnet3,id=net3,backend_gso=on,backend_guest_tso4=on,backend_guest_tso6=on


Tetsuya

>>>  
>>>  ##
>>>  # @NetdevSocketOptions
>>> @@ -2444,12 +2447,15 @@
>>>  #
>>>  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
>>>  #
>>> +# @backend_features: #optional feature flag to support vhost user backend (default: 0).
>>> +#
>> Long line. Please wrap to fit within 80 columns.  Missing (since 2.4)
>> deisgnation.  
> I will fix it in v2 patch.
>
>> Again, I don't like making users know a raw integer; an
>> enum type would be better.
>>
>>>  # Since 2.1
>>>  ##
>>>  { 'struct': 'NetdevVhostUserOptions',
>>>    'data': {
>>>      'chardev':        'str',
>>> -    '*vhostforce':    'bool' } }
>>> +    '*vhostforce':    'bool',
>>> +    '*backend_features':    'uint64' } }
>> This struct has no pre-existing _, so the name 'backend-features' is nicer.
>>
> Also I will fix it.
>
> Regards,
> Tetsuya
>
>

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

On 05/25/2015 10:29 PM, Tetsuya Mukawa wrote:

>>>>  { 'struct': 'NetdevTapOptions',
>>>> @@ -2259,7 +2261,8 @@
>>>>      '*vhostfd':    'str',
>>>>      '*vhostfds':   'str',
>>>>      '*vhostforce': 'bool',
>>>> -    '*queues':     'uint32'} }
>>>> +    '*queues':     'uint32',
>>>> +    '*backend_features':'uint64'} }
>>> Ewww. Making users figure out what integers to pass is NOT user
>>> friendly.  Better would be an enum type, and make the parameter an
>>> optional array of enum values.
>> Thanks for your comments. 
>> I guess below may be good example. Is this same as your suggestion?
>>
>>  virtio-net-pci,netdev=hostnet3,id=net3,gso=off,guest_tso4=off,guest_tso6=off
>>
>> So I will improve 'backend-features' like below.
>>
>>  backend-features=gso=off,guest_tso4=off,guest_tso6=off
>>
>> Also I will fix 'qapi-schema.json' to work like above.
> 
> I seems it's impossible to implement like above.
> I may need to implement like below.
> 
> virtio-net-pci,netdev=hostnet3,id=net3,backend_gso=on,backend_guest_tso4=on,backend_guest_tso6=on

Or even:

virtio-net-pci,netdev=hostnet3,id=net3,backend.gso=on,backend.guest_tso4=on,backend.guest_tso6=on

Look at -device for how to set up nested structs using '.' for a nice
hierarchy of options all belonging to a common substruct.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Tetsuya Mukawa]

On 2015/05/26 21:52, Eric Blake wrote:
> On 05/25/2015 10:29 PM, Tetsuya Mukawa wrote:
>
>>>>>  { 'struct': 'NetdevTapOptions',
>>>>> @@ -2259,7 +2261,8 @@
>>>>>      '*vhostfd':    'str',
>>>>>      '*vhostfds':   'str',
>>>>>      '*vhostforce': 'bool',
>>>>> -    '*queues':     'uint32'} }
>>>>> +    '*queues':     'uint32',
>>>>> +    '*backend_features':'uint64'} }
>>>> Ewww. Making users figure out what integers to pass is NOT user
>>>> friendly.  Better would be an enum type, and make the parameter an
>>>> optional array of enum values.
>>> Thanks for your comments. 
>>> I guess below may be good example. Is this same as your suggestion?
>>>
>>>  virtio-net-pci,netdev=hostnet3,id=net3,gso=off,guest_tso4=off,guest_tso6=off
>>>
>>> So I will improve 'backend-features' like below.
>>>
>>>  backend-features=gso=off,guest_tso4=off,guest_tso6=off
>>>
>>> Also I will fix 'qapi-schema.json' to work like above.
>> I seems it's impossible to implement like above.
>> I may need to implement like below.
>>
>> virtio-net-pci,netdev=hostnet3,id=net3,backend_gso=on,backend_guest_tso4=on,backend_guest_tso6=on
> Or even:
>
> virtio-net-pci,netdev=hostnet3,id=net3,backend.gso=on,backend.guest_tso4=on,backend.guest_tso6=on
>
> Look at -device for how to set up nested structs using '.' for a nice
> hierarchy of options all belonging to a common substruct.
>

I appreciate for your suggestion.
I will check '-device' option, and implement like above in v2 patch.

Tetsuya

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

[Tetsuya Mukawa]

On 2015/05/28 10:25, Tetsuya Mukawa wrote:
> On 2015/05/26 21:52, Eric Blake wrote:
>> On 05/25/2015 10:29 PM, Tetsuya Mukawa wrote:
>>
>>>>>>  { 'struct': 'NetdevTapOptions',
>>>>>> @@ -2259,7 +2261,8 @@
>>>>>>      '*vhostfd':    'str',
>>>>>>      '*vhostfds':   'str',
>>>>>>      '*vhostforce': 'bool',
>>>>>> -    '*queues':     'uint32'} }
>>>>>> +    '*queues':     'uint32',
>>>>>> +    '*backend_features':'uint64'} }
>>>>> Ewww. Making users figure out what integers to pass is NOT user
>>>>> friendly.  Better would be an enum type, and make the parameter an
>>>>> optional array of enum values.
>>>> Thanks for your comments. 
>>>> I guess below may be good example. Is this same as your suggestion?
>>>>
>>>>  virtio-net-pci,netdev=hostnet3,id=net3,gso=off,guest_tso4=off,guest_tso6=off
>>>>
>>>> So I will improve 'backend-features' like below.
>>>>
>>>>  backend-features=gso=off,guest_tso4=off,guest_tso6=off
>>>>
>>>> Also I will fix 'qapi-schema.json' to work like above.
>>> I seems it's impossible to implement like above.
>>> I may need to implement like below.
>>>
>>> virtio-net-pci,netdev=hostnet3,id=net3,backend_gso=on,backend_guest_tso4=on,backend_guest_tso6=on
>> Or even:
>>
>> virtio-net-pci,netdev=hostnet3,id=net3,backend.gso=on,backend.guest_tso4=on,backend.guest_tso6=on
>>
>> Look at -device for how to set up nested structs using '.' for a nice
>> hierarchy of options all belonging to a common substruct.
>>
> I appreciate for your suggestion.
> I will check '-device' option, and implement like above in v2 patch.

I've checked '-device' option and DeviceClass, and found I may not be
able to use above nice hierarchy with '-net' option.
Probably it is because '-net' option isn't for describing device itself,
so there is no DeviceClass.
And without DeviceClass I guess I cannot use '.' infrastructure.

I may be able to describe vhost-user backend features in '-device
virtio-net-pci,.....', but I guess it's not good.
Probably describing in '-net vhost-user,....' will be good.

As described above, I implemented like below in next patch.

-chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
-device virtio-net-pci,netdev=net0 \
-netdev
vhost-user,id=net0,chardev=chr0,vhostforce,backend_gso=on,backend_guest_ecn=on

BTW, '-device' option has already had options like below.
 - guest_csum
 - guest_tso4
 .....

So I used '_' like below
 - backend_guest_csum
 - backend_guest_tso4
 ......

Regards,
Tetsuya

[Qemu-devel] [PATCH v1 0/4] 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_mrg_rxbuf' is an one of new flags. To know all, check the last patch)

    * 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_mrg_rxbuf \
     -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_mrg_rxbuf \
     -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
----------
- 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 (4):
  vhost-user: Add ability to know vhost-user backend disconnection
  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 features

 hw/net/vhost_net.c             |   6 ++-
 hw/net/virtio-net.c            |  13 +++++
 hw/scsi/vhost-scsi.c           |   2 +-
 hw/virtio/vhost-user.c         |  24 ++++++---
 hw/virtio/vhost.c              |   7 ++-
 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               |  69 ++++++++++++++++++++++++-
 qapi-schema.json               | 114 +++++++++++++++++++++++++++++++++++------
 qemu-char.c                    |  15 ++++++
 qemu-options.hx                |  10 ++++
 16 files changed, 256 insertions(+), 29 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v1 1/4] 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 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 11899c5..1967ff4 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 {
@@ -113,12 +114,23 @@ 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];
+
+    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->chr->label);
@@ -126,6 +138,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->chr->label);
         break;
     }
-- 
2.1.4

Re: [Qemu-devel] [PATCH v1 1/4] vhost-user: Add ability to know vhost-user backend disconnection

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

On Fri, May 29, 2015 at 01:42:27PM +0900, Tetsuya Mukawa wrote:
> 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 | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

It is unfortunate that qemu-chr.c doesn't have a built-in way to handle
G_IO_HUP and raise the CHR_EVENT_CLOSED event.  This is not really
specific to vhost-user.

CCing Paolo Bonzini (Character Devices).  He is currently on vacation so
let's not hold up this patch.  It can be made generic later, if
necessary.

> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 11899c5..1967ff4 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 {
> @@ -113,12 +114,23 @@ 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];
> +
> +    qemu_chr_fe_read_all(s->chr, buf, sizeof(buf));
> +    return FALSE;
> +}

If you respin this patch, please add a comment like:

/* We don't actually want to read anything, but CHR_EVENT_CLOSED will be
 * raised as a side-effect of the read.
 */

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v1 1/4] vhost-user: Add ability to know vhost-user backend disconnection

[Tetsuya Mukawa]

On 2015/06/15 22:32, Stefan Hajnoczi wrote:
> On Fri, May 29, 2015 at 01:42:27PM +0900, Tetsuya Mukawa wrote:
>> 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 | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
> It is unfortunate that qemu-chr.c doesn't have a built-in way to handle
> G_IO_HUP and raise the CHR_EVENT_CLOSED event.  This is not really
> specific to vhost-user.
>
> CCing Paolo Bonzini (Character Devices).  He is currently on vacation so
> let's not hold up this patch.  It can be made generic later, if
> necessary.
>
>> diff --git a/net/vhost-user.c b/net/vhost-user.c
>> index 11899c5..1967ff4 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 {
>> @@ -113,12 +114,23 @@ 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];
>> +
>> +    qemu_chr_fe_read_all(s->chr, buf, sizeof(buf));
>> +    return FALSE;
>> +}
> If you respin this patch, please add a comment like:
>
> /* We don't actually want to read anything, but CHR_EVENT_CLOSED will be
>  * raised as a side-effect of the read.
>  */

I appreciate for your all comments.
I will add above comments in next patches.

Tetsuya

[Qemu-devel] [PATCH v1 2/4] 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 | 17 ++++++++++-------
 include/sysemu/char.h  |  7 +++++++
 qemu-char.c            | 15 +++++++++++++++
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e7ab829..4d7e3ba 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;
@@ -237,7 +238,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);
@@ -281,7 +282,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;
     }
 
@@ -297,32 +298,34 @@ 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;
             }
             memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
             break;
         default:
             error_report("Received unexpected msg type.");
-            return -1;
-            break;
         }
     }
 
     return 0;
+
+close:
+    qemu_chr_shutdown(chr, SHUT_RDWR);
+    return -1;
 }
 
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 832b7fe..d944ded 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_shutdown)(struct CharDriverState *chr, int how);
     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_shutdown:
+ *
+ * Shutdown a fd of character backend.
+ */
+void qemu_chr_shutdown(CharDriverState *chr, int how);
 
 /**
  * @qemu_chr_delete:
diff --git a/qemu-char.c b/qemu-char.c
index d0c1564..2b28808 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2839,6 +2839,13 @@ static void tcp_chr_disconnect(CharDriverState *chr)
     }
 }
 
+static void tcp_chr_shutdown(CharDriverState *chr, int how)
+{
+    TCPCharDriver *s = chr->opaque;
+
+    shutdown(s->fd, how);
+}
+
 static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -3836,6 +3843,13 @@ void qemu_chr_fe_release(CharDriverState *s)
     s->avail_connections++;
 }
 
+void qemu_chr_shutdown(CharDriverState *chr, int how)
+{
+    if (chr->chr_shutdown) {
+        chr->chr_shutdown(chr, how);
+    }
+}
+
 void qemu_chr_delete(CharDriverState *chr)
 {
     QTAILQ_REMOVE(&chardevs, chr, next);
@@ -4154,6 +4168,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_shutdown = tcp_chr_shutdown;
     chr->get_msgfds = tcp_get_msgfds;
     chr->set_msgfds = tcp_set_msgfds;
     chr->chr_add_client = tcp_chr_add_client;
-- 
2.1.4

Re: [Qemu-devel] [PATCH v1 2/4] vhost-user: Shutdown vhost-user connection when wrong messages are passed

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 5536 bytes --]

On Fri, May 29, 2015 at 01:42:28PM +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>
> ---
>  hw/virtio/vhost-user.c | 17 ++++++++++-------
>  include/sysemu/char.h  |  7 +++++++
>  qemu-char.c            | 15 +++++++++++++++
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index e7ab829..4d7e3ba 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;
> @@ -237,7 +238,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);
> @@ -281,7 +282,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;
>      }
>  
> @@ -297,32 +298,34 @@ 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;
>              }
>              memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
>              break;
>          default:
>              error_report("Received unexpected msg type.");
> -            return -1;
> -            break;
>          }
>      }
>  
>      return 0;
> +
> +close:
> +    qemu_chr_shutdown(chr, SHUT_RDWR);
> +    return -1;
>  }

Why is shutdown(2) necessary?  We're aborting the connection and don't
expect to process any more data, so we could just close it.

> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 832b7fe..d944ded 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_shutdown)(struct CharDriverState *chr, int how);
>      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_shutdown:
> + *
> + * Shutdown a fd of character backend.
> + */
> +void qemu_chr_shutdown(CharDriverState *chr, int how);
>  
>  /**
>   * @qemu_chr_delete:
> diff --git a/qemu-char.c b/qemu-char.c
> index d0c1564..2b28808 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2839,6 +2839,13 @@ static void tcp_chr_disconnect(CharDriverState *chr)
>      }
>  }
>  
> +static void tcp_chr_shutdown(CharDriverState *chr, int how)
> +{
> +    TCPCharDriver *s = chr->opaque;
> +
> +    shutdown(s->fd, how);
> +}
> +
>  static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
>  {
>      CharDriverState *chr = opaque;
> @@ -3836,6 +3843,13 @@ void qemu_chr_fe_release(CharDriverState *s)
>      s->avail_connections++;
>  }
>  
> +void qemu_chr_shutdown(CharDriverState *chr, int how)
> +{
> +    if (chr->chr_shutdown) {
> +        chr->chr_shutdown(chr, how);
> +    }
> +}
> +
>  void qemu_chr_delete(CharDriverState *chr)
>  {
>      QTAILQ_REMOVE(&chardevs, chr, next);
> @@ -4154,6 +4168,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_shutdown = tcp_chr_shutdown;
>      chr->get_msgfds = tcp_get_msgfds;
>      chr->set_msgfds = tcp_set_msgfds;
>      chr->chr_add_client = tcp_chr_add_client;

Please split this into a separate qemu-char.c patch.  I hesitate to add
a shutdown(2) interface since it adds a new state that the rest of the
qemu-char.c code doesn't know about.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v1 2/4] vhost-user: Shutdown vhost-user connection when wrong messages are passed

[Tetsuya Mukawa]

On 2015/06/15 22:40, Stefan Hajnoczi wrote:
> On Fri, May 29, 2015 at 01:42:28PM +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>
>> ---
>>  hw/virtio/vhost-user.c | 17 ++++++++++-------
>>  include/sysemu/char.h  |  7 +++++++
>>  qemu-char.c            | 15 +++++++++++++++
>>  3 files changed, 32 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index e7ab829..4d7e3ba 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;
>> @@ -237,7 +238,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);
>> @@ -281,7 +282,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;
>>      }
>>  
>> @@ -297,32 +298,34 @@ 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;
>>              }
>>              memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
>>              break;
>>          default:
>>              error_report("Received unexpected msg type.");
>> -            return -1;
>> -            break;
>>          }
>>      }
>>  
>>      return 0;
>> +
>> +close:
>> +    qemu_chr_shutdown(chr, SHUT_RDWR);
>> +    return -1;
>>  }
> Why is shutdown(2) necessary?  We're aborting the connection and don't
> expect to process any more data, so we could just close it.

Yes, you are right.
It's my fault and here is current wrong behavior of this patch.

1. socket is shutdown by QEMU.
2. Vhost-user backend detects it, because socket is shutdown.
3. Vhost-user backend close socket.
    (This behavior is came from my test program)
4. QEMU detects it, then start closing event handling.

Apparently, I needed to close socket by QEMU. So I've checked the
qemu-char code more.
And I've found 'tcp_chr_disconnect' is the function I need to call, also
I've found I may not be able to use 'tcp_chr_close' for my purpose,
because 'tcp_chr_close' closes not only accepted fd, but also listen fd.
In my case, I just want to close accepted fd.
Because of above, 'qemu_chr_delete' that calls 'tcp_chr_close' is not
for my purpose.

Unfortunately, there is no function like 'qemu_chr_disconnect' in qemu-char.
So I may need to add it instead of 'qemu_chr_shutdown' in next patches.
Is this correct direction?

>
>> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
>> index 832b7fe..d944ded 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_shutdown)(struct CharDriverState *chr, int how);
>>      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_shutdown:
>> + *
>> + * Shutdown a fd of character backend.
>> + */
>> +void qemu_chr_shutdown(CharDriverState *chr, int how);
>>  
>>  /**
>>   * @qemu_chr_delete:
>> diff --git a/qemu-char.c b/qemu-char.c
>> index d0c1564..2b28808 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -2839,6 +2839,13 @@ static void tcp_chr_disconnect(CharDriverState *chr)
>>      }
>>  }
>>  
>> +static void tcp_chr_shutdown(CharDriverState *chr, int how)
>> +{
>> +    TCPCharDriver *s = chr->opaque;
>> +
>> +    shutdown(s->fd, how);
>> +}
>> +
>>  static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
>>  {
>>      CharDriverState *chr = opaque;
>> @@ -3836,6 +3843,13 @@ void qemu_chr_fe_release(CharDriverState *s)
>>      s->avail_connections++;
>>  }
>>  
>> +void qemu_chr_shutdown(CharDriverState *chr, int how)
>> +{
>> +    if (chr->chr_shutdown) {
>> +        chr->chr_shutdown(chr, how);
>> +    }
>> +}
>> +
>>  void qemu_chr_delete(CharDriverState *chr)
>>  {
>>      QTAILQ_REMOVE(&chardevs, chr, next);
>> @@ -4154,6 +4168,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_shutdown = tcp_chr_shutdown;
>>      chr->get_msgfds = tcp_get_msgfds;
>>      chr->set_msgfds = tcp_set_msgfds;
>>      chr->chr_add_client = tcp_chr_add_client;
> Please split this into a separate qemu-char.c patch.  I hesitate to add
> a shutdown(2) interface since it adds a new state that the rest of the
> qemu-char.c code doesn't know about.

As described above, I may need to add 'qemu_chr_disconnect'.
It will be the interface of closing accepted fd.

Regards,
Tetsuya

[Qemu-devel] [PATCH v1 3/4] 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 1967ff4..f823d78 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)
@@ -178,6 +180,10 @@ static int net_vhost_chardev_opts(const char *name, const char *value,
         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_report("vhost-user does not support a chardev"
                      " with the following option:\n %s = %s",
-- 
2.1.4

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

On Fri, May 29, 2015 at 01:42:29PM +0900, Tetsuya Mukawa 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(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

On Fri, May 29, 2015 at 01:42:29PM +0900, Tetsuya Mukawa 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 1967ff4..f823d78 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)
> @@ -178,6 +180,10 @@ static int net_vhost_chardev_opts(const char *name, const char *value,
>          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;

Where is the code that uses these new options?

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Tetsuya Mukawa]

On 2015/06/15 22:58, Stefan Hajnoczi wrote:
> On Fri, May 29, 2015 at 01:42:29PM +0900, Tetsuya Mukawa 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 1967ff4..f823d78 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)
>> @@ -178,6 +180,10 @@ static int net_vhost_chardev_opts(const char *name, const char *value,
>>          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;
> Where is the code that uses these new options?

Above code is only for allowing below QEMU options.

-chardev socket,id=chr0,path=/tmp/sock,reconnect=3
-chardev socket,id=chr0,path=/tmp/sock,server,nowait

These options are needed, because we don't want to wait for vhost-user
backend connection.

Regards,
Tetsuya

[Qemu-devel] [PATCH v1 4/4] vhost-user: Add new option to specify vhost-user backend features

[Tetsuya Mukawa]

This patch adds below '-net' options to let QEMU know which features
vhost-user backend will support.

 [,backend_csum=on|off]
 [,backend_guest_csum=on|off]
 [,backend_gso=on|off]
 [,backend_guest_tso4=on|off]
 [,backend_guest_tso6=on|off]
 [,backend_guest_ecn=on|off]
 [,backend_guest_ufo=on|off]
 [,backend_guest_announce=on|off]
 [,backend_host_tso4=on|off]
 [,backend_host_tso6=on|off]
 [,backend_host_ecn=on|off]
 [,backend_host_ufo=on|off]
 [,backend_mrg_rxbuf=on|off]
 [,backend_status=on|off]
 [,backend_ctrl_vq=on|off]
 [,backend_ctrl_vx=on|off]
 [,backend_ctrl_vlan=on|off]
 [,backend_ctrl_rx_extra=on|off]
 [,backend_ctrl_mac_addr=on|off]
 [,backend_ctrl_guest_offloads=on|off]
 [,backend_mq=on|off]

If above features are specified, QEMU assumes vhost-user backend supports
the features, then QEMU can start without vhost-user backend connection.
(While no connection, link status of virtio-net device will be down)

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_mrg_rxbuf \
 -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_mrg_rxbuf \
 -device virtio-net-pci,netdev=net0 \

Above cases, QEMU assumes vhost-user backend will support
VIRTIO_NET_F_MRG_RXBUF feature defined in linux/virtio_net.h

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.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 hw/net/vhost_net.c             |   6 ++-
 hw/net/virtio-net.c            |  13 +++++
 hw/scsi/vhost-scsi.c           |   2 +-
 hw/virtio/vhost-user.c         |   7 +++
 hw/virtio/vhost.c              |   7 ++-
 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               |  49 +++++++++++++++++-
 qapi-schema.json               | 114 +++++++++++++++++++++++++++++++++++------
 qemu-options.hx                |  10 ++++
 14 files changed, 204 insertions(+), 22 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 47f8b89..8799c75 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -158,8 +158,12 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
     net->dev.nvqs = 2;
     net->dev.vqs = net->vqs;
 
+    if (options->backend_features)
+        net->dev.backend_features = options->backend_features;
+
     r = vhost_dev_init(&net->dev, options->opaque,
-                       options->backend_type, options->force);
+                       options->backend_type, options->force,
+                       options->backend_features);
     if (r < 0) {
         goto fail;
     }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3af6faf..7fbb306 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -366,6 +366,17 @@ 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 i;
@@ -463,6 +474,8 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
     }
 
     if (!get_vhost_net(nc->peer)) {
+        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 335f442..25fae56 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, true);
+                         VHOST_BACKEND_TYPE_KERNEL, true, 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 4d7e3ba..d847ea5 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -307,6 +307,13 @@ 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%lx",
+                                 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 54851b7..0663aed 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -811,7 +811,8 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
 }
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-                   VhostBackendType backend_type, bool force)
+                   VhostBackendType backend_type, bool force,
+                   unsigned long long backend_features)
 {
     uint64_t features;
     int i, r;
@@ -833,7 +834,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 
     r = hdev->vhost_ops->vhost_call(hdev, VHOST_GET_FEATURES, &features);
     if (r < 0) {
-        goto fail;
+        if (backend_features == 0)
+            goto fail;
+        features = backend_features;
     }
 
     for (i = 0; i < hdev->nvqs; ++i) {
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 8f04888..b75ed70 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -55,7 +55,8 @@ struct vhost_dev {
 };
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-                   VhostBackendType backend_type, bool force);
+                   VhostBackendType backend_type, bool force,
+                   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 e0dbb41..3edd175 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 e66ca03..16b855e 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -55,6 +55,7 @@ typedef bool (HasVnetHdrLen)(NetClientState *, int);
 typedef void (UsingVnetHdr)(NetClientState *, bool);
 typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
 typedef void (SetVnetHdrLen)(NetClientState *, int);
+typedef unsigned long long (BackendFeatures)(NetClientState *);
 
 typedef struct NetClientInfo {
     NetClientOptionsKind type;
@@ -73,6 +74,7 @@ typedef struct NetClientInfo {
     UsingVnetHdr *using_vnet_hdr;
     SetOffload *set_offload;
     SetVnetHdrLen *set_vnet_hdr_len;
+    BackendFeatures *backend_features;
 } NetClientInfo;
 
 struct NetClientState {
@@ -136,6 +138,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 b1c18a3..ca6c5ab 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -12,6 +12,7 @@ typedef struct VhostNetOptions {
     NetClientState *net_backend;
     void *opaque;
     bool force;
+    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 db6be12..7415eb2 100644
--- a/net/net.c
+++ b/net/net.c
@@ -510,6 +510,15 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
     nc->info->set_vnet_hdr_len(nc, len);
 }
 
+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 d1ca314..f2ba94d 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -656,6 +656,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.force = tap->has_vhostforce && tap->vhostforce;
+        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 f823d78..6474a31 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -8,6 +8,8 @@
  *
  */
 
+#include <linux/virtio_net.h>
+
 #include "clients.h"
 #include "net/vhost_net.h"
 #include "net/vhost-user.h"
@@ -19,6 +21,7 @@ typedef struct VhostUserState {
     NetClientState nc;
     CharDriverState *chr;
     VHostNetState *vhost_net;
+    unsigned long long backend_features;
     int watch;
 } VhostUserState;
 
@@ -54,6 +57,7 @@ static int vhost_user_start(VhostUserState *s)
     options.net_backend = &s->nc;
     options.opaque = s->chr;
     options.force = true;
+    options.backend_features = s->backend_features;
 
     s->vhost_net = vhost_net_init(&options);
 
@@ -91,12 +95,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)
@@ -148,7 +162,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)
+                               const char *name, CharDriverState *chr,
+                               unsigned long long backend_features)
 {
     NetClientState *nc;
     VhostUserState *s;
@@ -163,6 +178,7 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
     /* We don't provide a receive callback */
     s->nc.receive_disabled = 1;
     s->chr = chr;
+    s->backend_features = backend_features;
 
     qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
 
@@ -242,12 +258,38 @@ static int net_vhost_check_net(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+static inline unsigned long long
+net_get_vhost_backend_features(const NetdevVhostUserOptions *opts)
+{
+    return (opts->backend_csum ? 1 << VIRTIO_NET_F_CSUM : 0) |
+        (opts->backend_guest_csum ? 1 << VIRTIO_NET_F_GUEST_CSUM : 0) |
+        (opts->backend_gso ? 1 << VIRTIO_NET_F_GSO : 0) |
+        (opts->backend_guest_tso4 ? 1 << VIRTIO_NET_F_GUEST_TSO4 : 0) |
+        (opts->backend_guest_tso6 ? 1 << VIRTIO_NET_F_GUEST_TSO6 : 0) |
+        (opts->backend_guest_ecn ? 1 << VIRTIO_NET_F_GUEST_ECN : 0) |
+        (opts->backend_guest_ufo ? 1 << VIRTIO_NET_F_GUEST_UFO : 0) |
+        (opts->backend_guest_announce ? 1 << VIRTIO_NET_F_GUEST_ANNOUNCE : 0) |
+        (opts->backend_host_tso4 ? 1 << VIRTIO_NET_F_HOST_TSO4 : 0) |
+        (opts->backend_host_tso6 ? 1 << VIRTIO_NET_F_HOST_TSO6 : 0) |
+        (opts->backend_host_ecn ? 1 << VIRTIO_NET_F_HOST_ECN : 0) |
+        (opts->backend_host_ufo ? 1 << VIRTIO_NET_F_HOST_UFO : 0) |
+        (opts->backend_mrg_rxbuf ? 1 << VIRTIO_NET_F_MRG_RXBUF : 0) |
+        (opts->backend_status ? 1 << VIRTIO_NET_F_STATUS : 0) |
+        (opts->backend_ctrl_vq ? 1 << VIRTIO_NET_F_CTRL_VQ : 0) |
+        (opts->backend_ctrl_rx ? 1 << VIRTIO_NET_F_CTRL_RX : 0) |
+        (opts->backend_ctrl_vlan ? 1 << VIRTIO_NET_F_CTRL_VLAN : 0) |
+        (opts->backend_ctrl_rx_extra ? 1 << VIRTIO_NET_F_CTRL_RX_EXTRA : 0) |
+        (opts->backend_ctrl_mac_addr ? 1 << VIRTIO_NET_F_CTRL_MAC_ADDR : 0) |
+        (opts->backend_mq ? 1 << VIRTIO_NET_F_MQ : 0);
+}
+
 int net_init_vhost_user(const NetClientOptions *opts, const char *name,
                         NetClientState *peer, Error **errp)
 {
     /* FIXME error_setg(errp, ...) on failure */
     const NetdevVhostUserOptions *vhost_user_opts;
     CharDriverState *chr;
+    unsigned long long backend_features;
 
     assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
     vhost_user_opts = opts->vhost_user;
@@ -264,6 +306,9 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
         return -1;
     }
 
+    /* backend features */
+    backend_features = net_get_vhost_backend_features(vhost_user_opts);
 
-    return net_vhost_user_init(peer, "vhost_user", name, chr);
+    return net_vhost_user_init(peer, "vhost_user", name, chr,
+                                backend_features);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index f97ffa1..1e868a9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2241,25 +2241,29 @@
 #
 # @vhostforce: #optional vhost on for non-MSIX virtio guests
 #
+# @backend_features: #optional feature flag to support vhost user backend
+#                    (Since 2.4)
+#
 # @queues: #optional number of queues to be created for multiqueue capable tap
 #
 # Since 1.2
 ##
 { 'struct': 'NetdevTapOptions',
   'data': {
-    '*ifname':     'str',
-    '*fd':         'str',
-    '*fds':        'str',
-    '*script':     'str',
-    '*downscript': 'str',
-    '*helper':     'str',
-    '*sndbuf':     'size',
-    '*vnet_hdr':   'bool',
-    '*vhost':      'bool',
-    '*vhostfd':    'str',
-    '*vhostfds':   'str',
-    '*vhostforce': 'bool',
-    '*queues':     'uint32'} }
+    '*ifname':           'str',
+    '*fd':               'str',
+    '*fds':              'str',
+    '*script':           'str',
+    '*downscript':       'str',
+    '*helper':           'str',
+    '*sndbuf':           'size',
+    '*vnet_hdr':         'bool',
+    '*vhost':            'bool',
+    '*vhostfd':          'str',
+    '*vhostfds':         'str',
+    '*vhostforce':       'bool',
+    '*backend_features': 'uint64',
+    '*queues':           'uint32'} }
 
 ##
 # @NetdevSocketOptions
@@ -2444,12 +2448,92 @@
 #
 # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
 #
+# @backend_csum: #optional feature backend supports (default: false).
+#                (Since 2.4)
+#
+# @backend_guest_csum: #optional feature backend supports (default: false).
+#                      (Since 2.4)
+#
+# @backend_gso: #optional feature backend supports (default: false).
+#               (Since 2.4)
+#
+# @backend_guest_tso4: #optional feature backend supports (default: false).
+#                      (Since 2.4)
+#
+# @backend_guest_tso6: #optional feature backend supports (default: false).
+#                      (Since 2.4)
+#
+# @backend_guest_ecn: #optional feature backend supports (default: false).
+#                     (Since 2.4)
+#
+# @backend_guest_ufo: #optional feature backend supports (default: false).
+#                     (Since 2.4)
+#
+# @backend_guest_announce: #optional feature backend supports (default: false).
+#                          (Since 2.4)
+#
+# @backend_host_tso4: #optional feature backend supports (default: false).
+#                     (Since 2.4)
+#
+# @backend_host_tso6: #optional feature backend supports (default: false).
+#                     (Since 2.4)
+#
+# @backend_host_ecn: #optional feature backend supports (default: false).
+#                    (Since 2.4)
+#
+# @backend_host_ufo: #optional feature backend supports (default: false).
+#                    (Since 2.4)
+#
+# @backend_mrg_rxbuf: #optional feature backend supports (default: false).
+#                     (Since 2.4)
+#
+# @backend_status: #optional feature backend supports (default: false).
+#                  (Since 2.4)
+#
+# @backend_ctrl_vq: #optional feature backend supports (default: false).
+#                   (Since 2.4)
+#
+# @backend_ctrl_rx: #optional feature backend supports (default: false).
+#                   (Since 2.4)
+#
+# @backend_ctrl_vlan: #optional feature backend supports (default: false).
+#                     (Since 2.4)
+#
+# @backend_ctrl_rx_extra: #optional feature backend supports (default: false).
+#                         (Since 2.4)
+#
+# @backend_ctrl_mac_addr: #optional feature backend supports (default: false).
+#                         (Since 2.4)
+#
+# @backend_mq: #optional feature backend supports (default: false).
+#              (Since 2.4)
+#
 # Since 2.1
 ##
 { 'struct': 'NetdevVhostUserOptions',
   'data': {
-    'chardev':        'str',
-    '*vhostforce':    'bool' } }
+    'chardev':                 'str',
+    '*vhostforce':             'bool',
+    '*backend_csum':           'bool',
+    '*backend_guest_csum':     'bool',
+    '*backend_gso':            'bool',
+    '*backend_guest_tso4':     'bool',
+    '*backend_guest_tso6':     'bool',
+    '*backend_guest_ecn':      'bool',
+    '*backend_guest_ufo':      'bool',
+    '*backend_guest_announce': 'bool',
+    '*backend_host_tso4':      'bool',
+    '*backend_host_tso6':      'bool',
+    '*backend_host_ecn':       'bool',
+    '*backend_host_ufo':       'bool',
+    '*backend_mrg_rxbuf':      'bool',
+    '*backend_status':         'bool',
+    '*backend_ctrl_vq':        'bool',
+    '*backend_ctrl_rx':        'bool',
+    '*backend_ctrl_vlan':      'bool',
+    '*backend_ctrl_rx_extra':  'bool',
+    '*backend_ctrl_mac_addr':  'bool',
+    '*backend_mq':             'bool' } }
 
 ##
 # @NetClientOptions
diff --git a/qemu-options.hx b/qemu-options.hx
index 88d7661..133dc99 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1467,6 +1467,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-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"
+    "         [,backend_csum=on|off][,backend_guest_csum=on|off][,backend_gso=on|off]\n"
+    "         [,backend_guest_tso4=on|off][,backend_guest_tso6=on|off]\n"
+    "         [,backend_guest_ecn=on|off][,backend_guest_ufo=on|off]\n"
+    "         [,backend_guest_announce=on|off][,backend_host_tso4=on|off]\n"
+    "         [,backend_host_tso6=on|off][,backend_host_ecn=on|off]\n"
+    "         [,backend_host_ufo=on|off][,backend_mrg_rxbuf=on|off][,backend_status=on|off]\n"
+    "         [,backend_ctrl_vq=on|off][,backend_ctrl_vx=on|off][,backend_ctrl_vlan=on|off]\n"
+    "         [,backend_ctrl_rx_extra=on|off][,backend_ctrl_mac_addr=on|off]\n"
+    "         [,backend_ctrl_guest_offloads=on|off][,backend_mq=on|off]\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"
@@ -1486,6 +1495,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_*=on' to specify virtio-net feature that vhost-user backend supports\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 v1 4/4] vhost-user: Add new option to specify vhost-user backend features

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

On Fri, May 29, 2015 at 01:42:30PM +0900, Tetsuya Mukawa wrote:
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 3af6faf..7fbb306 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -366,6 +366,17 @@ 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;

QEMU coding style always uses {} even for single statement if bodies:

if (!peer_has_vnet_hdr(n)) {
    return 0;
}

> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 4d7e3ba..d847ea5 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -307,6 +307,13 @@ 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%lx",

Please use PRIx64 for msg.u64 to avoid compiler errors on 32-bit hosts.

> +                                 dev->backend_features, msg.u64);
> +                    goto close;

QEMU uses 4-space indentation.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v1 4/4] vhost-user: Add new option to specify vhost-user backend features

[Tetsuya Mukawa]

On 2015/06/15 22:58, Stefan Hajnoczi wrote:
> On Fri, May 29, 2015 at 01:42:30PM +0900, Tetsuya Mukawa wrote:
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 3af6faf..7fbb306 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -366,6 +366,17 @@ 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;
> QEMU coding style always uses {} even for single statement if bodies:
>
> if (!peer_has_vnet_hdr(n)) {
>     return 0;
> }
>
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index 4d7e3ba..d847ea5 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -307,6 +307,13 @@ 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%lx",
> Please use PRIx64 for msg.u64 to avoid compiler errors on 32-bit hosts.
>> +                                 dev->backend_features, msg.u64);
>> +                    goto close;
> QEMU uses 4-space indentation.

I will fix above 3 issues in next patches.

Regards,
Tetsuya

Re: [Qemu-devel] [PATCH v1 4/4] vhost-user: Add new option to specify vhost-user backend features

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 4176 bytes --]

On 05/28/2015 10:42 PM, Tetsuya Mukawa wrote:
> This patch adds below '-net' options to let QEMU know which features
> vhost-user backend will support.

[meta-comment: when posting a new revision of a series, do it as a new
top-level thread rather than buried in-reply-to an earlier version]

> If above features are specified, QEMU assumes vhost-user backend supports
> the features, then QEMU can start without vhost-user backend connection.
> (While no connection, link status of virtio-net device will be down)
> 

> 
> When connection between QEMU and the backend is established, QEMU checkes feature

s/checkes/checks/

> values of the backend to make sure the expected features are provided.
> If it doesn't, the connection will be closed by QEMU.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---

> +++ b/qapi-schema.json
> @@ -2241,25 +2241,29 @@
>  #
>  # @vhostforce: #optional vhost on for non-MSIX virtio guests
>  #
> +# @backend_features: #optional feature flag to support vhost user backend
> +#                    (Since 2.4)
> +#

s/backend_features/backend-features/

(we document that qapi should prefer - over _ unless there is a
consistency issue with _ already used in the struct)


> @@ -2444,12 +2448,92 @@
>  #
>  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
>  #
> +# @backend_csum: #optional feature backend supports (default: false).
> +#                (Since 2.4)

Again, s/_/-/ throughout this struct.

> +#
> +# @backend_mq: #optional feature backend supports (default: false).
> +#              (Since 2.4)

A lot of identical descriptions. Might be worth more specific text to
each option; for example,
 @backend_mq: #optional: True if backend supports multiqueue feature
(default: false). (Since 2.4)


> +++ b/qemu-options.hx
> @@ -1467,6 +1467,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>      "-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"
> +    "         [,backend_csum=on|off][,backend_guest_csum=on|off][,backend_gso=on|off]\n"
> +    "         [,backend_guest_tso4=on|off][,backend_guest_tso6=on|off]\n"
> +    "         [,backend_guest_ecn=on|off][,backend_guest_ufo=on|off]\n"
> +    "         [,backend_guest_announce=on|off][,backend_host_tso4=on|off]\n"
> +    "         [,backend_host_tso6=on|off][,backend_host_ecn=on|off]\n"
> +    "         [,backend_host_ufo=on|off][,backend_mrg_rxbuf=on|off][,backend_status=on|off]\n"
> +    "         [,backend_ctrl_vq=on|off][,backend_ctrl_vx=on|off][,backend_ctrl_vlan=on|off]\n"
> +    "         [,backend_ctrl_rx_extra=on|off][,backend_ctrl_mac_addr=on|off]\n"
> +    "         [,backend_ctrl_guest_offloads=on|off][,backend_mq=on|off]\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"
> @@ -1486,6 +1495,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_*=on' to specify virtio-net feature that vhost-user backend supports\n"

Bikeshedding: Rather than having a lot of booleans, I wonder if it would
be better to have an array of flag names.  That is, in QMP form, this
would select three features (and leave the others off)

'backend-features':[ 'csum', 'guest-csum', gso' ]

although I'm not sure on how that would best translate to the command line.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v1 4/4] vhost-user: Add new option to specify vhost-user backend features

[Tetsuya Mukawa]

On 2015/06/16 21:27, Eric Blake wrote:
> On 05/28/2015 10:42 PM, Tetsuya Mukawa wrote:
>> This patch adds below '-net' options to let QEMU know which features
>> vhost-user backend will support.
> [meta-comment: when posting a new revision of a series, do it as a new
> top-level thread rather than buried in-reply-to an earlier version]

Hi Eric,

Thanks for comments.
Yes, I will follow like above in next patches.

>> If above features are specified, QEMU assumes vhost-user backend supports
>> the features, then QEMU can start without vhost-user backend connection.
>> (While no connection, link status of virtio-net device will be down)
>>
>> When connection between QEMU and the backend is established, QEMU checkes feature
> s/checkes/checks/
>
>> values of the backend to make sure the expected features are provided.
>> If it doesn't, the connection will be closed by QEMU.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>> +++ b/qapi-schema.json
>> @@ -2241,25 +2241,29 @@
>>  #
>>  # @vhostforce: #optional vhost on for non-MSIX virtio guests
>>  #
>> +# @backend_features: #optional feature flag to support vhost user backend
>> +#                    (Since 2.4)
>> +#
> s/backend_features/backend-features/
>
> (we document that qapi should prefer - over _ unless there is a
> consistency issue with _ already used in the struct)
>
>
>> @@ -2444,12 +2448,92 @@
>>  #
>>  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
>>  #
>> +# @backend_csum: #optional feature backend supports (default: false).
>> +#                (Since 2.4)
> Again, s/_/-/ throughout this struct.

I will use '-' instead of '_'. Also will fix typo.

>> +#
>> +# @backend_mq: #optional feature backend supports (default: false).
>> +#              (Since 2.4)
> A lot of identical descriptions. Might be worth more specific text to
> each option; for example,
>  @backend_mq: #optional: True if backend supports multiqueue feature
> (default: false). (Since 2.4)
>

Sure I will add short descriptions, if we use this kind of command options.

>> +++ b/qemu-options.hx
>> @@ -1467,6 +1467,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>>      "-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"
>> +    "         [,backend_csum=on|off][,backend_guest_csum=on|off][,backend_gso=on|off]\n"
>> +    "         [,backend_guest_tso4=on|off][,backend_guest_tso6=on|off]\n"
>> +    "         [,backend_guest_ecn=on|off][,backend_guest_ufo=on|off]\n"
>> +    "         [,backend_guest_announce=on|off][,backend_host_tso4=on|off]\n"
>> +    "         [,backend_host_tso6=on|off][,backend_host_ecn=on|off]\n"
>> +    "         [,backend_host_ufo=on|off][,backend_mrg_rxbuf=on|off][,backend_status=on|off]\n"
>> +    "         [,backend_ctrl_vq=on|off][,backend_ctrl_vx=on|off][,backend_ctrl_vlan=on|off]\n"
>> +    "         [,backend_ctrl_rx_extra=on|off][,backend_ctrl_mac_addr=on|off]\n"
>> +    "         [,backend_ctrl_guest_offloads=on|off][,backend_mq=on|off]\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"
>> @@ -1486,6 +1495,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_*=on' to specify virtio-net feature that vhost-user backend supports\n"
> Bikeshedding: Rather than having a lot of booleans, I wonder if it would
> be better to have an array of flag names.  That is, in QMP form, this
> would select three features (and leave the others off)
>
> 'backend-features':[ 'csum', 'guest-csum', gso' ]
>
> although I'm not sure on how that would best translate to the command line.
>

One more suggestion is here.
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg03807.html

I am also not sure what is best. But I guess above explanation is
reasonable at this time.
When we find a case that user needs to describe or change
backend-features frequently by hand, expand backend-features and add
these human readable options.

Regards,
Tetsuya

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

[Tetsuya Mukawa]

On 2015/05/29 13:42, 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. 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_mrg_rxbuf' is an one of new flags. To know all, check the last patch)
>
>     * 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_mrg_rxbuf \
>      -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_mrg_rxbuf \
>      -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
> ----------
> - 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 (4):
>   vhost-user: Add ability to know vhost-user backend disconnection
>   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 features
>
>  hw/net/vhost_net.c             |   6 ++-
>  hw/net/virtio-net.c            |  13 +++++
>  hw/scsi/vhost-scsi.c           |   2 +-
>  hw/virtio/vhost-user.c         |  24 ++++++---
>  hw/virtio/vhost.c              |   7 ++-
>  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               |  69 ++++++++++++++++++++++++-
>  qapi-schema.json               | 114 +++++++++++++++++++++++++++++++++++------
>  qemu-char.c                    |  15 ++++++
>  qemu-options.hx                |  10 ++++
>  16 files changed, 256 insertions(+), 29 deletions(-)
>

Ping.

Could someone please review this patches?
http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=66139

Regards,
Tetsuya

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 4331 bytes --]

On Thu, Jun 11, 2015 at 10:56:06AM +0900, Tetsuya Mukawa wrote:
> On 2015/05/29 13:42, 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. 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_mrg_rxbuf' is an one of new flags. To know all, check the last patch)
> >
> >     * 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_mrg_rxbuf \
> >      -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_mrg_rxbuf \
> >      -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
> > ----------
> > - 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 (4):
> >   vhost-user: Add ability to know vhost-user backend disconnection
> >   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 features
> >
> >  hw/net/vhost_net.c             |   6 ++-
> >  hw/net/virtio-net.c            |  13 +++++
> >  hw/scsi/vhost-scsi.c           |   2 +-
> >  hw/virtio/vhost-user.c         |  24 ++++++---
> >  hw/virtio/vhost.c              |   7 ++-
> >  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               |  69 ++++++++++++++++++++++++-
> >  qapi-schema.json               | 114 +++++++++++++++++++++++++++++++++++------
> >  qemu-char.c                    |  15 ++++++
> >  qemu-options.hx                |  10 ++++
> >  16 files changed, 256 insertions(+), 29 deletions(-)
> >
> 
> Ping.
> 
> Could someone please review this patches?
> http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=66139

Nikolay, Jason, or Michael:
Can one of you please step up to become maintainer of net/vhost-user.c.

I don't follow vhost-user development much and can't give good review
feedback about the vhost-user specific aspects.

Please send a patch that adds you to the MAINTAINERS file for
net/vhost-user.c.  Then either include merged patches in Michael's pull
requests or ask me (and in the future, Jason) to include them in net
pull requests.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Michael S. Tsirkin]

On Mon, Jun 15, 2015 at 03:12:28PM +0100, Stefan Hajnoczi wrote:
> On Thu, Jun 11, 2015 at 10:56:06AM +0900, Tetsuya Mukawa wrote:
> > On 2015/05/29 13:42, 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. 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_mrg_rxbuf' is an one of new flags. To know all, check the last patch)
> > >
> > >     * 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_mrg_rxbuf \
> > >      -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_mrg_rxbuf \
> > >      -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
> > > ----------
> > > - 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 (4):
> > >   vhost-user: Add ability to know vhost-user backend disconnection
> > >   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 features
> > >
> > >  hw/net/vhost_net.c             |   6 ++-
> > >  hw/net/virtio-net.c            |  13 +++++
> > >  hw/scsi/vhost-scsi.c           |   2 +-
> > >  hw/virtio/vhost-user.c         |  24 ++++++---
> > >  hw/virtio/vhost.c              |   7 ++-
> > >  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               |  69 ++++++++++++++++++++++++-
> > >  qapi-schema.json               | 114 +++++++++++++++++++++++++++++++++++------
> > >  qemu-char.c                    |  15 ++++++
> > >  qemu-options.hx                |  10 ++++
> > >  16 files changed, 256 insertions(+), 29 deletions(-)
> > >
> > 
> > Ping.
> > 
> > Could someone please review this patches?
> > http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=66139
> 
> Nikolay, Jason, or Michael:
> Can one of you please step up to become maintainer of net/vhost-user.c.
> 
> I don't follow vhost-user development much and can't give good review
> feedback about the vhost-user specific aspects.
> 
> Please send a patch that adds you to the MAINTAINERS file for
> net/vhost-user.c.  Then either include merged patches in Michael's pull
> requests or ask me (and in the future, Jason) to include them in net
> pull requests.
> 
> Stefan

Done.

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]

On Fri, May 29, 2015 at 01:42:26PM +0900, Tetsuya Mukawa wrote:
>    - Change user interface to be able to specify each feature by name.

I disagree with this change, the uint64_t backend_features option was
better because:

The text options will just result in a very long command-line that is
hard to read for humans.  Instead of checking a bit in a single hex
number, you have to scan over long comma-separated text options which
can be in any arbitrary order to find the feature bit you were looking
for.

This option needs to be scripted or generated by a management tool
anyway since it depends on the vhost-user backend program that is being
paired with QEMU.  The number of human users constructing these option
is extremely low - if they use it they'll copy-paste it from their
vhost-user program's documentation so the contents don't matter much
(shorter is better though).

The text options must be updated when new feature bits are added to
virtio-net.

For these reasons I find the numeric backend_features option better.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Tetsuya Mukawa]

On 2015/06/15 23:08, Stefan Hajnoczi wrote:
> On Fri, May 29, 2015 at 01:42:26PM +0900, Tetsuya Mukawa wrote:
>>    - Change user interface to be able to specify each feature by name.
> I disagree with this change, the uint64_t backend_features option was
> better because:
>
> The text options will just result in a very long command-line that is
> hard to read for humans.  Instead of checking a bit in a single hex
> number, you have to scan over long comma-separated text options which
> can be in any arbitrary order to find the feature bit you were looking
> for.
>
> This option needs to be scripted or generated by a management tool
> anyway since it depends on the vhost-user backend program that is being
> paired with QEMU.  The number of human users constructing these option
> is extremely low - if they use it they'll copy-paste it from their
> vhost-user program's documentation so the contents don't matter much
> (shorter is better though).
>
> The text options must be updated when new feature bits are added to
> virtio-net.
>
> For these reasons I find the numeric backend_features option better.

Thanks for comments.
I guess it's reasonable, so I will revert it in next patches.

Regards,
Tetsuya