Skip to contentSkip to conversation

Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"

PATCH Superseded Yuanhan Liu 2 participants 15 messages Oct 16, 2015 – Oct 21, 2015

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Yuanhan Liu

author
#0 lore raw

PATCH 1/3 Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
author
#1 lore raw

PATCH 2/3 doc: vhost-user: request naming fix

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 <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
---
 docs/specs/vhost-user.txt | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)
docs/specs/vhost-user.txt +9 -8
index 4bcd17d..b23d88f 100644
@@ -112,18 +112,19 @@ 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_USER_GET_FEATURES
+ * VHOST_USER_GET_PROTOCOL_FEATURES
+ * VHOST_USER_GET_VRING_BASE
+ * VHOST_USER_GET_QUEUE_NUM
 
 There are several messages that the master sends with file descriptors passed
 in the ancillary data:
 
- * VHOST_SET_MEM_TABLE
- * 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_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.
Signature · 1 line
1.9.0
author
#2 lore raw

PATCH 3/3 vhost-user-test: add multiple queue test

In reply to 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 <[email protected]>
Cc Jason Wang <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
---
 tests/vhost-user-test.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
tests/vhost-user-test.c +16 -2
index f181391..f0aa36f 100644
@@ -33,7 +33,7 @@
 #define QEMU_CMD_MEM    " -m 512 -object memory-backend-file,id=mem,size=512M,"\
                         "mem-path=%s,share=on -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=chr0,path=%s"
-#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=chr0,vhostforce"
+#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=chr0,vhostforce,queues=2"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0 "
 #define QEMU_CMD_ROM    " -option-rom ../pc-bios/pxe-virtio.rom"
 

@@ -48,6 +48,8 @@
 
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+#define VHOST_USER_PROTOCOL_F_MQ 0
+
 typedef enum VhostUserRequest {
     VHOST_USER_NONE = 0,
     VHOST_USER_GET_FEATURES = 1,

@@ -66,6 +68,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;
 

@@ -232,7 +236,7 @@ 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 = 0;
+        msg.u64 = (1ULL << VHOST_USER_PROTOCOL_F_MQ);
         p = (uint8_t *) &msg;
         qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
         break;

@@ -266,6 +270,16 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
          */
         qemu_set_nonblock(fd);
         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 = 2;
+        p = (uint8_t *) &msg;
+        qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
+        break;
+
     default:
         break;
     }
Signature · 1 line
1.9.0
#3 lore raw

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

Quoted 2 lines · 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.

Well - let's add a macro, or something?

Quoted 67 lines
Cc Michael S. Tsirkin <[email protected]>
Cc Jason Wang <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
---
 tests/vhost-user-test.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
tests/vhost-user-test.c +16 -2
index f181391..f0aa36f 100644
@@ -33,7 +33,7 @@
 #define QEMU_CMD_MEM    " -m 512 -object memory-backend-file,id=mem,size=512M,"\
                         "mem-path=%s,share=on -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=chr0,path=%s"
-#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=chr0,vhostforce"
+#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=chr0,vhostforce,queues=2"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0 "
 #define QEMU_CMD_ROM    " -option-rom ../pc-bios/pxe-virtio.rom"
 

@@ -48,6 +48,8 @@
 
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+#define VHOST_USER_PROTOCOL_F_MQ 0
+
 typedef enum VhostUserRequest {
     VHOST_USER_NONE = 0,
     VHOST_USER_GET_FEATURES = 1,

@@ -66,6 +68,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;
 

@@ -232,7 +236,7 @@ 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 = 0;
+        msg.u64 = (1ULL << VHOST_USER_PROTOCOL_F_MQ);
         p = (uint8_t *) &msg;
         qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
         break;

@@ -266,6 +270,16 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
          */
         qemu_set_nonblock(fd);
         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 = 2;
+        p = (uint8_t *) &msg;
+        qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
+        break;
+
     default:
         break;
     }
Signature · 1 line
1.9.0
author
#4 lore raw

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

Quoted 5 lines · Michael S. Tsirkin
Quoted 2 lines · 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.

Well - let's add a macro, or something?

Yeah, a macro is better.

	--yliu
Quoted 68 lines
Quoted 68 lines
Cc Michael S. Tsirkin <[email protected]>
Cc Jason Wang <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
---
 tests/vhost-user-test.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
tests/vhost-user-test.c +16 -2
index f181391..f0aa36f 100644
@@ -33,7 +33,7 @@
 #define QEMU_CMD_MEM    " -m 512 -object memory-backend-file,id=mem,size=512M,"\
                         "mem-path=%s,share=on -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=chr0,path=%s"
-#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=chr0,vhostforce"
+#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=chr0,vhostforce,queues=2"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0 "
 #define QEMU_CMD_ROM    " -option-rom ../pc-bios/pxe-virtio.rom"
 

