* [PATCH QEMU v2 3/3] vhost-user-blk-pci: introduce auto-num-queues property
2023-08-10 7:07 [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk ~hyman
@ 2023-08-05 0:39 ` ~hyman
2023-08-05 2:33 ` [PATCH QEMU v2 1/3] virtio-scsi-pci: " ~hyman
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: ~hyman @ 2023-08-05 0:39 UTC (permalink / raw)
To: qemu-devel
Cc: Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Fam Zheng, yong.huang
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Commit "a4eef0711b vhost-user-blk-pci: default num_queues to -smp N"
implment sizing the number of vhost-user-blk-pci request virtqueues
to match the number of vCPUs automatically. Which improves IO
preformance remarkably.
To enable this feature for the existing VMs, the cloud platform
may migrate VMs from the source hypervisor (num_queues is set to
1 by default) to the destination hypervisor (num_queues is set to
-smp N) lively. The different num-queues for vhost-user-blk-pci
devices between the source side and the destination side will
result in migration failure due to loading vmstate incorrectly
on the destination side.
To provide a smooth upgrade solution, introduce the
auto-num-queues property for the vhost-user-blk-pci device. This
allows upper APPs, e.g., libvirt, to recognize the hypervisor's
capability of allocating the virtqueues automatically by probing
the vhost-user-blk-pci.auto-num-queues property. Basing on which,
upper APPs can ensure to allocate the same num-queues on the
destination side in case of migration failure.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com>
---
hw/block/vhost-user-blk.c | 1 +
hw/virtio/vhost-user-blk-pci.c | 9 ++++++++-
include/hw/virtio/vhost-user-blk.h | 5 +++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index eecf3f7a81..34e23b1727 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -566,6 +566,7 @@ static const VMStateDescription vmstate_vhost_user_blk = {
static Property vhost_user_blk_properties[] = {
DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
+ DEFINE_PROP_BOOL("auto-num-queues", VHostUserBlk, auto_num_queues, true),
DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
VHOST_USER_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
index eef8641a98..f7776e928a 100644
--- a/hw/virtio/vhost-user-blk-pci.c
+++ b/hw/virtio/vhost-user-blk-pci.c
@@ -56,7 +56,14 @@ static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
DeviceState *vdev = DEVICE(&dev->vdev);
if (dev->vdev.num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
- dev->vdev.num_queues = virtio_pci_optimal_num_queues(0);
+ /*
+ * Allocate virtqueues automatically only if auto_num_queues
+ * property set true.
+ */
+ if (dev->vdev.auto_num_queues)
+ dev->vdev.num_queues = virtio_pci_optimal_num_queues(0);
+ else
+ dev->vdev.num_queues = 1;
}
if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
index ea085ee1ed..e6f0515bc6 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -50,6 +50,11 @@ struct VHostUserBlk {
bool connected;
/* vhost_user_blk_start/vhost_user_blk_stop */
bool started_vu;
+ /*
+ * Set to true if virtqueues allow to be allocated to
+ * match the number of virtual CPUs automatically.
+ */
+ bool auto_num_queues;
};
#endif
--
2.38.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH QEMU v2 1/3] virtio-scsi-pci: introduce auto-num-queues property
2023-08-10 7:07 [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk ~hyman
2023-08-05 0:39 ` [PATCH QEMU v2 3/3] vhost-user-blk-pci: introduce auto-num-queues property ~hyman
@ 2023-08-05 2:33 ` ~hyman
2023-08-05 2:38 ` [PATCH QEMU v2 2/3] virtio-blk-pci: " ~hyman
2023-08-10 18:33 ` [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk Stefan Hajnoczi
3 siblings, 0 replies; 8+ messages in thread
From: ~hyman @ 2023-08-05 2:33 UTC (permalink / raw)
To: qemu-devel
Cc: Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Fam Zheng, yong.huang
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Commit "6a55882284 virtio-scsi-pci: default num_queues to -smp N"
implment sizing the number of virtio-scsi-pci request virtqueues
to match the number of vCPUs automatically. Which improves IO
preformance remarkably.
To enable this feature for the existing VMs, the cloud platform
may migrate VMs from the source hypervisor (num_queues is set to
1 by default) to the destination hypervisor (num_queues is set to
-smp N) lively. The different num-queues for virtio-scsi-pci
devices between the source side and the destination side will
result in migration failure due to loading vmstate incorrectly
on the destination side.
To provide a smooth upgrade solution, introduce the
auto-num-queues property for the virtio-scsi-pci device. This
allows upper APPs, e.g., libvirt, to recognize the hypervisor's
capability of allocating the virtqueues automatically by probing
the virtio-scsi-pci.auto-num-queues property. Basing on which,
upper APPs can ensure to allocate the same num-queues on the
destination side in case of migration failure.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com>
---
hw/scsi/vhost-scsi.c | 2 ++
hw/scsi/vhost-user-scsi.c | 2 ++
hw/scsi/virtio-scsi.c | 2 ++
hw/virtio/vhost-scsi-pci.c | 11 +++++++++--
hw/virtio/vhost-user-scsi-pci.c | 11 +++++++++--
hw/virtio/virtio-scsi-pci.c | 11 +++++++++--
include/hw/virtio/virtio-scsi.h | 5 +++++
7 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 443f67daa4..78a8929c49 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -284,6 +284,8 @@ static Property vhost_scsi_properties[] = {
DEFINE_PROP_STRING("vhostfd", VirtIOSCSICommon, conf.vhostfd),
DEFINE_PROP_STRING("wwpn", VirtIOSCSICommon, conf.wwpn),
DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0),
+ DEFINE_PROP_BOOL("auto_num_queues", VirtIOSCSICommon, auto_num_queues,
+ true),
DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues,
VIRTIO_SCSI_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size,
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index ee99b19e7a..1b837f370a 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -161,6 +161,8 @@ static void vhost_user_scsi_unrealize(DeviceState *dev)
static Property vhost_user_scsi_properties[] = {
DEFINE_PROP_CHR("chardev", VirtIOSCSICommon, conf.chardev),
DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0),
+ DEFINE_PROP_BOOL("auto_num_queues", VirtIOSCSICommon, auto_num_queues,
+ true),
DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues,
VIRTIO_SCSI_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size,
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 45b95ea070..2ec13032aa 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -1279,6 +1279,8 @@ static void virtio_scsi_device_unrealize(DeviceState *dev)
}
static Property virtio_scsi_properties[] = {
+ DEFINE_PROP_BOOL("auto_num_queues", VirtIOSCSI, parent_obj.auto_num_queues,
+ true),
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
VIRTIO_SCSI_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
diff --git a/hw/virtio/vhost-scsi-pci.c b/hw/virtio/vhost-scsi-pci.c
index 08980bc23b..927c155278 100644
--- a/hw/virtio/vhost-scsi-pci.c
+++ b/hw/virtio/vhost-scsi-pci.c
@@ -51,8 +51,15 @@ static void vhost_scsi_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
VirtIOSCSIConf *conf = &dev->vdev.parent_obj.parent_obj.conf;
if (conf->num_queues == VIRTIO_SCSI_AUTO_NUM_QUEUES) {
- conf->num_queues =
- virtio_pci_optimal_num_queues(VIRTIO_SCSI_VQ_NUM_FIXED);
+ /*
+ * Allocate virtqueues automatically only if auto_num_queues
+ * property set true.
+ */
+ if (dev->vdev.parent_obj.parent_obj.auto_num_queues)
+ conf->num_queues =
+ virtio_pci_optimal_num_queues(VIRTIO_SCSI_VQ_NUM_FIXED);
+ else
+ conf->num_queues = 1;
}
if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
diff --git a/hw/virtio/vhost-user-scsi-pci.c b/hw/virtio/vhost-user-scsi-pci.c
index 75882e3cf9..9c521a7f93 100644
--- a/hw/virtio/vhost-user-scsi-pci.c
+++ b/hw/virtio/vhost-user-scsi-pci.c
@@ -57,8 +57,15 @@ static void vhost_user_scsi_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
VirtIOSCSIConf *conf = &dev->vdev.parent_obj.parent_obj.conf;
if (conf->num_queues == VIRTIO_SCSI_AUTO_NUM_QUEUES) {
- conf->num_queues =
- virtio_pci_optimal_num_queues(VIRTIO_SCSI_VQ_NUM_FIXED);
+ /*
+ * Allocate virtqueues automatically only if auto_num_queues
+ * property set true.
+ */
+ if (dev->vdev.parent_obj.parent_obj.auto_num_queues)
+ conf->num_queues =
+ virtio_pci_optimal_num_queues(VIRTIO_SCSI_VQ_NUM_FIXED);
+ else
+ conf->num_queues = 1;
}
if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
diff --git a/hw/virtio/virtio-scsi-pci.c b/hw/virtio/virtio-scsi-pci.c
index e8e3442f38..7551857f90 100644
--- a/hw/virtio/virtio-scsi-pci.c
+++ b/hw/virtio/virtio-scsi-pci.c
@@ -52,8 +52,15 @@ static void virtio_scsi_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
char *bus_name;
if (conf->num_queues == VIRTIO_SCSI_AUTO_NUM_QUEUES) {
- conf->num_queues =
- virtio_pci_optimal_num_queues(VIRTIO_SCSI_VQ_NUM_FIXED);
+ /*
+ * Allocate virtqueues automatically only if auto_num_queues
+ * property set true.
+ */
+ if (dev->vdev.parent_obj.auto_num_queues)
+ conf->num_queues =
+ virtio_pci_optimal_num_queues(VIRTIO_SCSI_VQ_NUM_FIXED);
+ else
+ conf->num_queues = 1;
}
if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 779568ab5d..2660bbad22 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -72,6 +72,11 @@ struct VirtIOSCSICommon {
VirtQueue *ctrl_vq;
VirtQueue *event_vq;
VirtQueue **cmd_vqs;
+ /*
+ * Set to true if virtqueues allow to be allocated to
+ * match the number of virtual CPUs automatically.
+ */
+ bool auto_num_queues;
};
struct VirtIOSCSIReq;
--
2.38.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH QEMU v2 2/3] virtio-blk-pci: introduce auto-num-queues property
2023-08-10 7:07 [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk ~hyman
2023-08-05 0:39 ` [PATCH QEMU v2 3/3] vhost-user-blk-pci: introduce auto-num-queues property ~hyman
2023-08-05 2:33 ` [PATCH QEMU v2 1/3] virtio-scsi-pci: " ~hyman
@ 2023-08-05 2:38 ` ~hyman
2023-08-10 18:33 ` [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk Stefan Hajnoczi
3 siblings, 0 replies; 8+ messages in thread
From: ~hyman @ 2023-08-05 2:38 UTC (permalink / raw)
To: qemu-devel
Cc: Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
Stefan Hajnoczi, Paolo Bonzini, Fam Zheng, yong.huang
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Commit "9445e1e15 virtio-blk-pci: default num_queues to -smp N"
implment sizing the number of virtio-blk-pci request virtqueues
to match the number of vCPUs automatically. Which improves IO
preformance remarkably.
To enable this feature for the existing VMs, the cloud platform
may migrate VMs from the source hypervisor (num_queues is set to
1 by default) to the destination hypervisor (num_queues is set to
-smp N) lively. The different num-queues for virtio-blk-pci
devices between the source side and the destination side will
result in migration failure due to loading vmstate incorrectly
on the destination side.
To provide a smooth upgrade solution, introduce the
auto-num-queues property for the virtio-blk-pci device. This
allows upper APPs, e.g., libvirt, to recognize the hypervisor's
capability of allocating the virtqueues automatically by probing
the virtio-blk-pci.auto-num-queues property. Basing on which,
upper APPs can ensure to allocate the same num-queues on the
destination side in case of migration failure.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com>
---
hw/block/virtio-blk.c | 1 +
hw/virtio/virtio-blk-pci.c | 9 ++++++++-
include/hw/virtio/virtio-blk.h | 5 +++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 39e7f23fab..9e498ca64a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1716,6 +1716,7 @@ static Property virtio_blk_properties[] = {
#endif
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
+ DEFINE_PROP_BOOL("auto-num-queues", VirtIOBlock, auto_num_queues, true),
DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues,
VIRTIO_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
diff --git a/hw/virtio/virtio-blk-pci.c b/hw/virtio/virtio-blk-pci.c
index 9743bee965..4b6b4c4933 100644
--- a/hw/virtio/virtio-blk-pci.c
+++ b/hw/virtio/virtio-blk-pci.c
@@ -54,7 +54,14 @@ static void virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
VirtIOBlkConf *conf = &dev->vdev.conf;
if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) {
- conf->num_queues = virtio_pci_optimal_num_queues(0);
+ /*
+ * Allocate virtqueues automatically only if auto_num_queues
+ * property set true.
+ */
+ if (dev->vdev.auto_num_queues)
+ conf->num_queues = virtio_pci_optimal_num_queues(0);
+ else
+ conf->num_queues = 1;
}
if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index dafec432ce..dab6d7c70c 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -65,6 +65,11 @@ struct VirtIOBlock {
uint64_t host_features;
size_t config_size;
BlockRAMRegistrar blk_ram_registrar;
+ /*
+ * Set to true if virtqueues allow to be allocated to
+ * match the number of virtual CPUs automatically.
+ */
+ bool auto_num_queues;
};
typedef struct VirtIOBlockReq {
--
2.38.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk
2023-08-10 7:07 [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk ~hyman
` (2 preceding siblings ...)
2023-08-05 2:38 ` [PATCH QEMU v2 2/3] virtio-blk-pci: " ~hyman
@ 2023-08-10 18:33 ` Stefan Hajnoczi
2023-08-11 2:31 ` Yong Huang
3 siblings, 1 reply; 8+ messages in thread
From: Stefan Hajnoczi @ 2023-08-10 18:33 UTC (permalink / raw)
To: ~hyman
Cc: qemu-devel, Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf,
Hanna Reitz, Paolo Bonzini, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 4895 bytes --]
On Thu, Aug 10, 2023 at 07:07:09AM +0000, ~hyman wrote:
> Ping,
>
> This version is a copy of version 1 and is rebased
> on the master. No functional changes.
>
> A 1:1 virtqueue:vCPU mapping implementation for virtio-*-pci disk
> introduced since qemu >= 5.2.0, which improves IO performance
> remarkably. To enjoy this feature for exiting running VMs without
> service interruption, the common solution is to migrate VMs from the
> lower version of the hypervisor to the upgraded hypervisor, then wait
> for the next cold reboot of the VM to enable this feature. That's the
> way "discard" and "write-zeroes" features work.
>
> As to multi-queues disk allocation automatically, it's a little
> different because the destination will allocate queues to match the
> number of vCPUs automatically by default in the case of live migration,
> and the VMs on the source side remain 1 queue by default, which results
> in migration failure due to loading disk VMState incorrectly on the
> destination side.
Are you using QEMU's versioned machine types to freeze the VM
configuration?
If not, then live migration won't work reliably because you're migrating
between two potentially different VM configurations. This issue is not
specific to num-queues, it affects all device properties.
In commit 9445e1e15e66c19e42bea942ba810db28052cd05 ("virtio-blk-pci:
default num_queues to -smp N") the num_queues property is set to 1 for
versioned machine types <=5.1:
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 9ee2aa0f7b..7f65fa8743 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -31,6 +31,7 @@
GlobalProperty hw_compat_5_1[] = {
{ "vhost-scsi", "num_queues", "1"},
{ "vhost-user-scsi", "num_queues", "1"},
+ { "virtio-blk-device", "num-queues", "1"},
{ "virtio-scsi-device", "num_queues", "1"},
};
const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
Live migration works when the source and destination QEMU are launched
with the same versioned machine type. You can check the "info qtree"
output to confirm that starting a VM with -smp 4 -M pc-q35-5.1 results
in num-queues=1 while -smp 4 -M pc-q35-5.2 results in num-queues=4.
> This issue requires Qemu to provide a hint that shows
> multi-queues disk allocation is automatically supported, and this allows
> upper APPs, e.g., libvirt, to recognize the hypervisor's capability of
> this. And upper APPs can ensure to allocate the same num-queues on the
> destination side in case of migration failure.
>
> To fix the issue, we introduce the auto-num-queues property for
> virtio-*-pci as a solution, which would be probed by APPs, e.g., libvirt
> by querying the device properties of QEMU. When launching live
> migration, libvirt will send the auto-num-queues property as a migration
> cookie to the destination, and thus the destination knows if the source
> side supports auto-num-queues. If not, the destination would switch off
> by building the command line with "auto-num-queues=off" when preparing
> the incoming VM process. The following patches of libvirt show how it
> roughly works:
> https://github.com/newfriday/libvirt/commit/ce2bae2e1a6821afeb80756dc01f3680f525e506
> https://github.com/newfriday/libvirt/commit/f546972b009458c88148fe079544db7e9e1f43c3
> https://github.com/newfriday/libvirt/commit/5ee19c8646fdb4d87ab8b93f287c20925268ce83
>
> The smooth upgrade solution requires the introduction of the auto-num-
> queues property on the QEMU side, which is what the patch set does. I'm
> hoping for comments about the series.
Please take a look at versioned machine types. I think auto-num-queues
is not necessary if you use versioned machine types.
If you do think auto-num-queues is needed, please explain the issue in
more detail and state why versioned machine types don't help.
Thanks,
Stefan
>
> Please review, thanks.
> Yong
>
> Hyman Huang(黄勇) (3):
> virtio-scsi-pci: introduce auto-num-queues property
> virtio-blk-pci: introduce auto-num-queues property
> vhost-user-blk-pci: introduce auto-num-queues property
>
> hw/block/vhost-user-blk.c | 1 +
> hw/block/virtio-blk.c | 1 +
> hw/scsi/vhost-scsi.c | 2 ++
> hw/scsi/vhost-user-scsi.c | 2 ++
> hw/scsi/virtio-scsi.c | 2 ++
> hw/virtio/vhost-scsi-pci.c | 11 +++++++++--
> hw/virtio/vhost-user-blk-pci.c | 9 ++++++++-
> hw/virtio/vhost-user-scsi-pci.c | 11 +++++++++--
> hw/virtio/virtio-blk-pci.c | 9 ++++++++-
> hw/virtio/virtio-scsi-pci.c | 11 +++++++++--
> include/hw/virtio/vhost-user-blk.h | 5 +++++
> include/hw/virtio/virtio-blk.h | 5 +++++
> include/hw/virtio/virtio-scsi.h | 5 +++++
> 13 files changed, 66 insertions(+), 8 deletions(-)
>
> --
> 2.38.5
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk
2023-08-10 18:33 ` [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk Stefan Hajnoczi
@ 2023-08-11 2:31 ` Yong Huang
2023-08-14 13:18 ` Stefan Hajnoczi
0 siblings, 1 reply; 8+ messages in thread
From: Yong Huang @ 2023-08-11 2:31 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf,
Hanna Reitz, Paolo Bonzini, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 6157 bytes --]
Hi, Stefan, thank you for your interest in this series.
I'm trying to explain my point, if you think my explanation
doesn't stand up, please let me know.
On Fri, Aug 11, 2023 at 2:33 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Thu, Aug 10, 2023 at 07:07:09AM +0000, ~hyman wrote:
> > Ping,
> >
> > This version is a copy of version 1 and is rebased
> > on the master. No functional changes.
> >
> > A 1:1 virtqueue:vCPU mapping implementation for virtio-*-pci disk
> > introduced since qemu >= 5.2.0, which improves IO performance
> > remarkably. To enjoy this feature for exiting running VMs without
> > service interruption, the common solution is to migrate VMs from the
> > lower version of the hypervisor to the upgraded hypervisor, then wait
> > for the next cold reboot of the VM to enable this feature. That's the
> > way "discard" and "write-zeroes" features work.
> >
> > As to multi-queues disk allocation automatically, it's a little
> > different because the destination will allocate queues to match the
> > number of vCPUs automatically by default in the case of live migration,
> > and the VMs on the source side remain 1 queue by default, which results
> > in migration failure due to loading disk VMState incorrectly on the
> > destination side.
>
> Are you using QEMU's versioned machine types to freeze the VM
> configuration?
> If not, then live migration won't work reliably because you're migrating
> between two potentially different VM configurations. This issue is not
> specific to num-queues, it affects all device properties.
>
> In commit 9445e1e15e66c19e42bea942ba810db28052cd05 ("virtio-blk-pci:
> default num_queues to -smp N") the num_queues property is set to 1 for
> versioned machine types <=5.1:
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 9ee2aa0f7b..7f65fa8743 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -31,6 +31,7 @@
> GlobalProperty hw_compat_5_1[] = {
> { "vhost-scsi", "num_queues", "1"},
> { "vhost-user-scsi", "num_queues", "1"},
> + { "virtio-blk-device", "num-queues", "1"},
> { "virtio-scsi-device", "num_queues", "1"},
> };
> const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
>
> Live migration works when the source and destination QEMU are launched
> with the same versioned machine type. You can check the "info qtree"
> output to confirm that starting a VM with -smp 4 -M pc-q35-5.1 results
> in num-queues=1 while -smp 4 -M pc-q35-5.2 results in num-queues=4.
>
> > This issue requires Qemu to provide a hint that shows
> > multi-queues disk allocation is automatically supported, and this allows
> > upper APPs, e.g., libvirt, to recognize the hypervisor's capability of
> > this. And upper APPs can ensure to allocate the same num-queues on the
> > destination side in case of migration failure.
> >
> > To fix the issue, we introduce the auto-num-queues property for
> > virtio-*-pci as a solution, which would be probed by APPs, e.g., libvirt
> > by querying the device properties of QEMU. When launching live
> > migration, libvirt will send the auto-num-queues property as a migration
> > cookie to the destination, and thus the destination knows if the source
> > side supports auto-num-queues. If not, the destination would switch off
> > by building the command line with "auto-num-queues=off" when preparing
> > the incoming VM process. The following patches of libvirt show how it
> > roughly works:
> >
> https://github.com/newfriday/libvirt/commit/ce2bae2e1a6821afeb80756dc01f3680f525e506
> >
> https://github.com/newfriday/libvirt/commit/f546972b009458c88148fe079544db7e9e1f43c3
> >
> https://github.com/newfriday/libvirt/commit/5ee19c8646fdb4d87ab8b93f287c20925268ce83
> >
> > The smooth upgrade solution requires the introduction of the auto-num-
> > queues property on the QEMU side, which is what the patch set does. I'm
> > hoping for comments about the series.
>
> Please take a look at versioned machine types. I think auto-num-queues
> is not necessary if you use versioned machine types.
>
> If you do think auto-num-queues is needed, please explain the issue in
> more detail and state why versioned machine types don't help.
"Using the versioned machine types" is indeed the standard way to ensure
the proper functioning of live migration.
However, a stable version is strongly advised to maintain function in our
production environment and perhaps practically all the production
environments in other businesses. As a result, we must backport features
like "auto-allocation num-queues" while keeping the machine type the same.
This patch set is posted for that reason. The "feature-backport" scenario
is its target. I'm not sure if the upstream development strategy should
take this scenario into account; if it does, perhaps this patch set can be
of use. After all, the primary goal of this patch set is to broaden the uses
for this feature and essentially makes no functional changes.
> Thanks,
> Stefan
>
> >
> > Please review, thanks.
> > Yong
> >
> > Hyman Huang(黄勇) (3):
> > virtio-scsi-pci: introduce auto-num-queues property
> > virtio-blk-pci: introduce auto-num-queues property
> > vhost-user-blk-pci: introduce auto-num-queues property
> >
> > hw/block/vhost-user-blk.c | 1 +
> > hw/block/virtio-blk.c | 1 +
> > hw/scsi/vhost-scsi.c | 2 ++
> > hw/scsi/vhost-user-scsi.c | 2 ++
> > hw/scsi/virtio-scsi.c | 2 ++
> > hw/virtio/vhost-scsi-pci.c | 11 +++++++++--
> > hw/virtio/vhost-user-blk-pci.c | 9 ++++++++-
> > hw/virtio/vhost-user-scsi-pci.c | 11 +++++++++--
> > hw/virtio/virtio-blk-pci.c | 9 ++++++++-
> > hw/virtio/virtio-scsi-pci.c | 11 +++++++++--
> > include/hw/virtio/vhost-user-blk.h | 5 +++++
> > include/hw/virtio/virtio-blk.h | 5 +++++
> > include/hw/virtio/virtio-scsi.h | 5 +++++
> > 13 files changed, 66 insertions(+), 8 deletions(-)
> >
> > --
> > 2.38.5
> >
>
--
Best regards
[-- Attachment #2: Type: text/html, Size: 10243 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk
2023-08-11 2:31 ` Yong Huang
@ 2023-08-14 13:18 ` Stefan Hajnoczi
2023-08-15 1:37 ` Yong Huang
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Hajnoczi @ 2023-08-14 13:18 UTC (permalink / raw)
To: Yong Huang
Cc: qemu-devel, Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf,
Hanna Reitz, Paolo Bonzini, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 7273 bytes --]
On Fri, Aug 11, 2023 at 10:31:51AM +0800, Yong Huang wrote:
> Hi, Stefan, thank you for your interest in this series.
> I'm trying to explain my point, if you think my explanation
> doesn't stand up, please let me know.
>
> On Fri, Aug 11, 2023 at 2:33 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> > On Thu, Aug 10, 2023 at 07:07:09AM +0000, ~hyman wrote:
> > > Ping,
> > >
> > > This version is a copy of version 1 and is rebased
> > > on the master. No functional changes.
> > >
> > > A 1:1 virtqueue:vCPU mapping implementation for virtio-*-pci disk
> > > introduced since qemu >= 5.2.0, which improves IO performance
> > > remarkably. To enjoy this feature for exiting running VMs without
> > > service interruption, the common solution is to migrate VMs from the
> > > lower version of the hypervisor to the upgraded hypervisor, then wait
> > > for the next cold reboot of the VM to enable this feature. That's the
> > > way "discard" and "write-zeroes" features work.
> > >
> > > As to multi-queues disk allocation automatically, it's a little
> > > different because the destination will allocate queues to match the
> > > number of vCPUs automatically by default in the case of live migration,
> > > and the VMs on the source side remain 1 queue by default, which results
> > > in migration failure due to loading disk VMState incorrectly on the
> > > destination side.
> >
> > Are you using QEMU's versioned machine types to freeze the VM
> > configuration?
>
>
> > If not, then live migration won't work reliably because you're migrating
> > between two potentially different VM configurations. This issue is not
> > specific to num-queues, it affects all device properties.
> >
> > In commit 9445e1e15e66c19e42bea942ba810db28052cd05 ("virtio-blk-pci:
> > default num_queues to -smp N") the num_queues property is set to 1 for
> > versioned machine types <=5.1:
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 9ee2aa0f7b..7f65fa8743 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -31,6 +31,7 @@
> > GlobalProperty hw_compat_5_1[] = {
> > { "vhost-scsi", "num_queues", "1"},
> > { "vhost-user-scsi", "num_queues", "1"},
> > + { "virtio-blk-device", "num-queues", "1"},
> > { "virtio-scsi-device", "num_queues", "1"},
> > };
> > const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
> >
> > Live migration works when the source and destination QEMU are launched
> > with the same versioned machine type. You can check the "info qtree"
> > output to confirm that starting a VM with -smp 4 -M pc-q35-5.1 results
> > in num-queues=1 while -smp 4 -M pc-q35-5.2 results in num-queues=4.
> >
> > > This issue requires Qemu to provide a hint that shows
> > > multi-queues disk allocation is automatically supported, and this allows
> > > upper APPs, e.g., libvirt, to recognize the hypervisor's capability of
> > > this. And upper APPs can ensure to allocate the same num-queues on the
> > > destination side in case of migration failure.
> > >
> > > To fix the issue, we introduce the auto-num-queues property for
> > > virtio-*-pci as a solution, which would be probed by APPs, e.g., libvirt
> > > by querying the device properties of QEMU. When launching live
> > > migration, libvirt will send the auto-num-queues property as a migration
> > > cookie to the destination, and thus the destination knows if the source
> > > side supports auto-num-queues. If not, the destination would switch off
> > > by building the command line with "auto-num-queues=off" when preparing
> > > the incoming VM process. The following patches of libvirt show how it
> > > roughly works:
> > >
> > https://github.com/newfriday/libvirt/commit/ce2bae2e1a6821afeb80756dc01f3680f525e506
> > >
> > https://github.com/newfriday/libvirt/commit/f546972b009458c88148fe079544db7e9e1f43c3
> > >
> > https://github.com/newfriday/libvirt/commit/5ee19c8646fdb4d87ab8b93f287c20925268ce83
> > >
> > > The smooth upgrade solution requires the introduction of the auto-num-
> > > queues property on the QEMU side, which is what the patch set does. I'm
> > > hoping for comments about the series.
> >
> > Please take a look at versioned machine types. I think auto-num-queues
> > is not necessary if you use versioned machine types.
> >
> > If you do think auto-num-queues is needed, please explain the issue in
> > more detail and state why versioned machine types don't help.
>
>
> "Using the versioned machine types" is indeed the standard way to ensure
> the proper functioning of live migration.
>
> However, a stable version is strongly advised to maintain function in our
> production environment and perhaps practically all the production
> environments in other businesses. As a result, we must backport features
> like "auto-allocation num-queues" while keeping the machine type the same.
>
> This patch set is posted for that reason. The "feature-backport" scenario
> is its target. I'm not sure if the upstream development strategy should
> take this scenario into account; if it does, perhaps this patch set can be
> of use. After all, the primary goal of this patch set is to broaden the uses
> for this feature and essentially makes no functional changes.
Downstreams that backport only a subset of patches instead of following
upstream linearly must define their own versioned machine types since
upstream versioned machine types are not relevant downstream.
For example, here is how CentOS Stream defines its own machine types:
https://gitlab.com/redhat/centos-stream/src/qemu-kvm/-/commit/318178778db60b6475d1484509bee136317156d3
I think it's nicer for downstreams to define custom versioned machine
types than to add properties like auto-num-queues that are intended for
downstream usage and don't serve a purpose upstream.
My suggestion is for you to maintain your own custom versioned machine
types so you can systematically control device properties across
versions. Does that make sense?
Stefan
>
>
>
>
> > Thanks,
> > Stefan
> >
> > >
> > > Please review, thanks.
> > > Yong
> > >
> > > Hyman Huang(黄勇) (3):
> > > virtio-scsi-pci: introduce auto-num-queues property
> > > virtio-blk-pci: introduce auto-num-queues property
> > > vhost-user-blk-pci: introduce auto-num-queues property
> > >
> > > hw/block/vhost-user-blk.c | 1 +
> > > hw/block/virtio-blk.c | 1 +
> > > hw/scsi/vhost-scsi.c | 2 ++
> > > hw/scsi/vhost-user-scsi.c | 2 ++
> > > hw/scsi/virtio-scsi.c | 2 ++
> > > hw/virtio/vhost-scsi-pci.c | 11 +++++++++--
> > > hw/virtio/vhost-user-blk-pci.c | 9 ++++++++-
> > > hw/virtio/vhost-user-scsi-pci.c | 11 +++++++++--
> > > hw/virtio/virtio-blk-pci.c | 9 ++++++++-
> > > hw/virtio/virtio-scsi-pci.c | 11 +++++++++--
> > > include/hw/virtio/vhost-user-blk.h | 5 +++++
> > > include/hw/virtio/virtio-blk.h | 5 +++++
> > > include/hw/virtio/virtio-scsi.h | 5 +++++
> > > 13 files changed, 66 insertions(+), 8 deletions(-)
> > >
> > > --
> > > 2.38.5
> > >
> >
>
>
> --
> Best regards
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH QEMU v2 0/3] provide a smooth upgrade solution for multi-queues disk
2023-08-14 13:18 ` Stefan Hajnoczi
@ 2023-08-15 1:37 ` Yong Huang
0 siblings, 0 replies; 8+ messages in thread
From: Yong Huang @ 2023-08-15 1:37 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf,
Hanna Reitz, Paolo Bonzini, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 7980 bytes --]
On Tue, Aug 15, 2023 at 12:28 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:
> On Fri, Aug 11, 2023 at 10:31:51AM +0800, Yong Huang wrote:
> > Hi, Stefan, thank you for your interest in this series.
> > I'm trying to explain my point, if you think my explanation
> > doesn't stand up, please let me know.
> >
> > On Fri, Aug 11, 2023 at 2:33 AM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> >
> > > On Thu, Aug 10, 2023 at 07:07:09AM +0000, ~hyman wrote:
> > > > Ping,
> > > >
> > > > This version is a copy of version 1 and is rebased
> > > > on the master. No functional changes.
> > > >
> > > > A 1:1 virtqueue:vCPU mapping implementation for virtio-*-pci disk
> > > > introduced since qemu >= 5.2.0, which improves IO performance
> > > > remarkably. To enjoy this feature for exiting running VMs without
> > > > service interruption, the common solution is to migrate VMs from the
> > > > lower version of the hypervisor to the upgraded hypervisor, then wait
> > > > for the next cold reboot of the VM to enable this feature. That's the
> > > > way "discard" and "write-zeroes" features work.
> > > >
> > > > As to multi-queues disk allocation automatically, it's a little
> > > > different because the destination will allocate queues to match the
> > > > number of vCPUs automatically by default in the case of live
> migration,
> > > > and the VMs on the source side remain 1 queue by default, which
> results
> > > > in migration failure due to loading disk VMState incorrectly on the
> > > > destination side.
> > >
> > > Are you using QEMU's versioned machine types to freeze the VM
> > > configuration?
> >
> >
> > > If not, then live migration won't work reliably because you're
> migrating
> > > between two potentially different VM configurations. This issue is not
> > > specific to num-queues, it affects all device properties.
> > >
> > > In commit 9445e1e15e66c19e42bea942ba810db28052cd05 ("virtio-blk-pci:
> > > default num_queues to -smp N") the num_queues property is set to 1 for
> > > versioned machine types <=5.1:
> > >
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index 9ee2aa0f7b..7f65fa8743 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -31,6 +31,7 @@
> > > GlobalProperty hw_compat_5_1[] = {
> > > { "vhost-scsi", "num_queues", "1"},
> > > { "vhost-user-scsi", "num_queues", "1"},
> > > + { "virtio-blk-device", "num-queues", "1"},
> > > { "virtio-scsi-device", "num_queues", "1"},
> > > };
> > > const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
> > >
> > > Live migration works when the source and destination QEMU are launched
> > > with the same versioned machine type. You can check the "info qtree"
> > > output to confirm that starting a VM with -smp 4 -M pc-q35-5.1 results
> > > in num-queues=1 while -smp 4 -M pc-q35-5.2 results in num-queues=4.
> > >
> > > > This issue requires Qemu to provide a hint that shows
> > > > multi-queues disk allocation is automatically supported, and this
> allows
> > > > upper APPs, e.g., libvirt, to recognize the hypervisor's capability
> of
> > > > this. And upper APPs can ensure to allocate the same num-queues on
> the
> > > > destination side in case of migration failure.
> > > >
> > > > To fix the issue, we introduce the auto-num-queues property for
> > > > virtio-*-pci as a solution, which would be probed by APPs, e.g.,
> libvirt
> > > > by querying the device properties of QEMU. When launching live
> > > > migration, libvirt will send the auto-num-queues property as a
> migration
> > > > cookie to the destination, and thus the destination knows if the
> source
> > > > side supports auto-num-queues. If not, the destination would switch
> off
> > > > by building the command line with "auto-num-queues=off" when
> preparing
> > > > the incoming VM process. The following patches of libvirt show how it
> > > > roughly works:
> > > >
> > >
> https://github.com/newfriday/libvirt/commit/ce2bae2e1a6821afeb80756dc01f3680f525e506
> > > >
> > >
> https://github.com/newfriday/libvirt/commit/f546972b009458c88148fe079544db7e9e1f43c3
> > > >
> > >
> https://github.com/newfriday/libvirt/commit/5ee19c8646fdb4d87ab8b93f287c20925268ce83
> > > >
> > > > The smooth upgrade solution requires the introduction of the
> auto-num-
> > > > queues property on the QEMU side, which is what the patch set does.
> I'm
> > > > hoping for comments about the series.
> > >
> > > Please take a look at versioned machine types. I think auto-num-queues
> > > is not necessary if you use versioned machine types.
> > >
> > > If you do think auto-num-queues is needed, please explain the issue in
> > > more detail and state why versioned machine types don't help.
> >
> >
> > "Using the versioned machine types" is indeed the standard way to ensure
> > the proper functioning of live migration.
> >
> > However, a stable version is strongly advised to maintain function in our
> > production environment and perhaps practically all the production
> > environments in other businesses. As a result, we must backport features
> > like "auto-allocation num-queues" while keeping the machine type the
> same.
> >
> > This patch set is posted for that reason. The "feature-backport" scenario
> > is its target. I'm not sure if the upstream development strategy should
> > take this scenario into account; if it does, perhaps this patch set can
> be
> > of use. After all, the primary goal of this patch set is to broaden the
> uses
> > for this feature and essentially makes no functional changes.
>
> Downstreams that backport only a subset of patches instead of following
> upstream linearly must define their own versioned machine types since
> upstream versioned machine types are not relevant downstream.
>
> For example, here is how CentOS Stream defines its own machine types:
>
> https://gitlab.com/redhat/centos-stream/src/qemu-kvm/-/commit/318178778db60b6475d1484509bee136317156d3
>
> I think it's nicer for downstreams to define custom versioned machine
> types than to add properties like auto-num-queues that are intended for
> downstream usage and don't serve a purpose upstream.
>
> My suggestion is for you to maintain your own custom versioned machine
> types so you can systematically control device properties across
> versions. Does that make sense?
This is a long-term approach to production, and perhaps it works for us.
We'll test the options out and maintain in touch with the upstream party.
Anyway, I appreciate your interest and comments on this series. :)
Yong
>
> Stefan
>
> >
> >
> >
> >
> > > Thanks,
> > > Stefan
> > >
> > > >
> > > > Please review, thanks.
> > > > Yong
> > > >
> > > > Hyman Huang(黄勇) (3):
> > > > virtio-scsi-pci: introduce auto-num-queues property
> > > > virtio-blk-pci: introduce auto-num-queues property
> > > > vhost-user-blk-pci: introduce auto-num-queues property
> > > >
> > > > hw/block/vhost-user-blk.c | 1 +
> > > > hw/block/virtio-blk.c | 1 +
> > > > hw/scsi/vhost-scsi.c | 2 ++
> > > > hw/scsi/vhost-user-scsi.c | 2 ++
> > > > hw/scsi/virtio-scsi.c | 2 ++
> > > > hw/virtio/vhost-scsi-pci.c | 11 +++++++++--
> > > > hw/virtio/vhost-user-blk-pci.c | 9 ++++++++-
> > > > hw/virtio/vhost-user-scsi-pci.c | 11 +++++++++--
> > > > hw/virtio/virtio-blk-pci.c | 9 ++++++++-
> > > > hw/virtio/virtio-scsi-pci.c | 11 +++++++++--
> > > > include/hw/virtio/vhost-user-blk.h | 5 +++++
> > > > include/hw/virtio/virtio-blk.h | 5 +++++
> > > > include/hw/virtio/virtio-scsi.h | 5 +++++
> > > > 13 files changed, 66 insertions(+), 8 deletions(-)
> > > >
> > > > --
> > > > 2.38.5
> > > >
> > >
> >
> >
> > --
> > Best regards
>
--
Best regards
[-- Attachment #2: Type: text/html, Size: 11675 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread