[Qemu-devel] [PATCH v3 0/3] vhost-user mq sequel works

[Qemu-devel] [PATCH v3 0/3] vhost-user mq sequel works

[Yuanhan Liu]

Patch 1 is basically reverts the patch does the rename, as it
    may break build for VHOST_RESET_OWNER is no longer defined.

    As Michael stated, reverting it is not enough, we need
    send it from the right place, and send SET_VRING_ENABLE
    at start/stop, which is something I will do later.

Patch 2 is a minor fix or request naming in vhost-user spec

Patch 3 adds the vhost-user mq unit test

---
Yuanhan Liu (3):
  vhost: rename VHOST_RESET_DEVICE back to VHOST_RESET_OWNER
  doc: vhost-user: request naming fix
  vhost-user-test: add multiple queue test

 docs/specs/vhost-user.txt   | 25 +++++++++++++------------
 hw/virtio/vhost-backend.c   |  2 +-
 hw/virtio/vhost-user.c      |  6 +++---
 linux-headers/linux/vhost.h |  2 +-
 tests/vhost-user-test.c     | 27 +++++++++++++++++++++++----
 5 files changed, 41 insertions(+), 21 deletions(-)

-- 
1.9.0

[Qemu-devel] [PATCH v3 1/3] vhost: rename VHOST_RESET_DEVICE back to VHOST_RESET_OWNER

[Yuanhan Liu]

It turned out that it breaks stuff (VHOST_RESET_OWNER is not defined),
so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html

CC: "Michael S. Tsirkin" <mst@redhat.com>
Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/virtio/vhost-backend.c   | 2 +-
 hw/virtio/vhost-user.c      | 6 +++---
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index e0d71e2..d319715 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -255,10 +255,10 @@ Message types
       as an owner of the session. This can be used on the Slave as a
       "session start" flag.
 
- * VHOST_USER_RESET_DEVICE
+ * VHOST_USER_RESET_OWNER
 
       Id: 4
-      Equivalent ioctl: VHOST_RESET_DEVICE
+      Equivalent ioctl: VHOST_RESET_OWNER
       Master payload: N/A
 
       Issued when a new connection is about to be closed. The Master will no
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 1d5f684..b734a60 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -156,7 +156,7 @@ static int vhost_kernel_set_owner(struct vhost_dev *dev)
 
 static int vhost_kernel_reset_device(struct vhost_dev *dev)
 {
-    return vhost_kernel_call(dev, VHOST_RESET_DEVICE, NULL);
+    return vhost_kernel_call(dev, VHOST_RESET_OWNER, NULL);
 }
 
 static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 78442ba..7c532f4 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -43,7 +43,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_FEATURES = 1,
     VHOST_USER_SET_FEATURES = 2,
     VHOST_USER_SET_OWNER = 3,
-    VHOST_USER_RESET_DEVICE = 4,
+    VHOST_USER_RESET_OWNER = 4,
     VHOST_USER_SET_MEM_TABLE = 5,
     VHOST_USER_SET_LOG_BASE = 6,
     VHOST_USER_SET_LOG_FD = 7,
