[PATCH 0/6] qemu: snapshot: Refactors prior to introduction of <transient> disk support
We'll be installing overlays on top of <transient> disks via the snapshot code. Prepare some aspects. Peter Krempa (6): qemu: snapshot: Rename 'qemuSnapshotCreateDiskActive' to 'qemuSnapshotCreateActiveExternalDisks' qemuSnapshotDiskUpdateSource: Extract 'driver' and 'blockdev' from 'vm' qemuSnapshotDiskPrepare/Cleanup: simplify passing of 'driver' and 'blockdev' qemu: snapshot: Introduce qemuSnapshotDiskContext qemuSnapshotCreateActiveExternalDisks: Extract actual snapshot creation to 'qemuSnapshotDiskCreate' qemuSnapshotDiskPrepare: rename to qemuSnapshotDiskPrepareActiveExternal src/qemu/qemu_snapshot.c | 215 ++++++++++++++++++++++----------------- 1 file changed, 124 insertions(+), 91 deletions(-) -- 2.26.2
Be more specific about the role of the function. It's creating the disk portion of an external active snapshot. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 48a4e9dd41..0435d4c371 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1151,13 +1151,13 @@ qemuSnapshotDiskUpdateSource(virQEMUDriverPtr driver, /* The domain is expected to be locked and active. */ static int -qemuSnapshotCreateDiskActive(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMomentObjPtr snap, - virHashTablePtr blockNamedNodeData, - unsigned int flags, - virQEMUDriverConfigPtr cfg, - qemuDomainAsyncJob asyncJob) +qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMomentObjPtr snap, + virHashTablePtr blockNamedNodeData, + unsigned int flags, + virQEMUDriverConfigPtr cfg, + qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(virJSONValue) actions = NULL; @@ -1346,9 +1346,9 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver, /* the domain is now paused if a memory snapshot was requested */ - if ((ret = qemuSnapshotCreateDiskActive(driver, vm, snap, - blockNamedNodeData, flags, cfg, - QEMU_ASYNC_JOB_SNAPSHOT)) < 0) + if ((ret = qemuSnapshotCreateActiveExternalDisks(driver, vm, snap, + blockNamedNodeData, flags, cfg, + QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; /* the snapshot is complete now */ -- 2.26.2
Reduce the number of arguments by taking them from 'vm'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 0435d4c371..d6e0a75996 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1103,19 +1103,18 @@ qemuSnapshotDiskUpdateSourceRenumber(virStorageSourcePtr src) /** * qemuSnapshotDiskUpdateSource: - * @driver: QEMU driver * @vm: domain object * @dd: snapshot disk data object - * @blockdev: -blockdev is in use for the VM * * Updates disk definition after a successful snapshot. */ static void -qemuSnapshotDiskUpdateSource(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuSnapshotDiskDataPtr dd, - bool blockdev) +qemuSnapshotDiskUpdateSource(virDomainObjPtr vm, + qemuSnapshotDiskDataPtr dd) { + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + /* storage driver access won'd be needed */ if (dd->initialized) virStorageFileDeinit(dd->src); @@ -1138,7 +1137,7 @@ qemuSnapshotDiskUpdateSource(virQEMUDriverPtr driver, dd->disk->src = g_steal_pointer(&dd->src); /* fix numbering of disks */ - if (!blockdev) + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) qemuSnapshotDiskUpdateSourceRenumber(dd->disk->src); if (dd->persistdisk) { @@ -1201,7 +1200,7 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); if (rc == 0) - qemuSnapshotDiskUpdateSource(driver, vm, dd, blockdev); + qemuSnapshotDiskUpdateSource(vm, dd); } if (rc < 0) -- 2.26.2
Both can be fetched from 'vm'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index d6e0a75996..6fdc187d83 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -792,10 +792,11 @@ typedef qemuSnapshotDiskData *qemuSnapshotDiskDataPtr; static void qemuSnapshotDiskCleanup(qemuSnapshotDiskDataPtr data, size_t ndata, - virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob) { + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; virErrorPtr orig_err; size_t i; @@ -931,18 +932,19 @@ qemuSnapshotDiskPrepareOneBlockdev(virQEMUDriverPtr driver, static int -qemuSnapshotDiskPrepareOne(virQEMUDriverPtr driver, - virDomainObjPtr vm, +qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, virQEMUDriverConfigPtr cfg, virDomainDiskDefPtr disk, virDomainSnapshotDiskDefPtr snapdisk, qemuSnapshotDiskDataPtr dd, virHashTablePtr blockNamedNodeData, bool reuse, - bool blockdev, qemuDomainAsyncJob asyncJob, virJSONValuePtr actions) { + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); virDomainDiskDefPtr persistdisk; bool supportsCreate; bool updateRelativeBacking = false; @@ -1045,12 +1047,10 @@ qemuSnapshotDiskPrepareOne(virQEMUDriverPtr driver, * that are selected for the snapshot. */ static int -qemuSnapshotDiskPrepare(virQEMUDriverPtr driver, - virDomainObjPtr vm, +qemuSnapshotDiskPrepare(virDomainObjPtr vm, virDomainMomentObjPtr snap, virQEMUDriverConfigPtr cfg, bool reuse, - bool blockdev, virHashTablePtr blockNamedNodeData, qemuDomainAsyncJob asyncJob, qemuSnapshotDiskDataPtr *rdata, @@ -1070,11 +1070,11 @@ qemuSnapshotDiskPrepare(virQEMUDriverPtr driver, if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) continue; - if (qemuSnapshotDiskPrepareOne(driver, vm, cfg, vm->def->disks[i], + if (qemuSnapshotDiskPrepareOne(vm, cfg, vm->def->disks[i], snapdef->disks + i, data + ndata++, blockNamedNodeData, - reuse, blockdev, + reuse, asyncJob, actions) < 0) goto cleanup; @@ -1085,7 +1085,7 @@ qemuSnapshotDiskPrepare(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuSnapshotDiskCleanup(data, ndata, driver, vm, asyncJob); + qemuSnapshotDiskCleanup(data, ndata, vm, asyncJob); return ret; } @@ -1166,7 +1166,6 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; qemuSnapshotDiskDataPtr diskdata = NULL; size_t ndiskdata = 0; - bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); if (virDomainObjCheckActive(vm) < 0) return -1; @@ -1175,7 +1174,7 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (qemuSnapshotDiskPrepare(driver, vm, snap, cfg, reuse, blockdev, + if (qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, blockNamedNodeData, asyncJob, &diskdata, &ndiskdata, actions) < 0) goto cleanup; @@ -1214,7 +1213,7 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuSnapshotDiskCleanup(diskdata, ndiskdata, driver, vm, asyncJob); + qemuSnapshotDiskCleanup(diskdata, ndiskdata, vm, asyncJob); return ret; } -- 2.26.2
Add a container struct which holds all data needed to create and clean up after a (for now external) snapshot. This will aggregate all the 'qemuSnapshotDiskDataPtr' the 'actions' of a transaction QMP command and everything needed for cleanup at any given point. This aggregation allows to simplify the arguments of the functions which prepare the snapshot data and additionally will simplify the code necessary for creating overlays on top of <transient/> disks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 112 +++++++++++++++++++++++---------------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 6fdc187d83..b9b640058c 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -842,6 +842,52 @@ qemuSnapshotDiskCleanup(qemuSnapshotDiskDataPtr data, } +struct _qemuSnapshotDiskContext { + qemuSnapshotDiskDataPtr dd; + size_t ndd; + + virJSONValuePtr actions; + + /* needed for automatic cleanup of 'dd' */ + virDomainObjPtr vm; + qemuDomainAsyncJob asyncJob; +}; + +typedef struct _qemuSnapshotDiskContext qemuSnapshotDiskContext; +typedef qemuSnapshotDiskContext *qemuSnapshotDiskContextPtr; + + +static qemuSnapshotDiskContextPtr +qemuSnapshotDiskContextNew(size_t ndisks, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuSnapshotDiskContextPtr ret = g_new0(qemuSnapshotDiskContext, 1); + + ret->dd = g_new0(qemuSnapshotDiskData, ndisks); + ret->ndd = ndisks; + ret->actions = virJSONValueNewArray(); + ret->vm = vm; + ret->asyncJob = asyncJob; + + return ret; +} + + +static void +qemuSnapshotDiskContextCleanup(qemuSnapshotDiskContextPtr snapctxt) +{ + if (!snapctxt) + return; + + virJSONValueFree(snapctxt->actions); + + qemuSnapshotDiskCleanup(snapctxt->dd, snapctxt->ndd, snapctxt->vm, snapctxt->asyncJob); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuSnapshotDiskContext, qemuSnapshotDiskContextCleanup); + + /** * qemuSnapshotDiskBitmapsPropagate: * @@ -1046,25 +1092,19 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, * Collects and prepares a list of structures that hold information about disks * that are selected for the snapshot. */ -static int +static qemuSnapshotDiskContextPtr qemuSnapshotDiskPrepare(virDomainObjPtr vm, virDomainMomentObjPtr snap, virQEMUDriverConfigPtr cfg, bool reuse, virHashTablePtr blockNamedNodeData, - qemuDomainAsyncJob asyncJob, - qemuSnapshotDiskDataPtr *rdata, - size_t *rndata, - virJSONValuePtr actions) + qemuDomainAsyncJob asyncJob) { + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; size_t i; - qemuSnapshotDiskDataPtr data; - size_t ndata = 0; virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); - int ret = -1; - if (VIR_ALLOC_N(data, snapdef->ndisks) < 0) - return -1; + snapctxt = qemuSnapshotDiskContextNew(snapdef->ndisks, vm, asyncJob); for (i = 0; i < snapdef->ndisks; i++) { if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) @@ -1072,21 +1112,15 @@ qemuSnapshotDiskPrepare(virDomainObjPtr vm, if (qemuSnapshotDiskPrepareOne(vm, cfg, vm->def->disks[i], snapdef->disks + i, - data + ndata++, + snapctxt->dd + snapctxt->ndd++, blockNamedNodeData, reuse, asyncJob, - actions) < 0) - goto cleanup; + snapctxt->actions) < 0) + return NULL; } - *rdata = g_steal_pointer(&data); - *rndata = ndata; - ret = 0; - - cleanup: - qemuSnapshotDiskCleanup(data, ndata, vm, asyncJob); - return ret; + return g_steal_pointer(&snapctxt); } @@ -1159,42 +1193,34 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - g_autoptr(virJSONValue) actions = NULL; int rc; - int ret = -1; size_t i; bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; - qemuSnapshotDiskDataPtr diskdata = NULL; - size_t ndiskdata = 0; + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; if (virDomainObjCheckActive(vm) < 0) return -1; - actions = virJSONValueNewArray(); - /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, - blockNamedNodeData, asyncJob, - &diskdata, &ndiskdata, actions) < 0) - goto cleanup; + if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, + blockNamedNodeData, asyncJob))) + return -1; /* check whether there's anything to do */ - if (ndiskdata == 0) { - ret = 0; - goto cleanup; - } + if (snapctxt->ndd == 0) + return 0; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto cleanup; + return -1; - rc = qemuMonitorTransaction(priv->mon, &actions); + rc = qemuMonitorTransaction(priv->mon, &snapctxt->actions); if (qemuDomainObjExitMonitor(driver, vm) < 0) rc = -1; - for (i = 0; i < ndiskdata; i++) { - qemuSnapshotDiskDataPtr dd = &diskdata[i]; + for (i = 0; i < snapctxt->ndd; i++) { + qemuSnapshotDiskDataPtr dd = snapctxt->dd + i; virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); @@ -1203,18 +1229,14 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, } if (rc < 0) - goto cleanup; + return -1; if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 || (vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt, cfg->configDir) < 0)) - goto cleanup; - - ret = 0; + return -1; - cleanup: - qemuSnapshotDiskCleanup(diskdata, ndiskdata, vm, asyncJob); - return ret; + return 0; } -- 2.26.2
Extract the code which invokes the monitor and finalizes the snapshot into a separate function for easier reuse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 67 ++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index b9b640058c..6fe925763e 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1182,57 +1182,42 @@ qemuSnapshotDiskUpdateSource(virDomainObjPtr vm, } -/* The domain is expected to be locked and active. */ static int -qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMomentObjPtr snap, - virHashTablePtr blockNamedNodeData, - unsigned int flags, - virQEMUDriverConfigPtr cfg, - qemuDomainAsyncJob asyncJob) +qemuSnapshotDiskCreate(qemuSnapshotDiskContextPtr snapctxt, + virQEMUDriverConfigPtr cfg) { - qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; + qemuDomainObjPrivatePtr priv = snapctxt->vm->privateData; + virQEMUDriverPtr driver = priv->driver; size_t i; - bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; - g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; - - if (virDomainObjCheckActive(vm) < 0) - return -1; + int rc; - /* prepare a list of objects to use in the vm definition so that we don't - * have to roll back later */ - if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, - blockNamedNodeData, asyncJob))) - return -1; /* check whether there's anything to do */ if (snapctxt->ndd == 0) return 0; - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + if (qemuDomainObjEnterMonitorAsync(driver, snapctxt->vm, snapctxt->asyncJob) < 0) return -1; rc = qemuMonitorTransaction(priv->mon, &snapctxt->actions); - if (qemuDomainObjExitMonitor(driver, vm) < 0) + if (qemuDomainObjExitMonitor(driver, snapctxt->vm) < 0) rc = -1; for (i = 0; i < snapctxt->ndd; i++) { qemuSnapshotDiskDataPtr dd = snapctxt->dd + i; - virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); + virDomainAuditDisk(snapctxt->vm, dd->disk->src, dd->src, "snapshot", rc >= 0); if (rc == 0) - qemuSnapshotDiskUpdateSource(vm, dd); + qemuSnapshotDiskUpdateSource(snapctxt->vm, dd); } if (rc < 0) return -1; - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 || - (vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt, + if (virDomainObjSave(snapctxt->vm, driver->xmlopt, cfg->stateDir) < 0 || + (snapctxt->vm->newDef && virDomainDefSave(snapctxt->vm->newDef, driver->xmlopt, cfg->configDir) < 0)) return -1; @@ -1240,6 +1225,34 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, } +/* The domain is expected to be locked and active. */ +static int +qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm, + virDomainMomentObjPtr snap, + virHashTablePtr blockNamedNodeData, + unsigned int flags, + virQEMUDriverConfigPtr cfg, + qemuDomainAsyncJob asyncJob) +{ + bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + + if (virDomainObjCheckActive(vm) < 0) + return -1; + + /* prepare a list of objects to use in the vm definition so that we don't + * have to roll back later */ + if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, + blockNamedNodeData, asyncJob))) + return -1; + + if (qemuSnapshotDiskCreate(snapctxt, cfg) < 0) + return -1; + + return 0; +} + + static int qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -1366,7 +1379,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver, /* the domain is now paused if a memory snapshot was requested */ - if ((ret = qemuSnapshotCreateActiveExternalDisks(driver, vm, snap, + if ((ret = qemuSnapshotCreateActiveExternalDisks(vm, snap, blockNamedNodeData, flags, cfg, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; -- 2.26.2
On a Thursday in 2020, Peter Krempa wrote:
Extract the code which invokes the monitor and finalizes the snapshot into a separate function for easier reuse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 67 ++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index b9b640058c..6fe925763e 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1182,57 +1182,42 @@ qemuSnapshotDiskUpdateSource(virDomainObjPtr vm, }
-/* The domain is expected to be locked and active. */ static int -qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMomentObjPtr snap, - virHashTablePtr blockNamedNodeData, - unsigned int flags, - virQEMUDriverConfigPtr cfg, - qemuDomainAsyncJob asyncJob) +qemuSnapshotDiskCreate(qemuSnapshotDiskContextPtr snapctxt, + virQEMUDriverConfigPtr cfg) { - qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; + qemuDomainObjPrivatePtr priv = snapctxt->vm->privateData; + virQEMUDriverPtr driver = priv->driver; size_t i; - bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; - g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; - - if (virDomainObjCheckActive(vm) < 0) - return -1; + int rc;
- /* prepare a list of objects to use in the vm definition so that we don't - * have to roll back later */ - if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, - blockNamedNodeData, asyncJob))) - return -1;
This leaves a double space here. Jano
/* check whether there's anything to do */ if (snapctxt->ndd == 0) return 0;
- if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + if (qemuDomainObjEnterMonitorAsync(driver, snapctxt->vm, snapctxt->asyncJob) < 0) return -1;
rc = qemuMonitorTransaction(priv->mon, &snapctxt->actions);
- if (qemuDomainObjExitMonitor(driver, vm) < 0) + if (qemuDomainObjExitMonitor(driver, snapctxt->vm) < 0) rc = -1;
for (i = 0; i < snapctxt->ndd; i++) { qemuSnapshotDiskDataPtr dd = snapctxt->dd + i;
- virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); + virDomainAuditDisk(snapctxt->vm, dd->disk->src, dd->src, "snapshot", rc >= 0);
if (rc == 0) - qemuSnapshotDiskUpdateSource(vm, dd); + qemuSnapshotDiskUpdateSource(snapctxt->vm, dd); }
if (rc < 0) return -1;
- if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 || - (vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt, + if (virDomainObjSave(snapctxt->vm, driver->xmlopt, cfg->stateDir) < 0 || + (snapctxt->vm->newDef && virDomainDefSave(snapctxt->vm->newDef, driver->xmlopt, cfg->configDir) < 0)) return -1;
@@ -1240,6 +1225,34 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver, }
+/* The domain is expected to be locked and active. */ +static int +qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm, + virDomainMomentObjPtr snap, + virHashTablePtr blockNamedNodeData, + unsigned int flags, + virQEMUDriverConfigPtr cfg, + qemuDomainAsyncJob asyncJob) +{ + bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; + g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; + + if (virDomainObjCheckActive(vm) < 0) + return -1; + + /* prepare a list of objects to use in the vm definition so that we don't + * have to roll back later */ + if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, + blockNamedNodeData, asyncJob))) + return -1; + + if (qemuSnapshotDiskCreate(snapctxt, cfg) < 0) + return -1; + + return 0; +} + + static int qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -1366,7 +1379,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
/* the domain is now paused if a memory snapshot was requested */
- if ((ret = qemuSnapshotCreateActiveExternalDisks(driver, vm, snap, + if ((ret = qemuSnapshotCreateActiveExternalDisks(vm, snap, blockNamedNodeData, flags, cfg, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; -- 2.26.2
Make it obvious that the snapshot is prepared for the active external snapshot case. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 6fe925763e..5fe9771205 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1087,18 +1087,18 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm, /** - * qemuSnapshotDiskPrepare: + * qemuSnapshotDiskPrepareActiveExternal: * * Collects and prepares a list of structures that hold information about disks * that are selected for the snapshot. */ static qemuSnapshotDiskContextPtr -qemuSnapshotDiskPrepare(virDomainObjPtr vm, - virDomainMomentObjPtr snap, - virQEMUDriverConfigPtr cfg, - bool reuse, - virHashTablePtr blockNamedNodeData, - qemuDomainAsyncJob asyncJob) +qemuSnapshotDiskPrepareActiveExternal(virDomainObjPtr vm, + virDomainMomentObjPtr snap, + virQEMUDriverConfigPtr cfg, + bool reuse, + virHashTablePtr blockNamedNodeData, + qemuDomainAsyncJob asyncJob) { g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; size_t i; @@ -1242,8 +1242,8 @@ qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm, /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse, - blockNamedNodeData, asyncJob))) + if (!(snapctxt = qemuSnapshotDiskPrepareActiveExternal(vm, snap, cfg, reuse, + blockNamedNodeData, asyncJob))) return -1; if (qemuSnapshotDiskCreate(snapctxt, cfg) < 0) -- 2.26.2
On a Thursday in 2020, Peter Krempa wrote:
We'll be installing overlays on top of <transient> disks via the snapshot code. Prepare some aspects.
Peter Krempa (6): qemu: snapshot: Rename 'qemuSnapshotCreateDiskActive' to 'qemuSnapshotCreateActiveExternalDisks' qemuSnapshotDiskUpdateSource: Extract 'driver' and 'blockdev' from 'vm' qemuSnapshotDiskPrepare/Cleanup: simplify passing of 'driver' and 'blockdev' qemu: snapshot: Introduce qemuSnapshotDiskContext qemuSnapshotCreateActiveExternalDisks: Extract actual snapshot creation to 'qemuSnapshotDiskCreate' qemuSnapshotDiskPrepare: rename to qemuSnapshotDiskPrepareActiveExternal
src/qemu/qemu_snapshot.c | 215 ++++++++++++++++++++++----------------- 1 file changed, 124 insertions(+), 91 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa