[PATCH v2 0/2] add support for discard_granularity
This is v2 (or rather the first 2 patches) of: https://listman.redhat.com/archives/libvir-list/2023-August/241076.html diff to v1: * added ABI stability check (noticed by Peter) * splitting the series into two parts Kristina Hanicova (2): conf: add support for discard_granularity qemu: add support for discard_granularity docs/formatdomain.rst | 6 +++++- src/conf/domain_conf.c | 19 ++++++++++++++++++- src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 3 ++- src/conf/schemas/domaincommon.rng | 5 +++++ src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_domain.c | 2 ++ src/vz/vz_utils.c | 3 ++- .../disk-blockio.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/disk-blockio.xml | 2 +- 10 files changed, 39 insertions(+), 6 deletions(-) -- 2.41.0
This introduces the ability to set the discard granularity option for a disk. It defines the smallest amount of data that can be discarded in a single operation (useful for managing and optimizing storage). Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- docs/formatdomain.rst | 6 +++++- src/conf/domain_conf.c | 19 ++++++++++++++++++- src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 3 ++- src/conf/schemas/domaincommon.rng | 5 +++++ src/qemu/qemu_domain.c | 2 ++ 6 files changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index cd9cb02bf8..0d0812f08c 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2588,7 +2588,7 @@ paravirtualized driver is specified via the ``disk`` element. <driver name='qemu' type='raw'/> <source dev='/dev/sda'/> <geometry cyls='16383' heads='16' secs='63' trans='lba'/> - <blockio logical_block_size='512' physical_block_size='4096'/> + <blockio logical_block_size='512' physical_block_size='4096' discard_granularity='4096'/> <target dev='hdj' bus='ide'/> </disk> <disk type='volume' device='disk'> @@ -3435,6 +3435,10 @@ paravirtualized driver is specified via the ``disk`` element. this would be the value returned by the BLKPBSZGET ioctl and describes the disk's hardware sector size which can be relevant for the alignment of disk data. + ``discard_granularity`` + The smallest amount of data that can be discarded in a single operation. + It impacts the unmap operations and it must be a multiple of a + ``logical_block_size``. Filesystems ~~~~~~~~~~~ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 69934026ef..bb4f1fdb94 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8073,6 +8073,10 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, if (virXMLPropUInt(blockioNode, "physical_block_size", 10, VIR_XML_PROP_NONE, &def->blockio.physical_block_size) < 0) return NULL; + + if (virXMLPropUInt(blockioNode, "discard_granularity", 10, VIR_XML_PROP_NONE, + &def->blockio.discard_granularity) < 0) + return NULL; } if ((driverNode = virXPathNode("./driver", ctxt))) { @@ -19836,6 +19840,13 @@ virDomainDiskBlockIoCheckABIStability(virDomainDiskDef *src, dst->blockio.physical_block_size, src->blockio.physical_block_size); return false; } + + if (src->blockio.discard_granularity != dst->blockio.discard_granularity) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target disk discard_granularity %1$u does not match source %2$u"), + dst->blockio.discard_granularity, src->blockio.discard_granularity); + return false; + } return true; } @@ -22132,7 +22143,8 @@ virDomainDiskBlockIoDefFormat(virBuffer *buf, virDomainDiskDef *def) { if (def->blockio.logical_block_size > 0 || - def->blockio.physical_block_size > 0) { + def->blockio.physical_block_size > 0 || + def->blockio.discard_granularity > 0) { virBufferAddLit(buf, "<blockio"); if (def->blockio.logical_block_size > 0) { virBufferAsprintf(buf, @@ -22144,6 +22156,11 @@ virDomainDiskBlockIoDefFormat(virBuffer *buf, " physical_block_size='%u'", def->blockio.physical_block_size); } + if (def->blockio.discard_granularity > 0) { + virBufferAsprintf(buf, + " discard_granularity='%u'", + def->blockio.discard_granularity); + } virBufferAddLit(buf, "/>\n"); } } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8bef097542..ca195a52d2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -579,6 +579,7 @@ struct _virDomainDiskDef { struct { unsigned int logical_block_size; unsigned int physical_block_size; + unsigned int discard_granularity; } blockio; virDomainBlockIoTuneInfo blkdeviotune; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index b286990b19..fd3eed3230 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -466,7 +466,8 @@ virDomainDiskVhostUserValidate(const virDomainDiskDef *disk) } if (disk->blockio.logical_block_size > 0 || - disk->blockio.physical_block_size > 0) { + disk->blockio.physical_block_size > 0 || + disk->blockio.discard_granularity > 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("blockio is not supported with vhostuser disk")); return -1; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 2556ac01ed..de3bd1c35c 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2461,6 +2461,11 @@ <data type="integer"/> </attribute> </optional> + <optional> + <attribute name="discard_granularity"> + <data type="integer"/> + </attribute> + </optional> </element> </define> <!-- diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0b2b22a219..bfeddc7746 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8426,6 +8426,8 @@ qemuDomainDiskChangeSupported(virDomainDiskDef *disk, "blockio logical_block_size", false); CHECK_EQ(blockio.physical_block_size, "blockio physical_block_size", false); + CHECK_EQ(blockio.discard_granularity, + "blockio discard_granularity", false); CHECK_EQ(blkdeviotune.total_bytes_sec, "blkdeviotune total_bytes_sec", -- 2.41.0
On Fri, Aug 25, 2023 at 13:52:14 +0200, Kristina Hanicova wrote:
This introduces the ability to set the discard granularity option for a disk. It defines the smallest amount of data that can be discarded in a single operation (useful for managing and optimizing storage).
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- docs/formatdomain.rst | 6 +++++- src/conf/domain_conf.c | 19 ++++++++++++++++++- src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 3 ++- src/conf/schemas/domaincommon.rng | 5 +++++ src/qemu/qemu_domain.c | 2 ++ 6 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index cd9cb02bf8..0d0812f08c 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2588,7 +2588,7 @@ paravirtualized driver is specified via the ``disk`` element. <driver name='qemu' type='raw'/> <source dev='/dev/sda'/> <geometry cyls='16383' heads='16' secs='63' trans='lba'/> - <blockio logical_block_size='512' physical_block_size='4096'/> + <blockio logical_block_size='512' physical_block_size='4096' discard_granularity='4096'/> <target dev='hdj' bus='ide'/> </disk> <disk type='volume' device='disk'> @@ -3435,6 +3435,10 @@ paravirtualized driver is specified via the ``disk`` element. this would be the value returned by the BLKPBSZGET ioctl and describes the disk's hardware sector size which can be relevant for the alignment of disk data. + ``discard_granularity`` + The smallest amount of data that can be discarded in a single operation. + It impacts the unmap operations and it must be a multiple of a + ``logical_block_size``.
Add a note that most hypervisors select the proper setting and users don't really need to tweak this. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This commit adds building of `discard_granularity` disk option for qemu commandline. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1849570 Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_command.c | 2 ++ src/vz/vz_utils.c | 3 ++- tests/qemuxml2argvdata/disk-blockio.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/disk-blockio.xml | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6bcf2fdadd..b228820b5f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1760,6 +1760,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, unsigned int bootindex = 0; unsigned int logical_block_size = disk->blockio.logical_block_size; unsigned int physical_block_size = disk->blockio.physical_block_size; + unsigned int discard_granularity = disk->blockio.discard_granularity; g_autoptr(virJSONValue) wwn = NULL; g_autofree char *serial = NULL; virTristateSwitch removable = VIR_TRISTATE_SWITCH_ABSENT; @@ -1939,6 +1940,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "p:bootindex", bootindex, "p:logical_block_size", logical_block_size, "p:physical_block_size", physical_block_size, + "p:discard_granularity", discard_granularity, "A:wwn", &wwn, "p:rotation_rate", disk->rotation_rate, "S:vendor", disk->vendor, diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 7db7dbd419..de707df883 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -279,7 +279,8 @@ vzCheckDiskUnsupportedParams(virDomainDiskDef *disk) } if (disk->blockio.logical_block_size || - disk->blockio.physical_block_size) { + disk->blockio.physical_block_size || + disk->blockio.discard_granularity) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Setting disk block sizes is not " "supported by vz driver.")); diff --git a/tests/qemuxml2argvdata/disk-blockio.x86_64-latest.args b/tests/qemuxml2argvdata/disk-blockio.x86_64-latest.args index 7270613573..15f31ae60d 100644 --- a/tests/qemuxml2argvdata/disk-blockio.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-blockio.x86_64-latest.args @@ -32,7 +32,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -device '{"driver":"ide-cd","bus":"ide.0","unit":1,"drive":"libvirt-2-format","id":"ide0-0-1"}' \ -blockdev '{"driver":"file","filename":"/tmp/idedisk.img","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ --device '{"driver":"ide-hd","bus":"ide.0","unit":2,"drive":"libvirt-1-format","id":"ide0-0-2","bootindex":1,"logical_block_size":512,"physical_block_size":512}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":2,"drive":"libvirt-1-format","id":"ide0-0-2","bootindex":1,"logical_block_size":512,"physical_block_size":512,"discard_granularity":4096}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/disk-blockio.xml b/tests/qemuxml2argvdata/disk-blockio.xml index 170728371f..84943719d4 100644 --- a/tests/qemuxml2argvdata/disk-blockio.xml +++ b/tests/qemuxml2argvdata/disk-blockio.xml @@ -23,7 +23,7 @@ <source file='/tmp/idedisk.img'/> <target dev='hdc' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='2'/> - <blockio logical_block_size='512' physical_block_size='512'/> + <blockio logical_block_size='512' physical_block_size='512' discard_granularity='4096'/> </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> -- 2.41.0
On Fri, Aug 25, 2023 at 13:52:15 +0200, Kristina Hanicova wrote:
This commit adds building of `discard_granularity` disk option for qemu commandline.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1849570
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_command.c | 2 ++ src/vz/vz_utils.c | 3 ++- tests/qemuxml2argvdata/disk-blockio.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/disk-blockio.xml | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-)
[...]
diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 7db7dbd419..de707df883 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -279,7 +279,8 @@ vzCheckDiskUnsupportedParams(virDomainDiskDef *disk) }
if (disk->blockio.logical_block_size || - disk->blockio.physical_block_size) { + disk->blockio.physical_block_size || + disk->blockio.discard_granularity) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Setting disk block sizes is not " "supported by vz driver."));
This hunk doesn't belong to this patch.
diff --git a/tests/qemuxml2argvdata/disk-blockio.xml b/tests/qemuxml2argvdata/disk-blockio.xml index 170728371f..84943719d4 100644 --- a/tests/qemuxml2argvdata/disk-blockio.xml +++ b/tests/qemuxml2argvdata/disk-blockio.xml
Please also enable this test case in qemuxml2xmltest.
@@ -23,7 +23,7 @@ <source file='/tmp/idedisk.img'/> <target dev='hdc' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='2'/> - <blockio logical_block_size='512' physical_block_size='512'/> + <blockio logical_block_size='512' physical_block_size='512' discard_granularity='4096'/> </disk> <controller type='usb' index='0'/>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Kristina Hanicova -
Peter Krempa