@@ -48,6 +48,8 @@
 
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+#define VHOST_USER_PROTOCOL_F_MQ 0
+
 typedef enum VhostUserRequest {
     VHOST_USER_NONE = 0,
     VHOST_USER_GET_FEATURES = 1,

@@ -66,6 +68,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;
 

@@ -232,7 +236,7 @@ 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 = 0;
+        msg.u64 = (1ULL << VHOST_USER_PROTOCOL_F_MQ);
         p = (uint8_t *) &msg;
         qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
         break;

@@ -266,6 +270,16 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
          */
         qemu_set_nonblock(fd);
         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 = 2;
+        p = (uint8_t *) &msg;
+        qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
+        break;
+
     default:
         break;
     }
Signature · 1 line
1.9.0
#5 lore raw
In reply to Yuanhan Liu
Quoted 9 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>

OK but that's not enough. We need to also
- drop patch sending this on stop
- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

Quoted 106 lines
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
author
#6 lore raw
Quoted 5 lines · Michael S. Tsirkin
Quoted 4 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

Quoted 8 lines
Quoted 5 lines
    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop? If we don't send it on stop, when will we send it? The termination of QEMU?

Quoted 1 line

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu
Quoted 107 lines
Quoted 107 lines
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
#7 lore raw
Quoted 8 lines · Yuanhan Liu
Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload. It might be better than corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.

Quoted 7 lines
Quoted 1 line

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu

Unfortunately that's only there if MQ is negotiated.

If MQ is not negotiated, then I think you can get away with starting processing after you get the 1st kick.

Quoted 107 lines
Quoted 107 lines
Quoted 107 lines
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
author
#8 lore raw
Quoted 13 lines · Michael S. Tsirkin
Quoted 12 lines · Yuanhan Liu
Quoted 11 lines · Michael S. Tsirkin
Quoted 9 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Got it, and thanks for the explanation.

Quoted 7 lines
Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload. It might be better than corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.

Maybe we should just ignore it now, before I (we) have clear clue on how to handle it correctly.

Quoted 10 lines
Quoted 8 lines
Quoted 2 lines

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu

Unfortunately that's only there if MQ is negotiated.

Yes.

Quoted 3 lines

If MQ is not negotiated, then I think you can get away with starting processing after you get the 1st kick.

We mark a vring is ready by checking the callfd and kickfd:
if both are valid (> 0), then it's ready.

	--yliu
Quoted 109 lines
Quoted 109 lines
Quoted 109 lines
Quoted 109 lines


---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
#9 lore raw
Quoted 27 lines · Yuanhan Liu
Quoted 13 lines · Michael S. Tsirkin
Quoted 12 lines · Yuanhan Liu
Quoted 11 lines · Michael S. Tsirkin
Quoted 9 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Got it, and thanks for the explanation.

Quoted 3 lines
Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

Quoted 7 lines
Quoted 4 lines

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload. It might be better than corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.

Maybe we should just ignore it now, before I (we) have clear clue on how to handle it correctly.

Well if MQ is negotiated, then skipping this is clearly the right thing to do.

Quoted 14 lines
Quoted 10 lines
Quoted 8 lines
Quoted 2 lines

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu

Unfortunately that's only there if MQ is negotiated.

Yes.

Quoted 1 line
Quoted 7 lines
Quoted 2 lines

If MQ is not negotiated, then I think you can get away with starting processing after you get the 1st kick.

We mark a vring is ready by checking the callfd and kickfd:
if both are valid (> 0), then it's ready.

	--yliu

No, that's not right. If MQ is not there, you must not look at vring until you get at least 1 kick. In other words, until kickfd is readable. Whether callfd is set should not matter at all.
If MQ is there, you should just go by VHOST_USER_SET_VRING_ENABLE.

Quoted 109 lines
Quoted 109 lines
Quoted 109 lines
Quoted 109 lines
Quoted 109 lines


---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
author
#10 lore raw
Quoted 33 lines · Michael S. Tsirkin
Quoted 32 lines · Yuanhan Liu
Quoted 13 lines · Michael S. Tsirkin
Quoted 12 lines · Yuanhan Liu
Quoted 11 lines · Michael S. Tsirkin
Quoted 9 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Got it, and thanks for the explanation.

Quoted 7 lines
Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload. It might be better than corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.

Quoted 5 lines
Quoted 2 lines

Maybe we should just ignore it now, before I (we) have clear clue on how to handle it correctly.

Well if MQ is negotiated, then skipping this is clearly the right thing to do.

Let me put a bit more thoughts on this when get time.

Quoted 17 lines
Quoted 17 lines
Quoted 10 lines
Quoted 8 lines
Quoted 2 lines

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu

Unfortunately that's only there if MQ is negotiated.

Yes.

Quoted 3 lines

If MQ is not negotiated, then I think you can get away with starting processing after you get the 1st kick.

Quoted 8 lines
Quoted 4 lines

We mark a vring is ready by checking the callfd and kickfd:
if both are valid (> 0), then it's ready.

	--yliu

