* [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
@ 2015-10-16 7:18 Yuanhan Liu
2015-10-16 7:19 ` [Qemu-devel] [PATCH 2/3] doc: vhost-user: request naming fix Yuanhan Liu
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-16 7:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Yuanhan Liu, Michael S. Tsirkin
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" <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/net/vhost_net.c | 2 +-
hw/virtio/vhost-user.c | 8 ++++----
| 2 +-
tests/vhost-user-test.c | 2 +-
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 4eadad1..4bcd17d 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -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
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 2bce891..804f5c9 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -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);
}
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b11c0d2..12a9104 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -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:
--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 56df5cc..f181391 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -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,
--
1.9.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/3] doc: vhost-user: request naming fix
2015-10-16 7:18 [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Yuanhan Liu
@ 2015-10-16 7:19 ` Yuanhan Liu
2015-10-16 7:19 ` [Qemu-devel] [PATCH 3/3] vhost-user-test: add multiple queue test Yuanhan Liu
2015-10-16 7:21 ` [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Michael S. Tsirkin
2 siblings, 0 replies; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-16 7:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Yuanhan Liu, Michael S. Tsirkin
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 | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 4bcd17d..b23d88f 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -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.
--
1.9.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/3] vhost-user-test: add multiple queue test
2015-10-16 7:18 [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Yuanhan Liu
2015-10-16 7:19 ` [Qemu-devel] [PATCH 2/3] doc: vhost-user: request naming fix Yuanhan Liu
@ 2015-10-16 7:19 ` Yuanhan Liu
2015-10-16 7:20 ` Michael S. Tsirkin
2015-10-16 7:21 ` [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Michael S. Tsirkin
2 siblings, 1 reply; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-16 7:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Yuanhan Liu, Jason Wang, Michael S. Tsirkin
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 | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index f181391..f0aa36f 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -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;
}
--
1.9.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vhost-user-test: add multiple queue test
2015-10-16 7:19 ` [Qemu-devel] [PATCH 3/3] vhost-user-test: add multiple queue test Yuanhan Liu
@ 2015-10-16 7:20 ` Michael S. Tsirkin
2015-10-16 7:33 ` Yuanhan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-10-16 7:20 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Jason Wang, qemu-devel
On Fri, Oct 16, 2015 at 03:19:01PM +0800, Yuanhan Liu wrote:
> 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?
>
> 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 | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index f181391..f0aa36f 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -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;
> }
> --
> 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-16 7:18 [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Yuanhan Liu
2015-10-16 7:19 ` [Qemu-devel] [PATCH 2/3] doc: vhost-user: request naming fix Yuanhan Liu
2015-10-16 7:19 ` [Qemu-devel] [PATCH 3/3] vhost-user-test: add multiple queue test Yuanhan Liu
@ 2015-10-16 7:21 ` Michael S. Tsirkin
2015-10-16 7:32 ` Yuanhan Liu
2 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-10-16 7:21 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: qemu-devel
On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> 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" <mst@redhat.com>
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
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
> ---
> 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(-)
>
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 4eadad1..4bcd17d 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -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
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 2bce891..804f5c9 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -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);
> }
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index b11c0d2..12a9104 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -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:
> 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 56df5cc..f181391 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -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,
> --
> 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-16 7:21 ` [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Michael S. Tsirkin
@ 2015-10-16 7:32 ` Yuanhan Liu
2015-10-16 7:47 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-16 7:32 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Xie, Huawei, qemu-devel
On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > 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" <mst@redhat.com>
> > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> 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?
> - 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
>
> > ---
> > 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(-)
> >
> > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > index 4eadad1..4bcd17d 100644
> > --- a/docs/specs/vhost-user.txt
> > +++ b/docs/specs/vhost-user.txt
> > @@ -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
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 2bce891..804f5c9 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -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);
> > }
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index b11c0d2..12a9104 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -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:
> > 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 56df5cc..f181391 100644
> > --- a/tests/vhost-user-test.c
> > +++ b/tests/vhost-user-test.c
> > @@ -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,
> > --
> > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vhost-user-test: add multiple queue test
2015-10-16 7:20 ` Michael S. Tsirkin
@ 2015-10-16 7:33 ` Yuanhan Liu
0 siblings, 0 replies; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-16 7:33 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jason Wang, qemu-devel
On Fri, Oct 16, 2015 at 10:20:08AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 16, 2015 at 03:19:01PM +0800, Yuanhan Liu wrote:
> > 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
>
> >
> > 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 | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> > index f181391..f0aa36f 100644
> > --- a/tests/vhost-user-test.c
> > +++ b/tests/vhost-user-test.c
> > @@ -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;
> > }
> > --
> > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-16 7:32 ` Yuanhan Liu
@ 2015-10-16 7:47 ` Michael S. Tsirkin
2015-10-16 8:04 ` Yuanhan Liu
2015-10-21 8:55 ` Yuanhan Liu
0 siblings, 2 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-10-16 7:47 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Xie, Huawei, qemu-devel
On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > 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" <mst@redhat.com>
> > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > 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.
> 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.
> > - 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.
> >
> > > ---
> > > 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(-)
> > >
> > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > index 4eadad1..4bcd17d 100644
> > > --- a/docs/specs/vhost-user.txt
> > > +++ b/docs/specs/vhost-user.txt
> > > @@ -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
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 2bce891..804f5c9 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -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);
> > > }
> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > index b11c0d2..12a9104 100644
> > > --- a/hw/virtio/vhost-user.c
> > > +++ b/hw/virtio/vhost-user.c
> > > @@ -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:
> > > 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 56df5cc..f181391 100644
> > > --- a/tests/vhost-user-test.c
> > > +++ b/tests/vhost-user-test.c
> > > @@ -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,
> > > --
> > > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-16 7:47 ` Michael S. Tsirkin
@ 2015-10-16 8:04 ` Yuanhan Liu
2015-10-16 8:15 ` Michael S. Tsirkin
2015-10-21 8:55 ` Yuanhan Liu
1 sibling, 1 reply; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-16 8:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Xie, Huawei, qemu-devel
On Fri, Oct 16, 2015 at 10:47:49AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> > On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > > 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" <mst@redhat.com>
> > > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > >
> > > 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.
> > 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.
>
> > > - 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.
>
> 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
>
>
> > >
> > > > ---
> > > > 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(-)
> > > >
> > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > index 4eadad1..4bcd17d 100644
> > > > --- a/docs/specs/vhost-user.txt
> > > > +++ b/docs/specs/vhost-user.txt
> > > > @@ -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
> > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > index 2bce891..804f5c9 100644
> > > > --- a/hw/net/vhost_net.c
> > > > +++ b/hw/net/vhost_net.c
> > > > @@ -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);
> > > > }
> > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > index b11c0d2..12a9104 100644
> > > > --- a/hw/virtio/vhost-user.c
> > > > +++ b/hw/virtio/vhost-user.c
> > > > @@ -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:
> > > > 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 56df5cc..f181391 100644
> > > > --- a/tests/vhost-user-test.c
> > > > +++ b/tests/vhost-user-test.c
> > > > @@ -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,
> > > > --
> > > > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-16 8:04 ` Yuanhan Liu
@ 2015-10-16 8:15 ` Michael S. Tsirkin
2015-10-16 8:38 ` Yuanhan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-10-16 8:15 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Xie, Huawei, qemu-devel
On Fri, Oct 16, 2015 at 04:04:09PM +0800, Yuanhan Liu wrote:
> On Fri, Oct 16, 2015 at 10:47:49AM +0300, Michael S. Tsirkin wrote:
> > On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> > > On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > > > 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" <mst@redhat.com>
> > > > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > >
> > > > 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.
>
> > > 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.
Well if MQ is negotiated, then skipping this is clearly
the right thing to do.
> >
> > > > - 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.
>
> >
> > 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.
> >
> >
> > > >
> > > > > ---
> > > > > 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(-)
> > > > >
> > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > > index 4eadad1..4bcd17d 100644
> > > > > --- a/docs/specs/vhost-user.txt
> > > > > +++ b/docs/specs/vhost-user.txt
> > > > > @@ -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
> > > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > > index 2bce891..804f5c9 100644
> > > > > --- a/hw/net/vhost_net.c
> > > > > +++ b/hw/net/vhost_net.c
> > > > > @@ -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);
> > > > > }
> > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > > index b11c0d2..12a9104 100644
> > > > > --- a/hw/virtio/vhost-user.c
> > > > > +++ b/hw/virtio/vhost-user.c
> > > > > @@ -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:
> > > > > 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 56df5cc..f181391 100644
> > > > > --- a/tests/vhost-user-test.c
> > > > > +++ b/tests/vhost-user-test.c
> > > > > @@ -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,
> > > > > --
> > > > > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-16 8:15 ` Michael S. Tsirkin
@ 2015-10-16 8:38 ` Yuanhan Liu
0 siblings, 0 replies; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-16 8:38 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Xie, Huawei, qemu-devel
On Fri, Oct 16, 2015 at 11:15:23AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 16, 2015 at 04:04:09PM +0800, Yuanhan Liu wrote:
> > On Fri, Oct 16, 2015 at 10:47:49AM +0300, Michael S. Tsirkin wrote:
> > > On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> > > > On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > > > > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > > > > 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" <mst@redhat.com>
> > > > > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > > >
> > > > > 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.
> >
> > > > 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.
>
> 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.
> > >
> > > > > - 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.
> >
> > >
> > > 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.
That makes sense to me. Huawei, what do you think?
--yliu
> If MQ is there, you should just go by VHOST_USER_SET_VRING_ENABLE.
>
>
> > >
> > >
> > > > >
> > > > > > ---
> > > > > > 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(-)
> > > > > >
> > > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > > > index 4eadad1..4bcd17d 100644
> > > > > > --- a/docs/specs/vhost-user.txt
> > > > > > +++ b/docs/specs/vhost-user.txt
> > > > > > @@ -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
> > > > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > > > index 2bce891..804f5c9 100644
> > > > > > --- a/hw/net/vhost_net.c
> > > > > > +++ b/hw/net/vhost_net.c
> > > > > > @@ -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);
> > > > > > }
> > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > > > index b11c0d2..12a9104 100644
> > > > > > --- a/hw/virtio/vhost-user.c
> > > > > > +++ b/hw/virtio/vhost-user.c
> > > > > > @@ -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:
> > > > > > 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 56df5cc..f181391 100644
> > > > > > --- a/tests/vhost-user-test.c
> > > > > > +++ b/tests/vhost-user-test.c
> > > > > > @@ -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,
> > > > > > --
> > > > > > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-16 7:47 ` Michael S. Tsirkin
2015-10-16 8:04 ` Yuanhan Liu
@ 2015-10-21 8:55 ` Yuanhan Liu
2015-10-21 9:15 ` Michael S. Tsirkin
1 sibling, 1 reply; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-21 8:55 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Xie, Huawei, qemu-devel
On Fri, Oct 16, 2015 at 10:47:49AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> > On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > > 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" <mst@redhat.com>
> > > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > >
> > > 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.
>
> > 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
> It might be better than
> corrupting guest memory. And if we do, maybe we should keep
> the RESET_DEVICE name.
>
> > > - 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.
>
>
>
> > >
> > > > ---
> > > > 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(-)
> > > >
> > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > index 4eadad1..4bcd17d 100644
> > > > --- a/docs/specs/vhost-user.txt
> > > > +++ b/docs/specs/vhost-user.txt
> > > > @@ -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
> > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > index 2bce891..804f5c9 100644
> > > > --- a/hw/net/vhost_net.c
> > > > +++ b/hw/net/vhost_net.c
> > > > @@ -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);
> > > > }
> > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > index b11c0d2..12a9104 100644
> > > > --- a/hw/virtio/vhost-user.c
> > > > +++ b/hw/virtio/vhost-user.c
> > > > @@ -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:
> > > > 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 56df5cc..f181391 100644
> > > > --- a/tests/vhost-user-test.c
> > > > +++ b/tests/vhost-user-test.c
> > > > @@ -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,
> > > > --
> > > > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-21 8:55 ` Yuanhan Liu
@ 2015-10-21 9:15 ` Michael S. Tsirkin
2015-10-21 9:21 ` Yuanhan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-10-21 9:15 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Xie, Huawei, qemu-devel
On Wed, Oct 21, 2015 at 04:55:16PM +0800, Yuanhan Liu wrote:
> On Fri, Oct 16, 2015 at 10:47:49AM +0300, Michael S. Tsirkin wrote:
> > On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> > > On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > > > 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" <mst@redhat.com>
> > > > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > >
> > > > 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.
> >
> > > 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.
> --yliu
>
> > It might be better than
> > corrupting guest memory. And if we do, maybe we should keep
> > the RESET_DEVICE name.
> >
> > > > - 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.
> >
> >
> >
> > > >
> > > > > ---
> > > > > 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(-)
> > > > >
> > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > > index 4eadad1..4bcd17d 100644
> > > > > --- a/docs/specs/vhost-user.txt
> > > > > +++ b/docs/specs/vhost-user.txt
> > > > > @@ -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
> > > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > > index 2bce891..804f5c9 100644
> > > > > --- a/hw/net/vhost_net.c
> > > > > +++ b/hw/net/vhost_net.c
> > > > > @@ -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);
> > > > > }
> > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > > index b11c0d2..12a9104 100644
> > > > > --- a/hw/virtio/vhost-user.c
> > > > > +++ b/hw/virtio/vhost-user.c
> > > > > @@ -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:
> > > > > 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 56df5cc..f181391 100644
> > > > > --- a/tests/vhost-user-test.c
> > > > > +++ b/tests/vhost-user-test.c
> > > > > @@ -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,
> > > > > --
> > > > > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-21 9:15 ` Michael S. Tsirkin
@ 2015-10-21 9:21 ` Yuanhan Liu
2015-10-21 10:42 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Yuanhan Liu @ 2015-10-21 9:21 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Wed, Oct 21, 2015 at 12:15:52PM +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 21, 2015 at 04:55:16PM +0800, Yuanhan Liu wrote:
> > On Fri, Oct 16, 2015 at 10:47:49AM +0300, Michael S. Tsirkin wrote:
> > > On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> > > > On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > > > > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > > > > 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" <mst@redhat.com>
> > > > > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > > >
> > > > > 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.
> > >
> > > > 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.
Say, at virtio_net_reset()?
--yliu
> >
> > > It might be better than
> > > corrupting guest memory. And if we do, maybe we should keep
> > > the RESET_DEVICE name.
> > >
> > > > > - 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.
> > >
> > >
> > >
> > > > >
> > > > > > ---
> > > > > > 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(-)
> > > > > >
> > > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > > > index 4eadad1..4bcd17d 100644
> > > > > > --- a/docs/specs/vhost-user.txt
> > > > > > +++ b/docs/specs/vhost-user.txt
> > > > > > @@ -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
> > > > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > > > index 2bce891..804f5c9 100644
> > > > > > --- a/hw/net/vhost_net.c
> > > > > > +++ b/hw/net/vhost_net.c
> > > > > > @@ -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);
> > > > > > }
> > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > > > index b11c0d2..12a9104 100644
> > > > > > --- a/hw/virtio/vhost-user.c
> > > > > > +++ b/hw/virtio/vhost-user.c
> > > > > > @@ -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:
> > > > > > 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 56df5cc..f181391 100644
> > > > > > --- a/tests/vhost-user-test.c
> > > > > > +++ b/tests/vhost-user-test.c
> > > > > > @@ -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,
> > > > > > --
> > > > > > 1.9.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE"
2015-10-21 9:21 ` Yuanhan Liu
@ 2015-10-21 10:42 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2015-10-21 10:42 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: qemu-devel
On Wed, Oct 21, 2015 at 05:21:40PM +0800, Yuanhan Liu wrote:
> On Wed, Oct 21, 2015 at 12:15:52PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Oct 21, 2015 at 04:55:16PM +0800, Yuanhan Liu wrote:
> > > On Fri, Oct 16, 2015 at 10:47:49AM +0300, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 16, 2015 at 03:32:56PM +0800, Yuanhan Liu wrote:
> > > > > On Fri, Oct 16, 2015 at 10:21:40AM +0300, Michael S. Tsirkin wrote:
> > > > > > On Fri, Oct 16, 2015 at 03:18:59PM +0800, Yuanhan Liu wrote:
> > > > > > > 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" <mst@redhat.com>
> > > > > > > Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > > > >
> > > > > > 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.
> > > >
> > > > > 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.
>
> Say, at virtio_net_reset()?
>
> --yliu
Sounds good.
> > >
> > > > It might be better than
> > > > corrupting guest memory. And if we do, maybe we should keep
> > > > the RESET_DEVICE name.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-10-21 10:42 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-16 7:18 [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Yuanhan Liu
2015-10-16 7:19 ` [Qemu-devel] [PATCH 2/3] doc: vhost-user: request naming fix Yuanhan Liu
2015-10-16 7:19 ` [Qemu-devel] [PATCH 3/3] vhost-user-test: add multiple queue test Yuanhan Liu
2015-10-16 7:20 ` Michael S. Tsirkin
2015-10-16 7:33 ` Yuanhan Liu
2015-10-16 7:21 ` [Qemu-devel] [PATCH 1/3] Revert "vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE" Michael S. Tsirkin
2015-10-16 7:32 ` Yuanhan Liu
2015-10-16 7:47 ` Michael S. Tsirkin
2015-10-16 8:04 ` Yuanhan Liu
2015-10-16 8:15 ` Michael S. Tsirkin
2015-10-16 8:38 ` Yuanhan Liu
2015-10-21 8:55 ` Yuanhan Liu
2015-10-21 9:15 ` Michael S. Tsirkin
2015-10-21 9:21 ` Yuanhan Liu
2015-10-21 10:42 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.