[PATCH v4 00/11] qemu: Add <transient/> disk support
This version: - is rebased on top of current refactors - has more strict rules in the validation - adds tests - fixes bug with disk-unplug Masayoshi Mizuma (5): qemu: Block blockjobs when transient disk option is enabled qemu: Block disk hotplug when transient disk option is enabled qemu: Block migration when transient disk option is enabled qemu: process: Handle transient disks on VM startup qemu: validate: Allow <transient/> disks Peter Krempa (6): virDomainSnapshotDiskDefFree: Export and register as autoptr func qemu: prepare cleanup for <transient/> disk overlays qemu: snapshot: Introduce helpers for creating overlays on <transient/> disks qemu: hotplug: Remove overlay of <transient> disk on disk unplug tests: qemuxml2argv: Fix and enable 'disk-transient' case TODO: Add news entry TODO | 0 docs/formatdomain.rst | 5 +- src/conf/snapshot_conf.h | 5 + src/conf/snapshot_conf_priv.h | 3 - src/qemu/qemu_domain.c | 9 ++ src/qemu/qemu_domain.h | 4 + src/qemu/qemu_hotplug.c | 15 +++ src/qemu/qemu_migration.c | 10 ++ src/qemu/qemu_process.c | 27 ++++++ src/qemu/qemu_snapshot.c | 91 ++++++++++++++++++- src/qemu/qemu_snapshot.h | 5 + src/qemu/qemu_validate.c | 56 ++++++++++-- .../disk-transient.x86_64-4.1.0.err | 1 + .../disk-transient.x86_64-latest.args | 42 +++++++++ tests/qemuxml2argvdata/disk-transient.xml | 4 +- tests/qemuxml2argvtest.c | 2 + 16 files changed, 265 insertions(+), 14 deletions(-) create mode 100644 TODO create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-latest.args -- 2.26.2
Allow using the function for creating temporary snapshot disk definitions for creating <transient/> disk overlays. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/snapshot_conf.h | 5 +++++ src/conf/snapshot_conf_priv.h | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index fbc9b17c54..0f3987fc80 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -70,6 +70,11 @@ struct _virDomainSnapshotDiskDef { virStorageSourcePtr src; }; +void +virDomainSnapshotDiskDefFree(virDomainSnapshotDiskDefPtr disk); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSnapshotDiskDef, virDomainSnapshotDiskDefFree); + /* Stores the complete snapshot metadata */ struct _virDomainSnapshotDef { virDomainMomentDef parent; diff --git a/src/conf/snapshot_conf_priv.h b/src/conf/snapshot_conf_priv.h index b721a57c4b..369a023881 100644 --- a/src/conf/snapshot_conf_priv.h +++ b/src/conf/snapshot_conf_priv.h @@ -30,6 +30,3 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, virDomainSnapshotDiskDefPtr def, unsigned int flags, virDomainXMLOptionPtr xmlopt); - -void -virDomainSnapshotDiskDefFree(virDomainSnapshotDiskDefPtr disk); -- 2.26.2
Later patches will implement support for <transient/> disks in libvirt by installing an overlay on top of the configured image. This will require cleanup after the VM will be stopped so that the state is correctly discarded. Since the overlay will be installed only during the startup phase of the VM we need to ensure that qemuProcessStop doesn't delete the original file on some previous failure. This is solved by adding 'inhibitDiskTransientDelete' VM private data member which is set prior to any startup step and will be cleared once transient disk overlays are established. Based on that we can then delete the overlays for any <transient/> disk. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_process.c | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 279de2997d..dc5949edfa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1792,6 +1792,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) priv->dbusVMStateIds = NULL; priv->dbusVMState = false; + + priv->inhibitDiskTransientDelete = false; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c7c3c5c073..ec776ced72 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -263,6 +263,10 @@ struct _qemuDomainObjPrivate { char **dbusVMStateIds; /* true if -object dbus-vmstate was added */ bool dbusVMState; + + /* prevent deletion of <transient> disk overlay files between startup and + * succesful setup of the overlays */ + bool inhibitDiskTransientDelete; }; #define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f21b8f1585..ffb3afa9c5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5616,6 +5616,9 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(driver->xmlopt, vm, priv->qemuCaps) < 0) goto cleanup; + /* don't clean up files for <transient> disks until we set them up */ + priv->inhibitDiskTransientDelete = true; + if (flags & VIR_QEMU_PROCESS_START_PRETEND) { if (qemuDomainSetPrivatePaths(driver, vm) < 0) { virDomainObjRemoveTransientDef(vm); @@ -7710,6 +7713,18 @@ void qemuProcessStop(virQEMUDriverPtr driver, } qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src); + + /* for now transient disks are forbidden with migration so they + * can be handled here */ + if (disk->transient && + !priv->inhibitDiskTransientDelete) { + VIR_DEBUG("Removing transient overlay '%s' of disk '%s'", + disk->src->path, disk->dst); + if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) >= 0) { + virStorageFileUnlink(disk->src); + virStorageFileDeinit(disk->src); + } + } } } @@ -8125,6 +8140,10 @@ qemuProcessReconnect(void *opaque) cfg = virQEMUDriverGetConfig(driver); priv = obj->privateData; + /* expect that libvirt might have crashed during VM start, so prevent + * cleanup of transient disks */ + priv->inhibitDiskTransientDelete = true; + if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) < 0) goto error; jobStarted = true; @@ -8228,6 +8247,9 @@ qemuProcessReconnect(void *opaque) goto error; } + /* vm startup complete, we can remove transient disks if required */ + priv->inhibitDiskTransientDelete = false; + /* In case the domain shutdown while we were not running, * we need to finish the shutdown process. And we need to do it after * we have virQEMUCaps filled in. -- 2.26.2
On Thu, Sep 24, 2020 at 01:43:49PM +0200, Peter Krempa wrote:
Later patches will implement support for <transient/> disks in libvirt by installing an overlay on top of the configured image. This will require cleanup after the VM will be stopped so that the state is correctly discarded.
Since the overlay will be installed only during the startup phase of the VM we need to ensure that qemuProcessStop doesn't delete the original file on some previous failure. This is solved by adding 'inhibitDiskTransientDelete' VM private data member which is set prior to any startup step and will be cleared once transient disk overlays are established.
Based on that we can then delete the overlays for any <transient/> disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_process.c | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 279de2997d..dc5949edfa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1792,6 +1792,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) priv->dbusVMStateIds = NULL;
priv->dbusVMState = false; + + priv->inhibitDiskTransientDelete = false; }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c7c3c5c073..ec776ced72 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -263,6 +263,10 @@ struct _qemuDomainObjPrivate { char **dbusVMStateIds; /* true if -object dbus-vmstate was added */ bool dbusVMState; + + /* prevent deletion of <transient> disk overlay files between startup and + * succesful setup of the overlays */ + bool inhibitDiskTransientDelete; };
#define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f21b8f1585..ffb3afa9c5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5616,6 +5616,9 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(driver->xmlopt, vm, priv->qemuCaps) < 0) goto cleanup;
+ /* don't clean up files for <transient> disks until we set them up */ + priv->inhibitDiskTransientDelete = true; + if (flags & VIR_QEMU_PROCESS_START_PRETEND) { if (qemuDomainSetPrivatePaths(driver, vm) < 0) { virDomainObjRemoveTransientDef(vm); @@ -7710,6 +7713,18 @@ void qemuProcessStop(virQEMUDriverPtr driver, }
qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src); + + /* for now transient disks are forbidden with migration so they + * can be handled here */ + if (disk->transient && + !priv->inhibitDiskTransientDelete) { + VIR_DEBUG("Removing transient overlay '%s' of disk '%s'", + disk->src->path, disk->dst); + if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) >= 0) { + virStorageFileUnlink(disk->src); + virStorageFileDeinit(disk->src); + } + } } }
@@ -8125,6 +8140,10 @@ qemuProcessReconnect(void *opaque) cfg = virQEMUDriverGetConfig(driver); priv = obj->privateData;
+ /* expect that libvirt might have crashed during VM start, so prevent + * cleanup of transient disks */ + priv->inhibitDiskTransientDelete = true; + if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) < 0) goto error; jobStarted = true; @@ -8228,6 +8247,9 @@ qemuProcessReconnect(void *opaque) goto error; }
+ /* vm startup complete, we can remove transient disks if required */ + priv->inhibitDiskTransientDelete = false; + /* In case the domain shutdown while we were not running, * we need to finish the shutdown process. And we need to do it after * we have virQEMUCaps filled in. --
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks a lot! Masa
To implement <transient/> disks we'll need to install an overlay on top of the original disk image which will be discarded after the VM is turned off. This was initially implemented by qemu but libvirt never picked up this option. With blockdev the qemu feature became unsupported so we need to do this via the snapshot code anyways. The helpers introduced in this patch prepare a fake snapshot disk definition for a disk which is configured as <transient/> and use it to create a snapshot (without actually modifying metada or persistent def). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 91 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_snapshot.h | 5 +++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index ca051071aa..d10e7b6b3a 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -986,6 +986,7 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, qemuSnapshotDiskDataPtr dd, virHashTablePtr blockNamedNodeData, bool reuse, + bool updateConfig, qemuDomainAsyncJob asyncJob, virJSONValuePtr actions) { @@ -1008,7 +1009,8 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, return -1; /* modify disk in persistent definition only when the source is the same */ - if (vm->newDef && + if (updateConfig && + vm->newDef && (persistdisk = virDomainDiskByTarget(vm->newDef, dd->disk->dst)) && virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) { @@ -1116,6 +1118,54 @@ qemuSnapshotDiskPrepareActiveExternal(virDomainObjPtr vm, snapctxt->dd + snapctxt->ndd++, blockNamedNodeData, reuse, + true, + asyncJob, + snapctxt->actions) < 0) + return NULL; + } + + return g_steal_pointer(&snapctxt); +} + + +static qemuSnapshotDiskContextPtr +qemuSnapshotDiskPrepareDisksTransient(virDomainObjPtr vm, + virQEMUDriverConfigPtr cfg, + virHashTablePtr blockNamedNodeData, + qemuDomainAsyncJob asyncJob) +{ + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + size_t i; + + snapctxt = qemuSnapshotDiskContextNew(vm->def->ndisks, vm, asyncJob); + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr domdisk = vm->def->disks[i]; + g_autoptr(virDomainSnapshotDiskDef) snapdisk = g_new0(virDomainSnapshotDiskDef, 1); + + if (!domdisk->transient) + continue; + + /* validation code makes sure that we do this only for local disks + * with a file source */ + snapdisk->name = g_strdup(domdisk->dst); + snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + snapdisk->src = virStorageSourceNew(); + snapdisk->src->type = VIR_STORAGE_TYPE_FILE; + snapdisk->src->format = VIR_STORAGE_FILE_QCOW2; + snapdisk->src->path = g_strdup_printf("%s.TRANSIENT", domdisk->src->path); + if (virFileExists(snapdisk->src->path)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Overlay file '%s' for transient disk '%s' already exists"), + snapdisk->src->path, domdisk->dst); + return NULL; + } + + if (qemuSnapshotDiskPrepareOne(vm, cfg, domdisk, snapdisk, + snapctxt->dd + snapctxt->ndd++, + blockNamedNodeData, + false, + false, asyncJob, snapctxt->actions) < 0) return NULL; @@ -1253,6 +1303,45 @@ qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm, } +/** + * qemuSnapshotCreateDisksTransient: + * @vm: domain object + * @asyncJob: qemu async job type + * + * Creates overlays on top of disks which are configured as <transient/>. Note + * that the validation code ensures that <transient> disks have appropriate + * configuration. + */ +int +qemuSnapshotCreateDisksTransient(virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + g_autoptr(virHashTable) blockNamedNodeData = NULL; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob))) + return -1; + + if (!(snapctxt = qemuSnapshotDiskPrepareDisksTransient(vm, cfg, + blockNamedNodeData, + asyncJob))) + return -1; + + if (qemuSnapshotDiskCreate(snapctxt, cfg) < 0) + return -1; + } + + /* the overlays are established, so they can be deleted on a shutdown */ + priv->inhibitDiskTransientDelete = false; + + return 0; +} + + static int qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_snapshot.h b/src/qemu/qemu_snapshot.h index 8b3ebe87b1..cf9bfec542 100644 --- a/src/qemu/qemu_snapshot.h +++ b/src/qemu/qemu_snapshot.h @@ -21,6 +21,7 @@ #include "virconftypes.h" #include "datatypes.h" #include "qemu_conf.h" +#include "qemu_domainjob.h" virDomainMomentObjPtr qemuSnapObjFromName(virDomainObjPtr vm, @@ -53,3 +54,7 @@ int qemuSnapshotDelete(virDomainObjPtr vm, virDomainSnapshotPtr snapshot, unsigned int flags); + +int +qemuSnapshotCreateDisksTransient(virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); -- 2.26.2
On Thu, Sep 24, 2020 at 01:43:50PM +0200, Peter Krempa wrote:
To implement <transient/> disks we'll need to install an overlay on top of the original disk image which will be discarded after the VM is turned off. This was initially implemented by qemu but libvirt never > picked up this option. With blockdev the qemu feature became unsupported so we need to do this via the snapshot code anyways.
The helpers introduced in this patch prepare a fake snapshot disk definition for a disk which is configured as <transient/> and use it to create a snapshot (without actually modifying metada or persistent def).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 91 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_snapshot.h | 5 +++ 2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index ca051071aa..d10e7b6b3a 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -986,6 +986,7 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, qemuSnapshotDiskDataPtr dd, virHashTablePtr blockNamedNodeData, bool reuse, + bool updateConfig, qemuDomainAsyncJob asyncJob, virJSONValuePtr actions) { @@ -1008,7 +1009,8 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, return -1;
/* modify disk in persistent definition only when the source is the same */ - if (vm->newDef && + if (updateConfig && + vm->newDef && (persistdisk = virDomainDiskByTarget(vm->newDef, dd->disk->dst)) && virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) {
@@ -1116,6 +1118,54 @@ qemuSnapshotDiskPrepareActiveExternal(virDomainObjPtr vm, snapctxt->dd + snapctxt->ndd++, blockNamedNodeData, reuse, + true, + asyncJob, + snapctxt->actions) < 0) + return NULL; + } + + return g_steal_pointer(&snapctxt); +} + + +static qemuSnapshotDiskContextPtr +qemuSnapshotDiskPrepareDisksTransient(virDomainObjPtr vm, + virQEMUDriverConfigPtr cfg, + virHashTablePtr blockNamedNodeData, + qemuDomainAsyncJob asyncJob) +{ + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + size_t i; + + snapctxt = qemuSnapshotDiskContextNew(vm->def->ndisks, vm, asyncJob); + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr domdisk = vm->def->disks[i]; + g_autoptr(virDomainSnapshotDiskDef) snapdisk = g_new0(virDomainSnapshotDiskDef, 1); + + if (!domdisk->transient) + continue; + + /* validation code makes sure that we do this only for local disks + * with a file source */ + snapdisk->name = g_strdup(domdisk->dst); + snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + snapdisk->src = virStorageSourceNew(); + snapdisk->src->type = VIR_STORAGE_TYPE_FILE; + snapdisk->src->format = VIR_STORAGE_FILE_QCOW2; + snapdisk->src->path = g_strdup_printf("%s.TRANSIENT", domdisk->src->path); + if (virFileExists(snapdisk->src->path)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Overlay file '%s' for transient disk '%s' already exists"), + snapdisk->src->path, domdisk->dst); + return NULL; + } + + if (qemuSnapshotDiskPrepareOne(vm, cfg, domdisk, snapdisk, + snapctxt->dd + snapctxt->ndd++, + blockNamedNodeData, + false, + false, asyncJob, snapctxt->actions) < 0) return NULL; @@ -1253,6 +1303,45 @@ qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm, }
+/** + * qemuSnapshotCreateDisksTransient: + * @vm: domain object + * @asyncJob: qemu async job type + * + * Creates overlays on top of disks which are configured as <transient/>. Note + * that the validation code ensures that <transient> disks have appropriate + * configuration. + */ +int +qemuSnapshotCreateDisksTransient(virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + g_autoptr(virHashTable) blockNamedNodeData = NULL; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob))) + return -1; + + if (!(snapctxt = qemuSnapshotDiskPrepareDisksTransient(vm, cfg, + blockNamedNodeData, + asyncJob))) + return -1; + + if (qemuSnapshotDiskCreate(snapctxt, cfg) < 0) + return -1; + } + + /* the overlays are established, so they can be deleted on a shutdown */ + priv->inhibitDiskTransientDelete = false; + + return 0; +} + + static int qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_snapshot.h b/src/qemu/qemu_snapshot.h index 8b3ebe87b1..cf9bfec542 100644 --- a/src/qemu/qemu_snapshot.h +++ b/src/qemu/qemu_snapshot.h @@ -21,6 +21,7 @@ #include "virconftypes.h" #include "datatypes.h" #include "qemu_conf.h" +#include "qemu_domainjob.h"
virDomainMomentObjPtr qemuSnapObjFromName(virDomainObjPtr vm, @@ -53,3 +54,7 @@ int qemuSnapshotDelete(virDomainObjPtr vm, virDomainSnapshotPtr snapshot, unsigned int flags); + +int +qemuSnapshotCreateDisksTransient(virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); --
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks a lot! Masa
On 9/24/20 6:43 AM, Peter Krempa wrote:
To implement <transient/> disks we'll need to install an overlay on top of the original disk image which will be discarded after the VM is turned off. This was initially implemented by qemu but libvirt never picked up this option. With blockdev the qemu feature became unsupported
You may want to add mention that qemu's -transient command-line option lets qemu create the file, which fails under various SELinux scenarios (as one of the reasons why libvirt never used it).
so we need to do this via the snapshot code anyways.
The helpers introduced in this patch prepare a fake snapshot disk definition for a disk which is configured as <transient/> and use it to create a snapshot (without actually modifying metada or persistent def).
metadata
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 91 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_snapshot.h | 5 +++ 2 files changed, 95 insertions(+), 1 deletion(-)
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On a Thursday in 2020, Peter Krempa wrote:
To implement <transient/> disks we'll need to install an overlay on top of the original disk image which will be discarded after the VM is turned off. This was initially implemented by qemu but libvirt never picked up this option. With blockdev the qemu feature became unsupported so we need to do this via the snapshot code anyways.
The helpers introduced in this patch prepare a fake snapshot disk definition for a disk which is configured as <transient/> and use it to create a snapshot (without actually modifying metada or persistent def).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 91 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_snapshot.h | 5 +++ 2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index ca051071aa..d10e7b6b3a 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -986,6 +986,7 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, qemuSnapshotDiskDataPtr dd, virHashTablePtr blockNamedNodeData, bool reuse, + bool updateConfig, qemuDomainAsyncJob asyncJob, virJSONValuePtr actions) { @@ -1008,7 +1009,8 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, return -1;
/* modify disk in persistent definition only when the source is the same */ - if (vm->newDef && + if (updateConfig && + vm->newDef && (persistdisk = virDomainDiskByTarget(vm->newDef, dd->disk->dst)) && virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) {
@@ -1116,6 +1118,54 @@ qemuSnapshotDiskPrepareActiveExternal(virDomainObjPtr vm, snapctxt->dd + snapctxt->ndd++, blockNamedNodeData, reuse, + true, + asyncJob, + snapctxt->actions) < 0) + return NULL; + } + + return g_steal_pointer(&snapctxt); +} + + +static qemuSnapshotDiskContextPtr +qemuSnapshotDiskPrepareDisksTransient(virDomainObjPtr vm, + virQEMUDriverConfigPtr cfg, + virHashTablePtr blockNamedNodeData, + qemuDomainAsyncJob asyncJob) +{ + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + size_t i; + + snapctxt = qemuSnapshotDiskContextNew(vm->def->ndisks, vm, asyncJob); + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr domdisk = vm->def->disks[i]; + g_autoptr(virDomainSnapshotDiskDef) snapdisk = g_new0(virDomainSnapshotDiskDef, 1); + + if (!domdisk->transient) + continue; + + /* validation code makes sure that we do this only for local disks + * with a file source */ + snapdisk->name = g_strdup(domdisk->dst); + snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + snapdisk->src = virStorageSourceNew(); + snapdisk->src->type = VIR_STORAGE_TYPE_FILE; + snapdisk->src->format = VIR_STORAGE_FILE_QCOW2; + snapdisk->src->path = g_strdup_printf("%s.TRANSIENT", domdisk->src->path);
I'd prefer: a) an empty line between these assignments and the if() b) a lowercase name, but I guess standing out is important here as the file is not meant to stay.
+ if (virFileExists(snapdisk->src->path)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Overlay file '%s' for transient disk '%s' already exists"), + snapdisk->src->path, domdisk->dst); + return NULL; + } + + if (qemuSnapshotDiskPrepareOne(vm, cfg, domdisk, snapdisk, + snapctxt->dd + snapctxt->ndd++, + blockNamedNodeData, + false, + false, asyncJob, snapctxt->actions) < 0) return NULL; @@ -1253,6 +1303,45 @@ qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm, }
+/** + * qemuSnapshotCreateDisksTransient: + * @vm: domain object + * @asyncJob: qemu async job type + * + * Creates overlays on top of disks which are configured as <transient/>. Note + * that the validation code ensures that <transient> disks have appropriate + * configuration. + */ +int +qemuSnapshotCreateDisksTransient(virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + g_autoptr(virHashTable) blockNamedNodeData = NULL; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob))) + return -1; + + if (!(snapctxt = qemuSnapshotDiskPrepareDisksTransient(vm, cfg, + blockNamedNodeData, + asyncJob))) + return -1; + + if (qemuSnapshotDiskCreate(snapctxt, cfg) < 0) + return -1; + } + + /* the overlays are established, so they can be deleted on a shutdown */
*on shutdown sounds better to me
+ priv->inhibitDiskTransientDelete = false; + + return 0; +} + + static int qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver, virDomainObjPtr vm,
With the empty line added: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> For now we disallow blockjobs with transient disks to avoid dealing with obsoleted overlays. Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> --- src/qemu/qemu_domain.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dc5949edfa..0331fd55e0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10859,6 +10859,13 @@ qemuDomainDiskBlockJobIsSupported(virDomainObjPtr vm, return false; } + if (disk->transient) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("block jobs are not supported on transient disk '%s'"), + disk->dst); + return false; + } + return true; } -- 2.26.2
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> For now we disable disk hotplug of transient disk as it requires creating an overlay prior to adding the frontend. Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> --- src/qemu/qemu_hotplug.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fc0866c011..ed4d1580fa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1031,6 +1031,12 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, return -1; } + if (disk->transient) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("transient disk hotplug isn't supported")); + return -1; + } + if (virDomainDiskTranslateSourcePool(disk) < 0) goto cleanup; -- 2.26.2
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Block migration when transient disk option is enabled to simplify the handling of the overlay files. Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> --- src/qemu/qemu_migration.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5708334d2f..2f24d56312 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1394,6 +1394,16 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, _("cannot migrate this domain without dbus-vmstate support")); return false; } + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + if (disk->transient) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("migration with transient disk is not supported")); + return false; + } + } } return true; -- 2.26.2
Remove the overlay if the disk was <transient/>. Note that even if we'd forbid unplug of such a disk through the API, the disk can still be ejected from the guest. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ed4d1580fa..11b549b12b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4313,6 +4313,15 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup; + if (disk->transient) { + VIR_DEBUG("Removing transient overlay '%s' of disk '%s'", + disk->src->path, disk->dst); + if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) >= 0) { + virStorageFileUnlink(disk->src); + virStorageFileDeinit(disk->src); + } + } + ret = 0; cleanup: -- 2.26.2
On Thu, Sep 24, 2020 at 01:43:54PM +0200, Peter Krempa wrote:
Remove the overlay if the disk was <transient/>. Note that even if we'd forbid unplug of such a disk through the API, the disk can still be ejected from the guest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ed4d1580fa..11b549b12b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4313,6 +4313,15 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup;
+ if (disk->transient) { + VIR_DEBUG("Removing transient overlay '%s' of disk '%s'", + disk->src->path, disk->dst); + if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) >= 0) { + virStorageFileUnlink(disk->src); + virStorageFileDeinit(disk->src); + } + } + ret = 0;
cleanup: --
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks a lot! Masa
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Add overlays after the VM starts before we start executing guest code. Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ffb3afa9c5..9122069cc9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -60,6 +60,7 @@ #include "qemu_firmware.h" #include "qemu_backup.h" #include "qemu_dbus.h" +#include "qemu_snapshot.h" #include "cpu/cpu.h" #include "cpu/cpu_x86.h" @@ -7077,6 +7078,10 @@ qemuProcessLaunch(virConnectPtr conn, qemuProcessAutoDestroyAdd(driver, vm, conn) < 0) goto cleanup; + VIR_DEBUG("Setting up transient disk"); + if (qemuSnapshotCreateDisksTransient(vm, asyncJob) < 0) + goto cleanup; + ret = 0; cleanup: -- 2.26.2
On Thu, Sep 24, 2020 at 01:43:55PM +0200, Peter Krempa wrote:
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Add overlays after the VM starts before we start executing guest code.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ffb3afa9c5..9122069cc9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -60,6 +60,7 @@ #include "qemu_firmware.h" #include "qemu_backup.h" #include "qemu_dbus.h" +#include "qemu_snapshot.h"
#include "cpu/cpu.h" #include "cpu/cpu_x86.h" @@ -7077,6 +7078,10 @@ qemuProcessLaunch(virConnectPtr conn, qemuProcessAutoDestroyAdd(driver, vm, conn) < 0) goto cleanup;
+ VIR_DEBUG("Setting up transient disk"); + if (qemuSnapshotCreateDisksTransient(vm, asyncJob) < 0) + goto cleanup; + ret = 0;
cleanup: --
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks a lot! Masa
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Extract the validation of transient disk option. We support transient disks in qemu under the following conditions: - -blockdev is used - the disk source is a local file - the disk type is 'disk' - the disk is not readonly Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 5 ++-- src/qemu/qemu_validate.c | 56 +++++++++++++++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 888db5ea29..cc1467c0e6 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2974,8 +2974,9 @@ paravirtualized driver is specified via the ``disk`` element. ``transient`` If present, this indicates that changes to the device contents should be reverted automatically when the guest exits. With some hypervisors, marking a - disk transient prevents the domain from participating in migration or - snapshots. Only suppported in vmx hypervisor. :since:`Since 0.9.5` + disk transient prevents the domain from participating in migration, + snapshots, or blockjobs. Only suppported in vmx hypervisor + (:since:`Since 0.9.5`) and ``qemu`` hypervisor (:since:`Since 6.9.0`). ``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/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 3ed4039cdf..3196814aca 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2186,12 +2186,6 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } } - if (disk->transient) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("transient disks not supported yet")); - return -1; - } - if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { @@ -2340,6 +2334,53 @@ qemuValidateDomainDeviceDefDiskBlkdeviotune(const virDomainDiskDef *disk, } +static int +qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk, + virQEMUCapsPtr qemuCaps) + +{ + virStorageType actualType = virStorageSourceGetActualType(disk->src); + + if (!disk->transient) + return 0; + + if (virStorageSourceIsEmpty(disk->src)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk '%s' must not be empty"), disk->dst); + return -1; + } + + if (disk->src->readonly) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk '%s' must not be read-only"), disk->dst); + return -1; + } + + if (actualType != VIR_STORAGE_TYPE_FILE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk supported only with 'file' type (%s)"), + disk->dst); + return -1; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk supported only with 'disk' device (%s)"), + disk->dst); + return -1; + } + + if (qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk not supported by this qemu binary (%s)"), + disk->dst); + return -1; + } + + return 0; +} + + int qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, const virDomainDef *def, @@ -2357,6 +2398,9 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, if (qemuValidateDomainDeviceDefDiskBlkdeviotune(disk, def, qemuCaps) < 0) return -1; + if (qemuValidateDomainDeviceDefDiskTransient(disk, qemuCaps) < 0) + return -1; + if (disk->src->shared && !disk->src->readonly && !qemuBlockStorageSourceSupportsConcurrentAccess(disk->src)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 2.26.2
On Thu, Sep 24, 2020 at 01:43:56PM +0200, Peter Krempa wrote:
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Extract the validation of transient disk option. We support transient disks in qemu under the following conditions:
- -blockdev is used - the disk source is a local file - the disk type is 'disk' - the disk is not readonly
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 5 ++-- src/qemu/qemu_validate.c | 56 +++++++++++++++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 888db5ea29..cc1467c0e6 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2974,8 +2974,9 @@ paravirtualized driver is specified via the ``disk`` element. ``transient`` If present, this indicates that changes to the device contents should be reverted automatically when the guest exits. With some hypervisors, marking a - disk transient prevents the domain from participating in migration or - snapshots. Only suppported in vmx hypervisor. :since:`Since 0.9.5` + disk transient prevents the domain from participating in migration, + snapshots, or blockjobs. Only suppported in vmx hypervisor + (:since:`Since 0.9.5`) and ``qemu`` hypervisor (:since:`Since 6.9.0`). ``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/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 3ed4039cdf..3196814aca 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2186,12 +2186,6 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } }
- if (disk->transient) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("transient disks not supported yet")); - return -1; - } - if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { @@ -2340,6 +2334,53 @@ qemuValidateDomainDeviceDefDiskBlkdeviotune(const virDomainDiskDef *disk, }
+static int +qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk, + virQEMUCapsPtr qemuCaps) + +{ + virStorageType actualType = virStorageSourceGetActualType(disk->src); + + if (!disk->transient) + return 0; + + if (virStorageSourceIsEmpty(disk->src)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk '%s' must not be empty"), disk->dst); + return -1; + } + + if (disk->src->readonly) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk '%s' must not be read-only"), disk->dst); + return -1; + } + + if (actualType != VIR_STORAGE_TYPE_FILE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk supported only with 'file' type (%s)"), + disk->dst); + return -1; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk supported only with 'disk' device (%s)"), + disk->dst); + return -1; + } + + if (qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk not supported by this qemu binary (%s)"), + disk->dst); + return -1; + } + + return 0; +} + + int qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, const virDomainDef *def, @@ -2357,6 +2398,9 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, if (qemuValidateDomainDeviceDefDiskBlkdeviotune(disk, def, qemuCaps) < 0) return -1;
+ if (qemuValidateDomainDeviceDefDiskTransient(disk, qemuCaps) < 0) + return -1; + if (disk->src->shared && !disk->src->readonly && !qemuBlockStorageSourceSupportsConcurrentAccess(disk->src)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, --
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks a lot! Masa
On a Thursday in 2020, Peter Krempa wrote:
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Extract the validation of transient disk option. We support transient disks in qemu under the following conditions:
- -blockdev is used - the disk source is a local file - the disk type is 'disk' - the disk is not readonly
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 5 ++-- src/qemu/qemu_validate.c | 56 +++++++++++++++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 888db5ea29..cc1467c0e6 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2974,8 +2974,9 @@ paravirtualized driver is specified via the ``disk`` element. ``transient`` If present, this indicates that changes to the device contents should be reverted automatically when the guest exits. With some hypervisors, marking a - disk transient prevents the domain from participating in migration or - snapshots. Only suppported in vmx hypervisor. :since:`Since 0.9.5` + disk transient prevents the domain from participating in migration, + snapshots, or blockjobs. Only suppported in vmx hypervisor
I'd say: *supported but that's pre-existing.
+ (:since:`Since 0.9.5`) and ``qemu`` hypervisor (:since:`Since 6.9.0`). ``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/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 3ed4039cdf..3196814aca 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2186,12 +2186,6 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } }
- if (disk->transient) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("transient disks not supported yet")); - return -1; - } - if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { @@ -2340,6 +2334,53 @@ qemuValidateDomainDeviceDefDiskBlkdeviotune(const virDomainDiskDef *disk, }
+static int +qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk, + virQEMUCapsPtr qemuCaps) + +{ + virStorageType actualType = virStorageSourceGetActualType(disk->src); + + if (!disk->transient) + return 0; + + if (virStorageSourceIsEmpty(disk->src)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk '%s' must not be empty"), disk->dst); + return -1; + } + + if (disk->src->readonly) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk '%s' must not be read-only"), disk->dst); + return -1; + } + + if (actualType != VIR_STORAGE_TYPE_FILE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk supported only with 'file' type (%s)"), + disk->dst); + return -1; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk supported only with 'disk' device (%s)"), + disk->dst); + return -1; + } + + if (qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transient disk not supported by this qemu binary (%s)"),
s/qemu/QEMU/ Jano
+ disk->dst); + return -1; + } + + return 0; +} + + int qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, const virDomainDef *def,
We didn't actually use this file. Change the disk type to 'file' so that it works in qemu and add pre and post-blockdev invocations. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../disk-transient.x86_64-4.1.0.err | 1 + .../disk-transient.x86_64-latest.args | 42 +++++++++++++++++++ tests/qemuxml2argvdata/disk-transient.xml | 4 +- tests/qemuxml2argvtest.c | 2 + 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-latest.args diff --git a/tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err b/tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err new file mode 100644 index 0000000000..341902e144 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err @@ -0,0 +1 @@ +unsupported configuration: transient disk not supported by this qemu binary (hda) diff --git a/tests/qemuxml2argvdata/disk-transient.x86_64-latest.args b/tests/qemuxml2argvdata/disk-transient.x86_64-latest.args new file mode 100644 index 0000000000..1b9c64c96a --- /dev/null +++ b/tests/qemuxml2argvdata/disk-transient.x86_64-latest.args @@ -0,0 +1,42 @@ +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 \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-cpu qemu64 \ +-m 214 \ +-overcommit mem-lock=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,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-blockdev '{"driver":"file","filename":"/tmp/QEMUGuest1.img",\ +"node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},\ +"auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,\ +"cache":{"direct":true,"no-flush":false},"driver":"qcow2",\ +"file":"libvirt-1-storage"}' \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1,\ +write-cache=on \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-transient.xml b/tests/qemuxml2argvdata/disk-transient.xml index 00ddb6487a..cffb325336 100644 --- a/tests/qemuxml2argvdata/disk-transient.xml +++ b/tests/qemuxml2argvdata/disk-transient.xml @@ -14,9 +14,9 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-i386</emulator> - <disk type='block' device='disk'> + <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> - <source dev='/dev/HostVG/QEMUGuest1'/> + <source file='/tmp/QEMUGuest1.img'/> <target dev='hda' bus='ide'/> <transient/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a3c91fd5de..9da74adb60 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1171,6 +1171,8 @@ mymain(void) DO_TEST_CAPS_VER("disk-cache", "2.7.0"); DO_TEST_CAPS_VER("disk-cache", "2.12.0"); DO_TEST_CAPS_LATEST("disk-cache"); + DO_TEST_CAPS_ARCH_VER_PARSE_ERROR("disk-transient", "x86_64", "4.1.0"); + DO_TEST_CAPS_LATEST("disk-transient"); DO_TEST("disk-network-nbd", NONE); DO_TEST_CAPS_VER("disk-network-nbd", "2.12.0"); DO_TEST_CAPS_LATEST("disk-network-nbd"); -- 2.26.2
--- TODO | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 TODO diff --git a/TODO b/TODO new file mode 100644 index 0000000000..e69de29bb2 -- 2.26.2
On 9/24/20 6:43 AM, Peter Krempa wrote:
--- TODO | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 TODO
diff --git a/TODO b/TODO new file mode 100644 index 0000000000..e69de29bb2
TODO: add my review here... -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On a Thursday in 2020, Peter Krempa wrote:
This version: - is rebased on top of current refactors - has more strict rules in the validation - adds tests - fixes bug with disk-unplug
Masayoshi Mizuma (5): qemu: Block blockjobs when transient disk option is enabled qemu: Block disk hotplug when transient disk option is enabled qemu: Block migration when transient disk option is enabled qemu: process: Handle transient disks on VM startup qemu: validate: Allow <transient/> disks
Peter Krempa (6): virDomainSnapshotDiskDefFree: Export and register as autoptr func qemu: prepare cleanup for <transient/> disk overlays qemu: snapshot: Introduce helpers for creating overlays on <transient/> disks qemu: hotplug: Remove overlay of <transient> disk on disk unplug tests: qemuxml2argv: Fix and enable 'disk-transient' case
Up to here: Reviewed-by: Ján Tomko <jtomko@redhat.com> Tested-by: Ján Tomko <jtomko@redhat.com> Thank you for this feature!
TODO: Add news entry
If you wait until another feature freeze with writing the NEWS entry, you can get a reminder about it from Andrea. That is surely worth it. Jano
TODO | 0 docs/formatdomain.rst | 5 +- src/conf/snapshot_conf.h | 5 + src/conf/snapshot_conf_priv.h | 3 - src/qemu/qemu_domain.c | 9 ++ src/qemu/qemu_domain.h | 4 + src/qemu/qemu_hotplug.c | 15 +++ src/qemu/qemu_migration.c | 10 ++ src/qemu/qemu_process.c | 27 ++++++ src/qemu/qemu_snapshot.c | 91 ++++++++++++++++++- src/qemu/qemu_snapshot.h | 5 + src/qemu/qemu_validate.c | 56 ++++++++++-- .../disk-transient.x86_64-4.1.0.err | 1 + .../disk-transient.x86_64-latest.args | 42 +++++++++ tests/qemuxml2argvdata/disk-transient.xml | 4 +- tests/qemuxml2argvtest.c | 2 + 16 files changed, 265 insertions(+), 14 deletions(-) create mode 100644 TODO create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-latest.args
-- 2.26.2
participants (4)
-
Eric Blake -
Ján Tomko -
Masayoshi Mizuma -
Peter Krempa