@@ -157,7 +157,7 @@ static bool vhost_user_one_time_request(VhostUserRequest request)
 {
     switch (request) {
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
     case VHOST_USER_SET_MEM_TABLE:
     case VHOST_USER_GET_QUEUE_NUM:
         return true;
@@ -486,7 +486,7 @@ static int vhost_user_set_owner(struct vhost_dev *dev)
 static int vhost_user_reset_device(struct vhost_dev *dev)
 {
     VhostUserMsg msg = {
-        .request = VHOST_USER_RESET_DEVICE,
+        .request = VHOST_USER_RESET_OWNER,
         .flags = VHOST_USER_VERSION,
     };
 
diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index 14a0160..ead86db 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -78,7 +78,7 @@ struct vhost_memory {
 #define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
 /* Give up ownership, and reset the device to default values.
  * Allows subsequent call to VHOST_OWNER_SET to succeed. */
-#define VHOST_RESET_DEVICE _IO(VHOST_VIRTIO, 0x02)
+#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
 
 /* Set up/modify memory layout */
 #define VHOST_SET_MEM_TABLE	_IOW(VHOST_VIRTIO, 0x03, struct vhost_memory)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index a74c934..d2fc048 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -57,7 +57,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_FEATURES = 1,
     VHOST_USER_SET_FEATURES = 2,
     VHOST_USER_SET_OWNER = 3,
-    VHOST_USER_RESET_DEVICE = 4,
+    VHOST_USER_RESET_OWNER = 4,
     VHOST_USER_SET_MEM_TABLE = 5,
     VHOST_USER_SET_LOG_BASE = 6,
     VHOST_USER_SET_LOG_FD = 7,
@@ -307,7 +307,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
         g_cond_signal(&s->data_cond);
         break;
 
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         s->fds_num = 0;
         break;
 
-- 
1.9.0

[Qemu-devel] [PATCH v3 2/3] doc: vhost-user: request naming fix

[Yuanhan Liu]

They are VHOST_USER_XXX instead of VHOST_XXX messages.

Also, add VHOST_USER_GET_QUEUE_NUM to the section that
requries replies.

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 docs/specs/vhost-user.txt | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index d319715..0a5a346 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -112,20 +112,21 @@ The communication consists of master sending message requests and slave sending
 message replies. Most of the requests don't require replies. Here is a list of
 the ones that do:
 
- * VHOST_GET_FEATURES
- * VHOST_GET_PROTOCOL_FEATURES
- * VHOST_GET_VRING_BASE
- * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
+ * VHOST_USER_GET_FEATURES
+ * VHOST_USER_GET_PROTOCOL_FEATURES
+ * VHOST_USER_GET_VRING_BASE
+ * VHOST_USER_GET_QUEUE_NUM
+ * VHOST_USER_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
 
 There are several messages that the master sends with file descriptors passed
 in the ancillary data:
 
- * VHOST_SET_MEM_TABLE
- * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
- * VHOST_SET_LOG_FD
- * VHOST_SET_VRING_KICK
- * VHOST_SET_VRING_CALL
- * VHOST_SET_VRING_ERR
+ * VHOST_USER_SET_MEM_TABLE
+ * VHOST_USER_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
+ * VHOST_USER_SET_LOG_FD
+ * VHOST_USER_SET_VRING_KICK
+ * VHOST_USER_SET_VRING_CALL
+ * VHOST_USER_SET_VRING_ERR
 
 If Master is unable to send the full message or receives a wrong reply it will
 close the connection. An optional reconnection mechanism can be implemented.
-- 
1.9.0

[Qemu-devel] [PATCH v3 3/3] vhost-user-test: add multiple queue test

[Yuanhan Liu]

Setting VHOST_USER_PROTOCOL_F_MQ protocol feature bit to claim that we
support MQ feature, and simply assume we support 2 queue pairs at most.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 tests/vhost-user-test.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index d2fc048..50e6c75 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -30,11 +30,17 @@
 #define HAVE_MONOTONIC_TIME
 #endif
 
+#define MAX_QUEUES      2
+
+#define __STR(x)        #x
+#define STR(x)          __STR(x)
+
 #define QEMU_CMD_ACCEL  " -machine accel=tcg"
 #define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM,"\
                         "mem-path=%s,share=on -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s"
-#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
+#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce," \
+                        "queues=" STR(MAX_QUEUES)
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0 "
 #define QEMU_CMD_ROM    " -option-rom ../pc-bios/pxe-virtio.rom"
 
@@ -48,6 +54,7 @@
 #define VHOST_MEMORY_MAX_NREGIONS    8
 
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
+#define VHOST_USER_PROTOCOL_F_MQ        0
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
 
 #define VHOST_LOG_PAGE 0x1000
@@ -70,6 +77,8 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_VRING_ERR = 14,
     VHOST_USER_GET_PROTOCOL_FEATURES = 15,
     VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+    VHOST_USER_GET_QUEUE_NUM = 17,
+    VHOST_USER_SET_VRING_ENABLE = 18,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -258,7 +267,8 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
         /* send back features to qemu */
         msg.flags |= VHOST_USER_REPLY_MASK;
         msg.size = sizeof(m.u64);
-        msg.u64 = 1 << VHOST_USER_PROTOCOL_F_LOG_SHMFD;
+        msg.u64 = (1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD) |
+                  (1ULL << VHOST_USER_PROTOCOL_F_MQ);
         p = (uint8_t *) &msg;
         qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
         break;
@@ -307,6 +317,15 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
         g_cond_signal(&s->data_cond);
         break;
 
+    case VHOST_USER_GET_QUEUE_NUM:
+        /* send back the number of queues we support (let it be 2) to qemu */
+        msg.flags |= VHOST_USER_REPLY_MASK;
+        msg.size = sizeof(m.u64);
+        msg.u64 = MAX_QUEUES;
+        p = (uint8_t *) &msg;
+        qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
+        break;
+
     case VHOST_USER_RESET_OWNER:
         s->fds_num = 0;
         break;
-- 
1.9.0

Re: [Qemu-devel] [PATCH v3 1/3] vhost: rename VHOST_RESET_DEVICE back to VHOST_RESET_OWNER

[Michael S. Tsirkin]

On Fri, Oct 23, 2015 at 04:48:45PM +0800, Yuanhan Liu wrote:
> It turned out that it breaks stuff (VHOST_RESET_OWNER is not defined),
> so revert it:
> 
>     http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
> 
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Yes but just renaming is kind of beside the point.
We need to move it and add enable/disable vq commands
in its place.

> ---
>  docs/specs/vhost-user.txt   | 4 ++--
>  hw/virtio/vhost-backend.c   | 2 +-
>  hw/virtio/vhost-user.c      | 6 +++---
>  linux-headers/linux/vhost.h | 2 +-
>  tests/vhost-user-test.c     | 4 ++--
>  5 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index e0d71e2..d319715 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -255,10 +255,10 @@ Message types
>        as an owner of the session. This can be used on the Slave as a
>        "session start" flag.
>  
> - * VHOST_USER_RESET_DEVICE
> + * VHOST_USER_RESET_OWNER
>  
>        Id: 4
> -      Equivalent ioctl: VHOST_RESET_DEVICE
> +      Equivalent ioctl: VHOST_RESET_OWNER
>        Master payload: N/A
>  
>        Issued when a new connection is about to be closed. The Master will no
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 1d5f684..b734a60 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -156,7 +156,7 @@ static int vhost_kernel_set_owner(struct vhost_dev *dev)
>  
>  static int vhost_kernel_reset_device(struct vhost_dev *dev)
>  {
> -    return vhost_kernel_call(dev, VHOST_RESET_DEVICE, NULL);
> +    return vhost_kernel_call(dev, VHOST_RESET_OWNER, NULL);
>  }
>  
>  static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 78442ba..7c532f4 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -43,7 +43,7 @@ typedef enum VhostUserRequest {
>      VHOST_USER_GET_FEATURES = 1,
>      VHOST_USER_SET_FEATURES = 2,
>      VHOST_USER_SET_OWNER = 3,
> -    VHOST_USER_RESET_DEVICE = 4,
> +    VHOST_USER_RESET_OWNER = 4,
>      VHOST_USER_SET_MEM_TABLE = 5,
>      VHOST_USER_SET_LOG_BASE = 6,
>      VHOST_USER_SET_LOG_FD = 7,
> @@ -157,7 +157,7 @@ static bool vhost_user_one_time_request(VhostUserRequest request)
>  {
>      switch (request) {
>      case VHOST_USER_SET_OWNER:
> -    case VHOST_USER_RESET_DEVICE:
> +    case VHOST_USER_RESET_OWNER:
>      case VHOST_USER_SET_MEM_TABLE:
>      case VHOST_USER_GET_QUEUE_NUM:
>          return true;
> @@ -486,7 +486,7 @@ static int vhost_user_set_owner(struct vhost_dev *dev)
>  static int vhost_user_reset_device(struct vhost_dev *dev)
>  {
>      VhostUserMsg msg = {
> -        .request = VHOST_USER_RESET_DEVICE,
> +        .request = VHOST_USER_RESET_OWNER,
>          .flags = VHOST_USER_VERSION,
>      };
>  
> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
> index 14a0160..ead86db 100644
> --- a/linux-headers/linux/vhost.h
> +++ b/linux-headers/linux/vhost.h
> @@ -78,7 +78,7 @@ struct vhost_memory {
>  #define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
>  /* Give up ownership, and reset the device to default values.
>   * Allows subsequent call to VHOST_OWNER_SET to succeed. */
> -#define VHOST_RESET_DEVICE _IO(VHOST_VIRTIO, 0x02)
> +#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
>  
>  /* Set up/modify memory layout */
>  #define VHOST_SET_MEM_TABLE	_IOW(VHOST_VIRTIO, 0x03, struct vhost_memory)
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index a74c934..d2fc048 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -57,7 +57,7 @@ typedef enum VhostUserRequest {
>      VHOST_USER_GET_FEATURES = 1,
>      VHOST_USER_SET_FEATURES = 2,
>      VHOST_USER_SET_OWNER = 3,
> -    VHOST_USER_RESET_DEVICE = 4,
> +    VHOST_USER_RESET_OWNER = 4,
>      VHOST_USER_SET_MEM_TABLE = 5,
>      VHOST_USER_SET_LOG_BASE = 6,
>      VHOST_USER_SET_LOG_FD = 7,
> @@ -307,7 +307,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
>          g_cond_signal(&s->data_cond);
>          break;
>  
> -    case VHOST_USER_RESET_DEVICE:
> +    case VHOST_USER_RESET_OWNER:
>          s->fds_num = 0;
>          break;
>  
> -- 
> 1.9.0