[PATCH v2 0/3] support for virtio-blk-pci discard option

From: yuxiating <yuxiating@huawei.com> v1->v2 add docs/formatdomain.rst for discard_enable add qemuxml2argvtest change virDomainDiskDiscardEnable to virTristateSwitch yuxiating (3): qemu: probe for virtio-blk-pci discard option support conf: support for virtio-blk-pci discard option qemu: command: support for virtio-blk-pci discard option docs/formatdomain.rst | 3 ++ docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 8 +++++ src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 6 ++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 +++ .../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 + .../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 + .../qemuxml2argvdata/disk-virtio-discard.args | 29 +++++++++++++++ .../qemuxml2argvdata/disk-virtio-discard.xml | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 36 files changed, 125 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.xml -- 2.27.0

From: yuxiating <yuxiating@huawei.com> Signed-off-by: yuxiating <yuxiating@huawei.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + 27 files changed, 28 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 70c3ec2f0c..78ec31f807 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -638,6 +638,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "query-display-options", /* QEMU_CAPS_QUERY_DISPLAY_OPTIONS */ "s390-pv-guest", /* QEMU_CAPS_S390_PV_GUEST */ "set-action", /* QEMU_CAPS_SET_ACTION */ + "virtio-blk-pci.discard", /* QEMU_CAPS_DEVICE_DISCARD */ ); @@ -1406,6 +1407,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioBlk[] = { { "werror", QEMU_CAPS_STORAGE_WERROR, NULL }, { "packed", QEMU_CAPS_VIRTIO_PACKED_QUEUES, NULL }, { "acpi-index", QEMU_CAPS_ACPI_INDEX, NULL }, + { "discard", QEMU_CAPS_DEVICE_DISCARD, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioNet[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index bc762d1916..a05c98b71d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -618,6 +618,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command present */ QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */ QEMU_CAPS_SET_ACTION, /* 'set-action' QMP command */ + QEMU_CAPS_DEVICE_DISCARD, /* -device virtio-blk-pci,discard=on/off */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml index efb891fa01..8603c8a78d 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml @@ -174,6 +174,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml index 1e2b7c7fe6..67c7409176 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml @@ -181,6 +181,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml index 5872ecd491..ed906e4bb3 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml @@ -173,6 +173,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml index bb76faae2b..66888f597b 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml @@ -173,6 +173,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml index 51074b4f37..66ad9021e1 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml @@ -137,6 +137,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index 19b8a49394..d349e3e3c8 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -219,6 +219,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml index 841b753518..66916fdc67 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -226,6 +226,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml index 8116624181..2f2b19d3fa 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml @@ -189,6 +189,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml index d37c9b3426..e6b592a193 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml @@ -188,6 +188,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml index 258e382232..81754d504d 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml @@ -150,6 +150,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index 6e3aa7f5d9..871807ebcb 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -237,6 +237,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index 4ce8244540..3e6e36eff3 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -199,6 +199,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml index 07e00008ee..5b4567ba36 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml @@ -207,6 +207,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml index 1bbb9b98cd..27df3095b5 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml @@ -193,6 +193,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index 0c28645f69..a79331ee05 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -244,6 +244,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index fd77d9bbc9..e06ec38082 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -246,6 +246,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml index 4e31d8245e..255319da48 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml @@ -203,6 +203,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml index ac81364857..056ac9795c 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml @@ -209,6 +209,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml index 21a1a5c3dd..64c5eb2fcd 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml @@ -195,6 +195,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml index 13caca9626..81779831a2 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml @@ -157,6 +157,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index 234ac8f7ef..8db0ec48c1 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -247,6 +247,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='virtio-blk-pci.discard'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml index dcc41ed067..9776b2dfd1 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml @@ -211,6 +211,7 @@ <flag name='confidential-guest-support'/> <flag name='query-display-options'/> <flag name='set-action'/> + <flag name='virtio-blk-pci.discard'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml index ebcca6e114..a7b8b8b805 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml @@ -165,6 +165,7 @@ <flag name='query-display-options'/> <flag name='s390-pv-guest'/> <flag name='set-action'/> + <flag name='virtio-blk-pci.discard'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index 4951644354..bc45540def 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -255,6 +255,7 @@ <flag name='confidential-guest-support'/> <flag name='query-display-options'/> <flag name='set-action'/> + <flag name='virtio-blk-pci.discard'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index eca9facf80..7b2efe0aee 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -257,6 +257,7 @@ <flag name='confidential-guest-support'/> <flag name='query-display-options'/> <flag name='set-action'/> + <flag name='virtio-blk-pci.discard'/> <version>6001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.27.0

From: yuxiating <yuxiating@huawei.com> DISCARD and WRITE_ZEROES features for machine type >= 4.0 is enabled by default since 5c81161f8041("virtio-blk: add "discard" and "write-zeroes" properties). Virtio_blk kernel driver has a bug that causes memory corruption in virtblk_setup_discard_write_zeroes(); af822aa68fbd ("block: virtio_blk: fix handling single range discard request") has fix it. However, some operating systems are not fixed and need to disabled on the QEMU side. Signed-off-by: yuxiating <yuxiating@huawei.com> --- docs/formatdomain.rst | 3 +++ docs/schemas/domaincommon.rng | 8 ++++++++ src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 6 ++++++ 5 files changed, 26 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 479a3acfbb..e579b9f4dc 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3087,6 +3087,9 @@ paravirtualized driver is specified via the ``disk`` element. virtio-blk. ( :since:`Since 3.9.0` ) - For virtio disks, `Virtio-specific options <#elementsVirtio>`__ can also be set. ( :since:`Since 3.5.0` ) + - The optional ``discard_enable`` attribute controls whether to enable the + DISCRAD feature of virtio-blk.The value can be either "on" or "off". + :since:`Since 7.7.0 (QEMU and KVM only)` - The optional ``metadata_cache`` subelement controls aspects related to the format specific caching of storage image metadata. Note that this setting applies only on the top level image; the identically named subelement of diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 11fa24f398..72576cac01 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2363,6 +2363,9 @@ <ref name="positiveInteger"/> </attribute> </optional> + <optional> + <ref name="discard_enable"/> + </optional> <ref name="virtioOptions"/> <optional> <element name="metadata_cache"> @@ -2468,6 +2471,11 @@ </choice> </attribute> </define> + <define name="discard_enable"> + <attribute name='discard_enable'> + <ref name="virOnOff"/> + </attribute> + </define> <define name="controller"> <element name="controller"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6127513117..304015f42e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8930,6 +8930,10 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def, if (virXMLPropUInt(cur, "queues", 10, VIR_XML_PROP_NONE, &def->queues) < 0) return -1; + if (virXMLPropEnum(cur, "discard_enable", virTristateSwitchTypeFromString, + VIR_XML_PROP_NONZERO, &def->discard_enable) < 0) + return -1; + return 0; } @@ -23416,6 +23420,10 @@ virDomainDiskDefFormatDriver(virBuffer *buf, if (disk->queues) virBufferAsprintf(&attrBuf, " queues='%u'", disk->queues); + if (disk->discard_enable) + virBufferAsprintf(&attrBuf, " discard_enable='%s'", + virTristateSwitchTypeToString(disk->discard_enable)); + virDomainVirtioOptionsFormat(&attrBuf, disk->virtio); if (disk->src->metadataCacheMaxSize > 0) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c7e6df7981..56cb6e1244 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -589,6 +589,7 @@ struct _virDomainDiskDef { bool diskElementAuth; bool diskElementEnc; + virTristateSwitch discard_enable; }; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 60f7ccdddd..6eb346916a 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -612,6 +612,12 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->discard_enable) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("discard_enable attribute in disk driver element is only supported by virtio-blk")); + return -1; + } + if (disk->event_idx != VIR_TRISTATE_SWITCH_ABSENT) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk event_idx mode supported only for virtio bus")); -- 2.27.0

On Tue, Sep 07, 2021 at 15:12:09 +0800, yuxiating@huawei.com wrote:
From: yuxiating <yuxiating@huawei.com>
DISCARD and WRITE_ZEROES features for machine type >= 4.0 is enabled by default since 5c81161f8041("virtio-blk: add "discard" and "write-zeroes" properties). Virtio_blk kernel driver has a bug that causes memory corruption in virtblk_setup_discard_write_zeroes(); af822aa68fbd ("block: virtio_blk: fix handling single range discard request") has fix it. However, some operating systems are not fixed and need to disabled on the QEMU side.
I must say that I'm not persuaded that there's enough value in this workaround. Similarly users could use virtio-scsi instead as a workaround or fix their OS. For me this reasoning will not be enough, but other on the list might think otherwise.
Signed-off-by: yuxiating <yuxiating@huawei.com> --- docs/formatdomain.rst | 3 +++ docs/schemas/domaincommon.rng | 8 ++++++++ src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 6 ++++++ 5 files changed, 26 insertions(+)
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6127513117..304015f42e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[...]
@@ -23416,6 +23420,10 @@ virDomainDiskDefFormatDriver(virBuffer *buf, if (disk->queues) virBufferAsprintf(&attrBuf, " queues='%u'", disk->queues);
+ if (disk->discard_enable)
We tend to use explicit check: if (disk->discard_enable != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(&attrBuf, " discard_enable='%s'", + virTristateSwitchTypeToString(disk->discard_enable)); + virDomainVirtioOptionsFormat(&attrBuf, disk->virtio);
if (disk->src->metadataCacheMaxSize > 0) {
This definitely seems like a feature visible to the guest OS and thus _must_ be covered by the ABI stability check to prevent regressions in the ABI. See 'virDomainDiskDefCheckABIStability'.

On Wed, Sep 08, 2021 at 11:56:02 +0200, Peter Krempa wrote:
On Tue, Sep 07, 2021 at 15:12:09 +0800, yuxiating@huawei.com wrote:
From: yuxiating <yuxiating@huawei.com>
DISCARD and WRITE_ZEROES features for machine type >= 4.0 is enabled by default since 5c81161f8041("virtio-blk: add "discard" and "write-zeroes" properties). Virtio_blk kernel driver has a bug that causes memory corruption in virtblk_setup_discard_write_zeroes(); af822aa68fbd ("block: virtio_blk: fix handling single range discard request") has fix it. However, some operating systems are not fixed and need to disabled on the QEMU side.
I must say that I'm not persuaded that there's enough value in this workaround. Similarly users could use virtio-scsi instead as a workaround or fix their OS.
For me this reasoning will not be enough, but other on the list might think otherwise.
I want to elaborate a bit. Detecting whether a guest OS has a fix is hard and additionally the guest can be patched without restart of the VM where the user would lose discards even when it isn't a problem any more. This brings me to another thing I forgot to point out. Using discard_enable='off' must be made incompatible with discard='unmap'. e.g. <disk type='file' device='disk'> <driver name='qemu' type='raw' discard_enable='off' discard='unmap'/> <source file='/tmp/data.img'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk> XML must be rejected by the validator as discards would not be propagated to the file even on an explicit request of the user. I'm also slightly unhappy that we are stashing frontend config under <driver> but there's prior art in doing that so that doesn't need to be changed.

From: yuxiating <yuxiating@huawei.com> This patch introduces a new attribute "discard". An example of the XML: <disk type='file' device='disk'> <driver name='qemu' type='qcow2' discard_enable='4'/> The corresponding QEMU command line: -device virtio-blk-pci,scsi=off,discard=off,id=virtio-disk0 Signed-off-by: yuxiating <yuxiating@huawei.com> --- src/qemu/qemu_command.c | 5 +++ .../qemuxml2argvdata/disk-virtio-discard.args | 29 +++++++++++++++ .../qemuxml2argvdata/disk-virtio-discard.xml | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 71 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b230314f7f..cf5a4ca7cf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1739,6 +1739,11 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); } + if (disk->discard_enable) { + virBufferAsprintf(&opt, ",discard=%s", + virTristateSwitchTypeToString(disk->discard_enable)); + } + qemuBuildVirtioOptionsStr(&opt, disk->virtio); if (qemuBuildDeviceAddressStr(&opt, def, &disk->info) < 0) diff --git a/tests/qemuxml2argvdata/disk-virtio-discard.args b/tests/qemuxml2argvdata/disk-virtio-discard.args new file mode 100644 index 0000000000..0e952ca81a --- /dev/null +++ b/tests/qemuxml2argvdata/disk-virtio-discard.args @@ -0,0 +1,29 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i386 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-drive file=/tmp/data.img,format=raw,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-pci,discard=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-virtio-discard.xml b/tests/qemuxml2argvdata/disk-virtio-discard.xml new file mode 100644 index 0000000000..37450f0bfc --- /dev/null +++ b/tests/qemuxml2argvdata/disk-virtio-discard.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i386</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw' discard_enable='off'/> + <source file='/tmp/data.img'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3b331d5fd4..4375b06d40 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1341,6 +1341,8 @@ mymain(void) DO_TEST("disk-order", QEMU_CAPS_VIRTIO_BLK_SCSI); DO_TEST("disk-virtio-queues", QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES); + DO_TEST("disk-virtio-discard", + QEMU_CAPS_DEVICE_DISCARD); DO_TEST_NOCAPS("disk-boot-disk"); DO_TEST_NOCAPS("disk-boot-cdrom"); DO_TEST_NOCAPS("floppy-drive-fat"); -- 2.27.0

On Tue, Sep 07, 2021 at 15:12:10 +0800, yuxiating@huawei.com wrote:
From: yuxiating <yuxiating@huawei.com>
This patch introduces a new attribute "discard". An example of the XML:
<disk type='file' device='disk'> <driver name='qemu' type='qcow2' discard_enable='4'/>
The corresponding QEMU command line:
-device virtio-blk-pci,scsi=off,discard=off,id=virtio-disk0
Signed-off-by: yuxiating <yuxiating@huawei.com> --- src/qemu/qemu_command.c | 5 +++ .../qemuxml2argvdata/disk-virtio-discard.args | 29 +++++++++++++++ .../qemuxml2argvdata/disk-virtio-discard.xml | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 71 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-discard.xml
[...]
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3b331d5fd4..4375b06d40 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1341,6 +1341,8 @@ mymain(void) DO_TEST("disk-order", QEMU_CAPS_VIRTIO_BLK_SCSI); DO_TEST("disk-virtio-queues", QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES); + DO_TEST("disk-virtio-discard", + QEMU_CAPS_DEVICE_DISCARD);
Please always use DO_TEST_CAPS_LATEST with new test cases as it covers real qemu capabilities.
DO_TEST_NOCAPS("disk-boot-disk"); DO_TEST_NOCAPS("disk-boot-cdrom"); DO_TEST_NOCAPS("floppy-drive-fat"); -- 2.27.0
participants (2)
-
Peter Krempa
-
yuxiating@huawei.com