On Tue, Aug 31, 2021 at 18:51:07 +0900, Hiroki Narukawa wrote:
The option "queue-size" in virtio-blk was added in qemu-2.12.0, and default value increased from qemu-5.0.0. However, increasing this value may lead to drop of random access performance. This is configurable value, so we want to use it via libvirt.
Signed-off-by: Hiroki Narukawa <hnarukaw@yahoo-corp.jp> --- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 6 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 5 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 3 ++ src/qemu/qemu_validate.c | 7 ++++ .../caps_2.12.0.aarch64.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + .../caps_3.0.0.riscv32.xml | 1 + .../caps_3.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 + .../caps_3.1.0.x86_64.xml | 1 + .../caps_4.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml | 1 + .../caps_4.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../caps_6.1.0.x86_64.xml | 1 + .../disk-virtio-queue-size.args | 29 +++++++++++++++ .../disk-virtio-queue-size.xml | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../disk-virtio-queue-size.xml | 35 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 47 files changed, 165 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-queue-size.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-queue-size.xml
You'll have to split this into multiple patches. Generally we split it into following logical steps (note that after every single patch the tree must build cleanly) 1) XML/public api changes 2) qemu capability probing changes 3) qemu implementation and test updates [...]
@@ -5053,6 +5054,10 @@ virQEMUCapsInitQMPBasicArch(virQEMUCaps *qemuCaps) static void virQEMUCapsInitQMPVersionCaps(virQEMUCaps *qemuCaps) { + /* virtio-blk queue-size is added on QEMU 2.12 */ + if (qemuCaps->version >= 2012000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE);
NACK to this hunk, there should be a reasonable way to detect this other than version check by probing the properties of virtio-blk which we should already be doing.
+ /* -enable-fips is deprecated in QEMU 5.2.0, and QEMU * should be built with gcrypt to achieve FIPS compliance * automatically / implicitly
[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b230314f7f..5c360b7c6f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1738,6 +1738,9 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, if (disk->queues) { virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); } + if (disk->queue_size) {
Please use an explicit '> 0' check here since it's not a boolean.
+ virBufferAsprintf(&opt, ",queue-size=%u", disk->queue_size); + }
qemuBuildVirtioOptionsStr(&opt, disk->virtio);
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 8906aa52d9..c72aaa2163 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2822,6 +2822,13 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, "QEMU binary")); return -1; } + if (disk->queue_size &&
same as above.
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("queue-size property isn't supported by this " + "QEMU binary"));
Error messages on a single line please.
+ return -1; + } break;