No, that's not right. If MQ is not there, you must not look at vring until you get at least 1 kick. In other words, until kickfd is readable. Whether callfd is set should not matter at all.

That makes sense to me. Huawei, what do you think?

	--yliu
Quoted 112 lines

If MQ is there, you should just go by VHOST_USER_SET_VRING_ENABLE.

Quoted 109 lines
Quoted 109 lines
Quoted 109 lines
Quoted 109 lines
Quoted 109 lines


---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
author
#11 lore raw
In reply to Michael S. Tsirkin
Quoted 18 lines · Michael S. Tsirkin
Quoted 8 lines · Yuanhan Liu
Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Quoted 8 lines

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload.

Sorry, what place do you mean exactly?

	--yliu
Quoted 126 lines

It might be better than
corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.

Quoted 7 lines
Quoted 1 line

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu

Unfortunately that's only there if MQ is negotiated.

If MQ is not negotiated, then I think you can get away with starting processing after you get the 1st kick.

Quoted 107 lines
Quoted 107 lines
Quoted 107 lines
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
#12 lore raw
Quoted 22 lines · Yuanhan Liu
Quoted 21 lines · Michael S. Tsirkin
Quoted 8 lines · Yuanhan Liu
Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload.

Sorry, what place do you mean exactly?

There's no such code - you'll have to add
a callback into vhost from virtio.

Quoted 128 lines
	--yliu
Quoted 126 lines

It might be better than
corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.

Quoted 7 lines
Quoted 1 line

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu

Unfortunately that's only there if MQ is negotiated.

If MQ is not negotiated, then I think you can get away with starting processing after you get the 1st kick.

Quoted 107 lines
Quoted 107 lines
Quoted 107 lines
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
author
#13 lore raw
Quoted 26 lines · Michael S. Tsirkin
Quoted 25 lines · Yuanhan Liu
Quoted 24 lines · Michael S. Tsirkin
Quoted 8 lines · Yuanhan Liu
Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload.

Sorry, what place do you mean exactly?

There's no such code - you'll have to add
a callback into vhost from virtio.

Say, at virtio_net_reset()?

	--yliu
Quoted 127 lines
Quoted 127 lines
Quoted 127 lines

It might be better than
corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.

Quoted 7 lines
Quoted 1 line

- send VHOST_USER_SET_VRING_ENABLE on stop and start instead

That indeed looks more reasonable. And actually, Huawei asked whether there is a message from QEMU to tell us that a vring is ready in our weekly meeting this morning. And here it is :)

	--yliu

Unfortunately that's only there if MQ is negotiated.

If MQ is not negotiated, then I think you can get away with starting processing after you get the 1st kick.

Quoted 107 lines
Quoted 107 lines
Quoted 107 lines
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 8 ++++----
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
docs/specs/vhost-user.txt +2 -2
index 4eadad1..4bcd17d 100644
@@ -211,10 +211,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
hw/net/vhost_net.c +1 -1
index 2bce891..804f5c9 100644
@@ -296,7 +296,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
                                           NULL);
             assert(r >= 0);
         }
hw/virtio/vhost-user.c +4 -4
index b11c0d2..12a9104 100644
@@ -34,7 +34,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,

@@ -102,7 +102,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
+    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */

@@ -192,7 +192,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;

@@ -249,7 +249,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_RESET_OWNER:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
linux-headers/linux/vhost.h +1 -1
index 14a0160..ead86db 100644
@@ -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)
tests/vhost-user-test.c +1 -1
index 56df5cc..f181391 100644
@@ -53,7 +53,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,
Signature · 1 line
1.9.0
#14 lore raw
Quoted 30 lines · Yuanhan Liu
Quoted 29 lines · Michael S. Tsirkin
Quoted 28 lines · Yuanhan Liu
Quoted 26 lines · Michael S. Tsirkin
Quoted 8 lines · Yuanhan Liu
Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Yuanhan Liu

This reverts commit d1f8b30ec8dde0318fd1b98d24a64926feae9625.

It turned out that it breaks stuff, so revert it:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg00949.html
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
CC "Michael S. Tsirkin" <[email protected]>
Reported-by Paolo Bonzini <[email protected]>
Signed-off-by Yuanhan Liu <[email protected]>

OK but that's not enough. We need to also
- drop patch sending this on stop

I could do that. But may I know why we have to drop it on stop?

Because we need to get the state from remote after stop. RESET_OWNER discards that, so we can't resume the VM.

Quoted 2 lines

If we don't send it on stop, when will we send it? The termination of QEMU?

If mq is negotiated, we don't need it. If mq isn't negotiated, maybe we should send it on reset/driver unload.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Sorry, what place do you mean exactly?

There's no such code - you'll have to add
a callback into vhost from virtio.

Say, at virtio_net_reset()?

	--yliu

Sounds good.

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

It might be better than
corrupting guest memory. And if we do, maybe we should keep the RESET_DEVICE name.