Re: [PATCH] qemu: Give the users possibility to overwrite temp files for <transient\> disk

Normally when starting up a VM with a transient disk, if the file libvirt would use as the temp overlay for the original disk image exists libvirt will not touch it and fail startup. This is done in order to avoid any potential removal of user files if they manage to create a colliding filename. In case when the user doesn't want the above behaviour this patch' introduces a 'overwriteTemp' attribute for the '<transient/>' element which allows users to opt into simply removing the file before the next start. Closes: https://gitlab.com/libvirt/libvirt/-/issues/684 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatdomain.rst | 7 +++++++ src/conf/domain_conf.c | 7 +++++++ src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 5 +++++ src/qemu/qemu_snapshot.c | 17 +++++++++++++---- .../disk-transient.x86_64-latest.xml | 2 +- tests/qemuxmlconfdata/disk-transient.xml | 2 +- 7 files changed, 35 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index e6f09a728f..ab2fccdd97 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3479,6 +3479,13 @@ paravirtualized driver is specified via the ``disk`` element. ``shareBacking`` attribute should be set to ``yes``. Note that hypervisor drivers may need to hotplug such disk and thus it works only with configurations supporting hotplug. :since:`Since 7.4.0` + + Hypervisors may need to store a temporary file containing the data written + by the domain while running, which may be stored in the same location + as the original source of the disk. Note that in some cases the temporary + file can't be cleaned up (e.g. when the domain is not shutdown before the host). + An optional attribute ``overwriteTemp`` set to ``yes`` (:since:`Since 10.10`) + indicates that the hypervisor may overwrite the file rather than fail startup.

``serial`` If present, this specify serial number of virtual hard drive. For example, it may look like ``<serial>WD-WMAP9A966149</serial>``. Not supported for diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d7dee7956..359591d8f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8240,6 +8240,11 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, VIR_XML_PROP_NONE, &def->transientShareBacking) < 0) return NULL; + + if (virXMLPropTristateBool(transientNode, "overwriteTemp", + VIR_XML_PROP_NONE, + &def->transientOverwriteTemp) < 0) + return NULL; } if (virDomainDiskDefIotuneParse(def, ctxt) < 0) @@ -23233,6 +23238,8 @@ virDomainDiskDefFormat(virBuffer *buf, virBufferAddLit(&childBuf, "<transient"); if (def->transientShareBacking == VIR_TRISTATE_BOOL_YES) virBufferAddLit(&childBuf, " shareBacking='yes'"); + if (def->transientOverwriteTemp == VIR_TRISTATE_BOOL_YES) + virBufferAddLit(&childBuf, " overwriteTemp='yes'"); virBufferAddLit(&childBuf, "/>\n"); } virBufferEscapeString(&childBuf, "<serial>%s</serial>\n", def->serial); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a15af4fae3..169c626584 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -569,6 +569,7 @@ struct _virDomainDiskDef { virDomainStartupPolicy startupPolicy; bool transient; virTristateBool transientShareBacking; + virTristateBool transientOverwriteTemp; virDomainDeviceInfo info; virTristateBool rawio; virDomainDeviceSGIO sgio; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index efb5f00d77..cd87e83410 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -1618,6 +1618,11 @@ <ref name='virYesNo'/> </attribute> </optional> + <optional> + <attribute name="overwriteTemp"> + <ref name='virYesNo'/> + </attribute> + </optional> <empty/> </element> </optional> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 1187ebf276..b8ca045900 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1287,10 +1287,19 @@ qemuSnapshotGetTransientDiskDef(virDomainDiskDef *domdisk, domdisk->src->path, suffix); if (virFileExists(snapdisk->src->path)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("Overlay file '%1$s' for transient disk '%2$s' already exists"), - snapdisk->src->path, domdisk->dst); - return NULL; + if (domdisk->transientOverwriteTemp == VIR_TRISTATE_BOOL_YES) { + if (unlink(snapdisk->src->path) != 0) { + virReportSystemError(errno, + _("Failed to delete overlay file '%1$s' for transient disk '%2$s'"), + snapdisk->src->path, domdisk->dst); + return NULL; + } + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Overlay file '%1$s' for transient disk '%2$s' already exists"), + snapdisk->src->path, domdisk->dst); + return NULL; + } } return g_steal_pointer(&snapdisk); diff --git a/tests/qemuxmlconfdata/disk-transient.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-transient.x86_64-latest.xml index aab5894447..5f8f15f714 100644 --- a/tests/qemuxmlconfdata/disk-transient.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/disk-transient.x86_64-latest.xml @@ -21,7 +21,7 @@ <driver name='qemu' type='qcow2' cache='none'/> <source file='/tmp/QEMUGuest2.img'/> <target dev='vda' bus='virtio'/> - <transient shareBacking='yes'/> + <transient shareBacking='yes' overwriteTemp='yes'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </disk> <disk type='file' device='disk'> diff --git a/tests/qemuxmlconfdata/disk-transient.xml b/tests/qemuxmlconfdata/disk-transient.xml index edd65a0da0..722f1f9a92 100644 --- a/tests/qemuxmlconfdata/disk-transient.xml +++ b/tests/qemuxmlconfdata/disk-transient.xml @@ -18,7 +18,7 @@ <driver name='qemu' type='qcow2' cache='none'/> <source file='/tmp/QEMUGuest2.img'/> <target dev='vda' bus='virtio'/> - <transient shareBacking='yes'/> + <transient shareBacking='yes' overwriteTemp='yes'/> </disk> <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> -- 2.47.0