[PATCH 0/2] Validate 'removable' and support 'dpofua' properties of SCSI disks
Patch 1 adds a missing ABI stability check for 'removable' and patch 2 implements 'dpofua' for SCSI disks. Peter Krempa (2): virDomainDiskDefCheckABIStability: Add missing check for 'removable' property Add support for 'dpofua' setting for SCSI disks docs/formatdomain.rst | 5 ++++ src/conf/domain_conf.c | 23 +++++++++++++++++++ src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 7 ++++++ src/conf/schemas/domaincommon.rng | 5 ++++ src/qemu/qemu_command.c | 1 + .../disk-scsi.x86_64-latest.args | 4 ++-- .../disk-scsi.x86_64-latest.xml | 4 ++-- tests/qemuxmlconfdata/disk-scsi.xml | 4 ++-- 9 files changed, 48 insertions(+), 6 deletions(-) -- 2.51.1
From: Peter Krempa <pkrempa@redhat.com> The 'removable' state is guest-visible. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d2dea6952e..15d5cd9d80 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20845,6 +20845,14 @@ virDomainDiskDefCheckABIStability(virDomainDiskDef *src, return false; } + if (src->removable != dst->removable) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target disk 'removable' property %1$s does not match source %2$s"), + virTristateSwitchTypeToString(dst->removable), + virTristateSwitchTypeToString(src->removable)); + return false; + } + if (src->queues != dst->queues) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target disk queue count %1$u does not match source %2$u"), -- 2.51.1
From: Peter Krempa <pkrempa@redhat.com> Add the 'dpofua' setting in the XML and for the qemu driver. DPO - Disable Page Out and FUA - Force Unit Access are two features implemented by SCSI disks (either both together or neither of them) which influence how caching is handled. QEMU provides a good default but in certain specific occasions changing the default may have performance benefits. Add support for setting them via the XML. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 5 +++++ src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 7 +++++++ src/conf/schemas/domaincommon.rng | 5 +++++ src/qemu/qemu_command.c | 1 + .../qemuxmlconfdata/disk-scsi.x86_64-latest.args | 4 ++-- tests/qemuxmlconfdata/disk-scsi.x86_64-latest.xml | 4 ++-- tests/qemuxmlconfdata/disk-scsi.xml | 4 ++-- 9 files changed, 40 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index c6d0b183d0..1197369481 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3378,6 +3378,11 @@ paravirtualized driver is specified via the ``disk`` element. :since:`since after 0.4.4`; "sata" attribute value :since:`since 0.9.7`; "removable" attribute value :since:`since 1.1.3`; "rotation_rate" attribute value :since:`since 7.3.0` + The optional attribute ``dpofua`` (:since:`Since 11.10.0, only QEMU driver`) + controls the support of DPO(Disable Page Out) and FUA(Force Unit Access) + properties of a SCSI disk cache access (both must be present or absent). + If the value is omitted hypervisor default is applied (which may depend on + the machine type version) and is the suggested setting. ``throttlefilters`` The optional ``throttlefilters`` element provides the ability to provide additional per-device throttle chain :since:`Since 11.2.0` diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15d5cd9d80..f6c6c70374 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8593,6 +8593,10 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, if (virXMLPropUInt(targetNode, "rotation_rate", 10, VIR_XML_PROP_NONE, &def->rotation_rate) < 0) return NULL; + + if (virXMLPropTristateSwitch(targetNode, "dpofua", VIR_XML_PROP_NONE, + &def->dpofua) < 0) + return NULL; } if ((geometryNode = virXPathNode("./geometry", ctxt))) { @@ -20853,6 +20857,14 @@ virDomainDiskDefCheckABIStability(virDomainDiskDef *src, return false; } + if (src->dpofua != dst->dpofua) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target disk 'dpofua' property %1$s does not match source %2$s"), + virTristateSwitchTypeToString(dst->dpofua), + virTristateSwitchTypeToString(src->dpofua)); + return false; + } + if (src->queues != dst->queues) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target disk queue count %1$u does not match source %2$u"), @@ -24120,6 +24132,9 @@ virDomainDiskDefFormat(virBuffer *buf, } if (def->rotation_rate) virBufferAsprintf(&childBuf, " rotation_rate='%u'", def->rotation_rate); + if (def->dpofua != VIR_TRISTATE_SWITCH_ABSENT) + virBufferAsprintf(&childBuf, " dpofua='%s'", + virTristateSwitchTypeToString(def->dpofua)); virBufferAddLit(&childBuf, "/>\n"); virDomainDiskDefFormatIotune(&childBuf, def); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 11eb46ae53..ccd89e42a6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -601,6 +601,7 @@ struct _virDomainDiskDef { unsigned int queue_size; virDomainDiskModel model; virDomainVirtioOptions *virtio; + virTristateSwitch dpofua; /* only SCSI disks; control of the DPO/FUA cache setting */ bool diskElementAuth; bool diskElementEnc; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 8085d782c5..90add3e185 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1024,6 +1024,13 @@ virDomainDiskDefValidate(const virDomainDef *def, } } + if (disk->dpofua != VIR_TRISTATE_SWITCH_ABSENT && + disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'dpofua' option is available only for SCSI disks")); + return -1; + } + return 0; } diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 75b5124c33..4f690837ce 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2615,6 +2615,11 @@ <ref name="positiveInteger"/> </attribute> </optional> + <optional> + <attribute name="dpofua"> + <ref name="virOnOff"/> + </attribute> + </optional> </element> </define> <define name="geometry"> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c56c321a6e..d0f8d5c5a7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1936,6 +1936,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "S:werror", wpolicy, "S:rerror", rpolicy, "A:stats-intervals", &statistics, + "T:dpofua", disk->dpofua, /* SCSI-only, ensured by validation */ NULL) < 0) return NULL; diff --git a/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.args b/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.args index cd07cd2d22..ae0a1ffe0c 100644 --- a/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.args @@ -41,9 +41,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -blockdev '{"driver":"file","filename":"/tmp/scsidisk3.img","node-name":"libvirt-3-storage","read-only":false}' \ -device '{"driver":"scsi-hd","bus":"scsi2.0","channel":0,"scsi-id":0,"lun":0,"device_id":"drive-scsi2-0-0-0","drive":"libvirt-3-storage","id":"scsi2-0-0-0","wwn":5764824127192592812}' \ -blockdev '{"driver":"file","filename":"/tmp/scsidisk4.img","node-name":"libvirt-2-storage","read-only":false}' \ --device '{"driver":"scsi-hd","bus":"scsi3.0","channel":0,"scsi-id":0,"lun":0,"device_id":"drive-scsi3-0-0-0","drive":"libvirt-2-storage","id":"scsi3-0-0-0"}' \ +-device '{"driver":"scsi-hd","bus":"scsi3.0","channel":0,"scsi-id":0,"lun":0,"device_id":"drive-scsi3-0-0-0","drive":"libvirt-2-storage","id":"scsi3-0-0-0","dpofua":true}' \ -blockdev '{"driver":"file","filename":"/tmp/scsidisk5.img","node-name":"libvirt-1-storage","read-only":false}' \ --device '{"driver":"scsi-hd","bus":"scsi4.0","channel":0,"scsi-id":0,"lun":0,"device_id":"drive-scsi4-0-0-0","drive":"libvirt-1-storage","id":"scsi4-0-0-0","removable":true}' \ +-device '{"driver":"scsi-hd","bus":"scsi4.0","channel":0,"scsi-id":0,"lun":0,"device_id":"drive-scsi4-0-0-0","drive":"libvirt-1-storage","id":"scsi4-0-0-0","removable":true,"dpofua":false}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x6"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.xml index 3e645ea343..33de2a6595 100644 --- a/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.xml @@ -46,13 +46,13 @@ <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/tmp/scsidisk4.img'/> - <target dev='sdd' bus='scsi'/> + <target dev='sdd' bus='scsi' dpofua='on'/> <address type='drive' controller='3' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/tmp/scsidisk5.img'/> - <target dev='sde' bus='scsi'/> + <target dev='sde' bus='scsi' dpofua='off'/> <address type='drive' controller='4' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0' model='piix3-uhci'> diff --git a/tests/qemuxmlconfdata/disk-scsi.xml b/tests/qemuxmlconfdata/disk-scsi.xml index 755c856b98..7e20c70dfe 100644 --- a/tests/qemuxmlconfdata/disk-scsi.xml +++ b/tests/qemuxmlconfdata/disk-scsi.xml @@ -38,12 +38,12 @@ </disk> <disk type='file' device='disk'> <source file='/tmp/scsidisk4.img'/> - <target dev='sdd' bus='scsi'/> + <target dev='sdd' bus='scsi' dpofua='on'/> <address type='drive' controller='3' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <source file='/tmp/scsidisk5.img'/> - <target dev='sde' bus='scsi' removable='on'/> + <target dev='sde' bus='scsi' removable='on' dpofua='off'/> <address type='drive' controller='4' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> -- 2.51.1
On Fri, Nov 21, 2025 at 03:01:12PM +0100, Peter Krempa via Devel wrote:
Patch 1 adds a missing ABI stability check for 'removable' and patch 2 implements 'dpofua' for SCSI disks.
Peter Krempa (2): virDomainDiskDefCheckABIStability: Add missing check for 'removable' property Add support for 'dpofua' setting for SCSI disks
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Pavel Hrdina -
Peter Krempa