Re: [PATCH 5/6] qemuSnapshotCreateActiveExternalDisks: Extract actual snapshot creation to 'qemuSnapshotDiskCreate'

Extract the code which invokes the monitor and finalizes the snapshot into a separate function for easier reuse. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- 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