[libvirt] [PATCH v2 0/7] qemu: checkpoints: Collect most code in a single place

The checkpoint code is quite complex and was dispersed in many places. Refactor it to be in one new separate file. I also plan to do the same to the snapshot code once this is dealt with. Additionally aggregating all the code in one place will allow refactoring and reuse in the incremental backup implementation. diff to v1: - rebased on top of the patches to remove checking of AUTODESTROY - kept only one instance of virCheckFlags per API (inside qemu_checkpoint.c) Peter Krempa (7): qemu: Move, rename and export qemuDomObjFromDomain conf: Drop pointless 'domain' argument from virDomainCheckpointRedefinePrep conf: Drop pointless 'domain' argument from virDomainSnapshotRedefinePrep qemu: driver: Remove misplaced qemuDomainObjEndJob in qemuDomainCheckpointGetXMLDesc qemu: driver: Move checkpoint-related code to qemu_checkpoint.c qemu: domain: Move checkpoint related code to qemu_checkpoint.c qemu: driver: Don't pull in qemu_monitor_json.h directly src/conf/checkpoint_conf.c | 7 +- src/conf/checkpoint_conf.h | 3 +- src/conf/snapshot_conf.c | 5 +- src/conf/snapshot_conf.h | 3 +- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_checkpoint.c | 646 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_checkpoint.h | 55 +++ src/qemu/qemu_domain.c | 193 ++--------- src/qemu/qemu_domain.h | 16 +- src/qemu/qemu_driver.c | 689 ++++++++----------------------------- src/test/test_driver.c | 4 +- 11 files changed, 887 insertions(+), 736 deletions(-) create mode 100644 src/qemu/qemu_checkpoint.c create mode 100644 src/qemu/qemu_checkpoint.h -- 2.21.0

Move it to qemu_domain.c and rename it to qemuDomainObjFromDomain. This will allow reusing it after splitting out checkpoint code from qemu_driver.c. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 31 +++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 303 +++++++++++++++++++---------------------- 3 files changed, 169 insertions(+), 166 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2aa2164953..e8e895d9aa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -119,6 +119,37 @@ VIR_ENUM_IMPL(qemuDomainNamespace, "mount", ); + +/** + * qemuDomainObjFromDomain: + * @domain: Domain pointer that has to be looked up + * + * This function looks up @domain and returns the appropriate virDomainObjPtr + * that has to be released by calling virDomainObjEndAPI(). + * + * Returns the domain object with incremented reference counter which is locked + * on success, NULL otherwise. + */ +virDomainObjPtr +qemuDomainObjFromDomain(virDomainPtr domain) +{ + virDomainObjPtr vm; + virQEMUDriverPtr driver = domain->conn->privateData; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + vm = virDomainObjListFindByUUID(driver->domains, domain->uuid); + if (!vm) { + virUUIDFormat(domain->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s' (%s)"), + uuidstr, domain->name); + return NULL; + } + + return vm; +} + + struct _qemuDomainLogContext { virObject parent; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 6c490bd9d8..c740bb955a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -593,6 +593,7 @@ struct _qemuDomainXmlNsDef { char **capsdel; }; +virDomainObjPtr qemuDomainObjFromDomain(virDomainPtr domain); qemuDomainSaveCookiePtr qemuDomainSaveCookieNew(virDomainObjPtr vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0c65414a1a..b2d42affe3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -161,42 +161,13 @@ static int qemuARPGetInterfaces(virDomainObjPtr vm, static virQEMUDriverPtr qemu_driver; -/** - * qemuDomObjFromDomain: - * @domain: Domain pointer that has to be looked up - * - * This function looks up @domain and returns the appropriate virDomainObjPtr - * that has to be released by calling virDomainObjEndAPI(). - * - * Returns the domain object with incremented reference counter which is locked - * on success, NULL otherwise. - */ -static virDomainObjPtr -qemuDomObjFromDomain(virDomainPtr domain) -{ - virDomainObjPtr vm; - virQEMUDriverPtr driver = domain->conn->privateData; - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - vm = virDomainObjListFindByUUID(driver->domains, domain->uuid); - if (!vm) { - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s' (%s)"), - uuidstr, domain->name); - return NULL; - } - - return vm; -} - /* Looks up the domain object from snapshot and unlocks the * driver. The returned domain object is locked and ref'd and the * caller must call virDomainObjEndAPI() on it. */ static virDomainObjPtr qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot) { - return qemuDomObjFromDomain(snapshot->domain); + return qemuDomainObjFromDomain(snapshot->domain); } @@ -230,7 +201,7 @@ qemuSnapObjFromSnapshot(virDomainObjPtr vm, static virDomainObjPtr qemuDomObjFromCheckpoint(virDomainCheckpointPtr checkpoint) { - return qemuDomObjFromDomain(checkpoint->domain); + return qemuDomainObjFromDomain(checkpoint->domain); } @@ -1742,7 +1713,7 @@ static int qemuDomainIsActive(virDomainPtr dom) virDomainObjPtr obj; int ret = -1; - if (!(obj = qemuDomObjFromDomain(dom))) + if (!(obj = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainIsActiveEnsureACL(dom->conn, obj->def) < 0) @@ -1760,7 +1731,7 @@ static int qemuDomainIsPersistent(virDomainPtr dom) virDomainObjPtr obj; int ret = -1; - if (!(obj = qemuDomObjFromDomain(dom))) + if (!(obj = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainIsPersistentEnsureACL(dom->conn, obj->def) < 0) @@ -1778,7 +1749,7 @@ static int qemuDomainIsUpdated(virDomainPtr dom) virDomainObjPtr obj; int ret = -1; - if (!(obj = qemuDomObjFromDomain(dom))) + if (!(obj = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainIsUpdatedEnsureACL(dom->conn, obj->def) < 0) @@ -1958,7 +1929,7 @@ static int qemuDomainSuspend(virDomainPtr dom) int state; virQEMUDriverConfigPtr cfg = NULL; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainSuspendEnsureACL(dom->conn, vm->def) < 0) @@ -2013,7 +1984,7 @@ static int qemuDomainResume(virDomainPtr dom) int reason; virQEMUDriverConfigPtr cfg = NULL; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); @@ -2076,7 +2047,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (vm->def->onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_RESTART || @@ -2180,7 +2151,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN | VIR_DOMAIN_REBOOT_GUEST_AGENT, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (vm->def->onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY || @@ -2268,7 +2239,7 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags) virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainResetEnsureACL(dom->conn, vm->def) < 0) @@ -2331,7 +2302,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_DESTROY_GRACEFUL, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -2396,7 +2367,7 @@ static char *qemuDomainGetOSType(virDomainPtr dom) { virDomainObjPtr vm; char *type = NULL; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetOSTypeEnsureACL(dom->conn, vm->def) < 0) @@ -2416,7 +2387,7 @@ qemuDomainGetMaxMemory(virDomainPtr dom) virDomainObjPtr vm; unsigned long long ret = 0; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetMaxMemoryEnsureACL(dom->conn, vm->def) < 0) @@ -2444,7 +2415,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_MEM_MAXIMUM, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; cfg = virQEMUDriverGetConfig(driver); @@ -2571,7 +2542,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; cfg = virQEMUDriverGetConfig(driver); @@ -2642,7 +2613,7 @@ static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainInjectNMIEnsureACL(domain->conn, vm->def) < 0) @@ -2702,7 +2673,7 @@ static int qemuDomainSendKey(virDomainPtr domain, } } - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; priv = vm->privateData; @@ -2738,7 +2709,7 @@ qemuDomainGetInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetInfoEnsureACL(dom->conn, vm->def) < 0) @@ -2794,7 +2765,7 @@ qemuDomainGetState(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetStateEnsureACL(dom->conn, vm->def) < 0) @@ -2819,7 +2790,7 @@ qemuDomainGetControlInfo(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetControlInfoEnsureACL(dom->conn, vm->def) < 0) @@ -3629,7 +3600,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, "save", false)) < 0) goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainSaveFlagsEnsureACL(dom->conn, vm->def) < 0) @@ -3684,7 +3655,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainManagedSaveEnsureACL(dom->conn, vm->def) < 0) @@ -3755,7 +3726,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainHasManagedSaveImageEnsureACL(dom->conn, vm->def) < 0) @@ -3778,7 +3749,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainManagedSaveRemoveEnsureACL(dom->conn, vm->def) < 0) @@ -4021,7 +3992,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, VIR_DUMP_BYPASS_CACHE | VIR_DUMP_RESET | VIR_DUMP_MEMORY_ONLY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainCoreDumpWithFormatEnsureACL(dom->conn, vm->def) < 0) @@ -4135,7 +4106,7 @@ qemuDomainScreenshot(virDomainPtr dom, virCheckFlags(0, NULL); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -5198,7 +5169,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, VIR_DOMAIN_VCPU_GUEST | VIR_DOMAIN_VCPU_HOTPLUGGABLE, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0) @@ -5345,7 +5316,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, cfg = virQEMUDriverGetConfig(driver); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainPinVcpuFlagsEnsureACL(dom->conn, vm->def, flags) < 0) @@ -5425,7 +5396,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetVcpuPinInfoEnsureACL(dom->conn, vm->def) < 0) @@ -5470,7 +5441,7 @@ qemuDomainPinEmulator(virDomainPtr dom, cfg = virQEMUDriverGetConfig(driver); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainPinEmulatorEnsureACL(dom->conn, vm->def, flags) < 0) @@ -5574,7 +5545,7 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetEmulatorPinInfoEnsureACL(dom->conn, vm->def) < 0) @@ -5623,7 +5594,7 @@ qemuDomainGetVcpus(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetVcpusEnsureACL(dom->conn, vm->def) < 0) @@ -5660,7 +5631,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_GUEST, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0) @@ -5895,7 +5866,7 @@ qemuDomainGetIOThreadInfo(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetIOThreadInfoEnsureACL(dom->conn, vm->def) < 0) @@ -5942,7 +5913,7 @@ qemuDomainPinIOThread(virDomainPtr dom, cfg = virQEMUDriverGetConfig(driver); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -6519,7 +6490,7 @@ qemuDomainAddIOThread(virDomainPtr dom, return -1; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainAddIOThreadEnsureACL(dom->conn, vm->def, flags) < 0) @@ -6554,7 +6525,7 @@ qemuDomainDelIOThread(virDomainPtr dom, return -1; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainDelIOThreadEnsureACL(dom->conn, vm->def, flags) < 0) @@ -6603,7 +6574,7 @@ qemuDomainSetIOThreadParams(virDomainPtr dom, iothread.iothread_id = iothread_id; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainSetIOThreadParamsEnsureACL(dom->conn, vm->def, flags) < 0) @@ -6629,7 +6600,7 @@ static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secl memset(seclabel, 0, sizeof(*seclabel)); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainGetSecurityLabelEnsureACL(dom->conn, vm->def) < 0) @@ -6670,7 +6641,7 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom, size_t i; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainGetSecurityLabelListEnsureACL(dom->conn, vm->def) < 0) @@ -7360,7 +7331,7 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return ret; priv = vm->privateData; @@ -7402,7 +7373,7 @@ qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, char *path = NULL; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainManagedSaveDefineXMLEnsureACL(conn, vm->def) < 0) @@ -7518,7 +7489,7 @@ static char virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU, NULL); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0) @@ -7758,7 +7729,7 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) virNWFilterReadLockFilterUpdates(); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0) @@ -7911,7 +7882,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, return -1; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); @@ -8936,7 +8907,7 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom, virNWFilterReadLockFilterUpdates(); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) @@ -8995,7 +8966,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -9266,7 +9237,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, virDomainObjPtr vm = NULL; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) @@ -9301,7 +9272,7 @@ qemuDomainDetachDeviceAlias(virDomainPtr dom, virDomainObjPtr vm = NULL; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainDetachDeviceAliasEnsureACL(dom->conn, vm->def, flags) < 0) @@ -9339,7 +9310,7 @@ static int qemuDomainGetAutostart(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetAutostartEnsureACL(dom->conn, vm->def) < 0) @@ -9362,7 +9333,7 @@ static int qemuDomainSetAutostart(virDomainPtr dom, int ret = -1; virQEMUDriverConfigPtr cfg = NULL; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); @@ -9438,7 +9409,7 @@ static char *qemuDomainGetSchedulerType(virDomainPtr dom, qemuDomainObjPrivatePtr priv; virQEMUDriverPtr driver = dom->conn->privateData; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -9693,7 +9664,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, NULL) < 0) return -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -9901,7 +9872,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, * that can't parse it. */ flags &= ~VIR_TYPED_PARAM_STRING_OKAY; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -10003,7 +9974,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, return -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -10137,7 +10108,7 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, VIR_DOMAIN_AFFECT_CONFIG | VIR_TYPED_PARAM_STRING_OKAY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -10291,7 +10262,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, NULL) < 0) return -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -10410,7 +10381,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, VIR_DOMAIN_AFFECT_CONFIG | VIR_TYPED_PARAM_STRING_OKAY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -10528,7 +10499,7 @@ qemuDomainSetPerfEvents(virDomainPtr dom, NULL) < 0) return -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); @@ -10607,7 +10578,7 @@ qemuDomainGetPerfEvents(virDomainPtr dom, VIR_DOMAIN_AFFECT_CONFIG | VIR_TYPED_PARAM_STRING_OKAY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetPerfEventsEnsureACL(dom->conn, vm->def) < 0) @@ -10802,7 +10773,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, NULL) < 0) return -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -11223,7 +11194,7 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, VIR_DOMAIN_AFFECT_CONFIG | VIR_TYPED_PARAM_STRING_OKAY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -11351,7 +11322,7 @@ qemuDomainBlockResize(virDomainPtr dom, size *= 1024; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -11553,7 +11524,7 @@ qemuDomainBlockStats(virDomainPtr dom, int ret = -1; virDomainObjPtr vm; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0) @@ -11611,7 +11582,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, /* We don't return strings, and thus trivially support this flag. */ flags &= ~VIR_TYPED_PARAM_STRING_OKAY; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainBlockStatsFlagsEnsureACL(dom->conn, vm->def) < 0) @@ -11687,7 +11658,7 @@ qemuDomainInterfaceStats(virDomainPtr dom, virDomainNetDefPtr net = NULL; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainInterfaceStatsEnsureACL(dom->conn, vm->def) < 0) @@ -11754,7 +11725,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, NULL) < 0) return -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); @@ -11945,7 +11916,7 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom, VIR_DOMAIN_AFFECT_CONFIG | VIR_TYPED_PARAM_STRING_OKAY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainGetInterfaceParametersEnsureACL(dom->conn, vm->def) < 0) @@ -12088,7 +12059,7 @@ qemuDomainMemoryStats(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0) @@ -12122,7 +12093,7 @@ qemuDomainBlockPeek(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainBlockPeekEnsureACL(dom->conn, vm->def) < 0) @@ -12190,7 +12161,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; cfg = virQEMUDriverGetConfig(driver); @@ -12479,7 +12450,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); @@ -12817,7 +12788,7 @@ qemuDomainMigratePerform(virDomainPtr dom, QEMU_MIGRATION_SOURCE))) goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMigratePerformEnsureACL(dom->conn, vm->def) < 0) @@ -12904,7 +12875,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return NULL; if (virDomainMigrateBegin3EnsureACL(domain->conn, vm->def) < 0) { @@ -12950,7 +12921,7 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, if (nmigrate_disks < 0) goto cleanup; - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) { @@ -13246,7 +13217,7 @@ qemuDomainMigratePerform3(virDomainPtr dom, QEMU_MIGRATION_SOURCE))) goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMigratePerform3EnsureACL(dom->conn, vm->def) < 0) @@ -13332,7 +13303,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, QEMU_MIGRATION_SOURCE))) goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0) @@ -13453,7 +13424,7 @@ qemuDomainMigrateConfirm3(virDomainPtr domain, virCheckFlags(QEMU_MIGRATION_FLAGS, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainMigrateConfirm3EnsureACL(domain->conn, vm->def) < 0) { @@ -13481,7 +13452,7 @@ qemuDomainMigrateConfirm3Params(virDomainPtr domain, if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) return -1; - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainMigrateConfirm3ParamsEnsureACL(domain->conn, vm->def) < 0) { @@ -14055,7 +14026,7 @@ qemuDomainGetJobInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetJobInfoEnsureACL(dom->conn, vm->def) < 0) @@ -14095,7 +14066,7 @@ qemuDomainGetJobStats(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetJobStatsEnsureACL(dom->conn, vm->def) < 0) @@ -14132,7 +14103,7 @@ static int qemuDomainAbortJob(virDomainPtr dom) qemuDomainObjPrivatePtr priv; int reason; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainAbortJobEnsureACL(dom->conn, vm->def) < 0) @@ -14203,7 +14174,7 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMigrateSetMaxDowntimeEnsureACL(dom->conn, vm->def) < 0) @@ -14245,7 +14216,7 @@ qemuDomainMigrateGetMaxDowntime(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainMigrateGetMaxDowntimeEnsureACL(dom->conn, vm->def) < 0) @@ -14298,7 +14269,7 @@ qemuDomainMigrateGetCompressionCache(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMigrateGetCompressionCacheEnsureACL(dom->conn, vm->def) < 0) @@ -14346,7 +14317,7 @@ qemuDomainMigrateSetCompressionCache(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMigrateSetCompressionCacheEnsureACL(dom->conn, vm->def) < 0) @@ -14398,7 +14369,7 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -14530,7 +14501,7 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -14564,7 +14535,7 @@ qemuDomainMigrateStartPostCopy(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainMigrateStartPostCopyEnsureACL(dom->conn, vm->def) < 0) @@ -15899,7 +15870,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (redefine) parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; priv = vm->privateData; @@ -16171,7 +16142,7 @@ qemuDomainSnapshotListNames(virDomainPtr domain, VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainSnapshotListNamesEnsureACL(domain->conn, vm->def) < 0) @@ -16197,7 +16168,7 @@ qemuDomainSnapshotNum(virDomainPtr domain, VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainSnapshotNumEnsureACL(domain->conn, vm->def) < 0) @@ -16223,7 +16194,7 @@ qemuDomainListAllSnapshots(virDomainPtr domain, VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainListAllSnapshotsEnsureACL(domain->conn, vm->def) < 0) @@ -16340,7 +16311,7 @@ qemuDomainSnapshotLookupByName(virDomainPtr domain, virCheckFlags(0, NULL); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return NULL; if (virDomainSnapshotLookupByNameEnsureACL(domain->conn, vm->def) < 0) @@ -16366,7 +16337,7 @@ qemuDomainHasCurrentSnapshot(virDomainPtr domain, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainHasCurrentSnapshotEnsureACL(domain->conn, vm->def) < 0) @@ -16424,7 +16395,7 @@ qemuDomainSnapshotCurrent(virDomainPtr domain, virCheckFlags(0, NULL); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return NULL; if (virDomainSnapshotCurrentEnsureACL(domain->conn, vm->def) < 0) @@ -17290,7 +17261,7 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, update_current = false; } - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { @@ -17428,7 +17399,7 @@ qemuDomainListAllCheckpoints(virDomainPtr domain, VIR_DOMAIN_CHECKPOINT_LIST_TOPOLOGICAL | VIR_DOMAIN_CHECKPOINT_FILTERS_ALL, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; if (virDomainListAllCheckpointsEnsureACL(domain->conn, vm->def) < 0) @@ -17485,7 +17456,7 @@ qemuDomainCheckpointLookupByName(virDomainPtr domain, virCheckFlags(0, NULL); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return NULL; if (virDomainCheckpointLookupByNameEnsureACL(domain->conn, vm->def) < 0) @@ -17690,7 +17661,7 @@ static int qemuDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, virCheckFlags(VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; if (virDomainQemuMonitorCommandEnsureACL(domain->conn, vm->def) < 0) @@ -17737,7 +17708,7 @@ qemuDomainOpenConsole(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_CONSOLE_SAFE | VIR_DOMAIN_CONSOLE_FORCE, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0) @@ -17814,7 +17785,7 @@ qemuDomainOpenChannel(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_CHANNEL_FORCE, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainOpenChannelEnsureACL(dom->conn, vm->def) < 0) @@ -18082,7 +18053,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC | VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainBlockJobAbortEnsureACL(dom->conn, vm->def) < 0) @@ -18244,7 +18215,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainGetBlockJobInfoEnsureACL(dom->conn, vm->def) < 0) @@ -18316,7 +18287,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, speed <<= 20; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainBlockJobSetSpeedEnsureACL(dom->conn, vm->def) < 0) @@ -18774,7 +18745,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, VIR_DOMAIN_BLOCK_REBASE_COPY_DEV | VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0) @@ -18864,7 +18835,7 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, NULL) < 0) return -1; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainBlockCopyEnsureACL(dom->conn, vm->def) < 0) @@ -18930,7 +18901,7 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, virDomainObjPtr vm; virCheckFlags(VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainBlockPullEnsureACL(dom->conn, vm->def) < 0) { @@ -18988,7 +18959,7 @@ qemuDomainBlockCommit(virDomainPtr dom, VIR_DOMAIN_BLOCK_COMMIT_RELATIVE | VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -19211,7 +19182,7 @@ qemuDomainOpenGraphics(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainOpenGraphicsEnsureACL(dom->conn, vm->def) < 0) @@ -19283,7 +19254,7 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainOpenGraphicsFdEnsureACL(dom->conn, vm->def) < 0) @@ -19500,7 +19471,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, memset(&info, 0, sizeof(info)); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; if (virDomainSetBlockIoTuneEnsureACL(dom->conn, vm->def, flags) < 0) @@ -19797,7 +19768,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, /* We don't return strings, and thus trivially support this flag. */ flags &= ~VIR_TYPED_PARAM_STRING_OKAY; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; priv = vm->privateData; @@ -19944,7 +19915,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -20024,7 +19995,7 @@ qemuDomainSetMetadata(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); @@ -20066,7 +20037,7 @@ qemuDomainGetMetadata(virDomainPtr dom, virDomainObjPtr vm; char *ret = NULL; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return NULL; if (virDomainGetMetadataEnsureACL(dom->conn, vm->def) < 0) @@ -20095,7 +20066,7 @@ qemuDomainGetCPUStats(virDomainPtr domain, virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) return -1; priv = vm->privateData; @@ -20179,7 +20150,7 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, return -1; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) @@ -20262,7 +20233,7 @@ qemuDomainPMWakeup(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainPMWakeupEnsureACL(dom->conn, vm->def) < 0) @@ -20323,7 +20294,7 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, virCheckFlags(0, NULL); - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; if (virDomainQemuAgentCommandEnsureACL(domain->conn, vm->def) < 0) @@ -20423,7 +20394,7 @@ qemuDomainFSTrim(virDomainPtr dom, return -1; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainFSTrimEnsureACL(dom->conn, vm->def) < 0) @@ -20603,7 +20574,7 @@ qemuDomainGetHostname(virDomainPtr dom, virCheckFlags(0, NULL); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return NULL; if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0) @@ -20645,7 +20616,7 @@ qemuDomainGetTime(virDomainPtr dom, virCheckFlags(0, ret); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return ret; if (virDomainGetTimeEnsureACL(dom->conn, vm->def) < 0) @@ -20694,7 +20665,7 @@ qemuDomainSetTime(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return ret; if (virDomainSetTimeEnsureACL(dom->conn, vm->def) < 0) @@ -20769,7 +20740,7 @@ qemuDomainFSFreeze(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainFSFreezeEnsureACL(dom->conn, vm->def) < 0) @@ -20810,7 +20781,7 @@ qemuDomainFSThaw(virDomainPtr dom, return ret; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainFSThawEnsureACL(dom->conn, vm->def) < 0) @@ -22036,7 +22007,7 @@ qemuDomainGetFSInfo(virDomainPtr dom, virCheckFlags(0, ret); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return ret; if (virDomainGetFSInfoEnsureACL(dom->conn, vm->def) < 0) @@ -22078,7 +22049,7 @@ qemuDomainInterfaceAddresses(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainInterfaceAddressesEnsureACL(dom->conn, vm->def) < 0) @@ -22296,7 +22267,7 @@ qemuDomainSetUserPassword(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_PASSWORD_ENCRYPTED, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) return ret; if (virDomainSetUserPasswordEnsureACL(dom->conn, vm->def) < 0) @@ -22456,7 +22427,7 @@ static int qemuDomainRename(virDomainPtr dom, virCheckFlags(0, ret); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainRenameEnsureACL(dom->conn, vm->def) < 0) @@ -22596,7 +22567,7 @@ qemuDomainGetGuestVcpus(virDomainPtr dom, virCheckFlags(0, ret); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetGuestVcpusEnsureACL(dom->conn, vm->def) < 0) @@ -22655,7 +22626,7 @@ qemuDomainSetGuestVcpus(virDomainPtr dom, if (virBitmapParse(cpumap, &map, QEMU_GUEST_VCPU_MAX_ID) < 0) goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainSetGuestVcpusEnsureACL(dom->conn, vm->def) < 0) @@ -22748,7 +22719,7 @@ qemuDomainSetVcpu(virDomainPtr dom, goto cleanup; } - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainSetVcpuEnsureACL(dom->conn, vm->def, flags) < 0) @@ -22806,7 +22777,7 @@ qemuDomainSetBlockThreshold(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -22902,7 +22873,7 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, if (!virDomainDefLifecycleActionAllowed(type, action)) goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; priv = vm->privateData; @@ -23076,7 +23047,7 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, virDomainObjPtr vm; int ret = -1; - if (!(vm = qemuDomObjFromDomain(domain))) + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0) @@ -23129,7 +23100,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, virCheckFlags(0, -1); qemuDomainGetGuestInfoCheckSupport(&supportedTypes); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0) -- 2.21.0

On Wed, Sep 25, 2019 at 02:54:37PM +0200, Peter Krempa wrote:
Move it to qemu_domain.c and rename it to qemuDomainObjFromDomain. This will allow reusing it after splitting out checkpoint code from qemu_driver.c.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 31 +++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 303 +++++++++++++++++++---------------------- 3 files changed, 169 insertions(+), 166 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

'vm' is passed in which contains the definition which contains the UUID so we don't need another parameter for this. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 7 +++---- src/conf/checkpoint_conf.h | 3 +-- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 5c998267aa..a83863adcc 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -532,8 +532,7 @@ virDomainCheckpointDefFormat(virDomainCheckpointDefPtr def, int -virDomainCheckpointRedefinePrep(virDomainPtr domain, - virDomainObjPtr vm, +virDomainCheckpointRedefinePrep(virDomainObjPtr vm, virDomainCheckpointDefPtr *defptr, virDomainMomentObjPtr *chk, virDomainXMLOptionPtr xmlopt, @@ -544,13 +543,13 @@ virDomainCheckpointRedefinePrep(virDomainPtr domain, virDomainMomentObjPtr other = NULL; virDomainCheckpointDefPtr otherdef = NULL; - virUUIDFormat(domain->uuid, uuidstr); + virUUIDFormat(vm->def->uuid, uuidstr); if (virDomainCheckpointCheckCycles(vm->checkpoints, def, vm->def->name) < 0) return -1; if (!def->parent.dom || - memcmp(def->parent.dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) { + memcmp(def->parent.dom->uuid, vm->def->uuid, VIR_UUID_BUFLEN)) { virReportError(VIR_ERR_INVALID_ARG, _("definition for checkpoint %s must use uuid %s"), def->parent.name, uuidstr); diff --git a/src/conf/checkpoint_conf.h b/src/conf/checkpoint_conf.h index 47ff69eb4d..2be041ff56 100644 --- a/src/conf/checkpoint_conf.h +++ b/src/conf/checkpoint_conf.h @@ -90,8 +90,7 @@ virDomainCheckpointDefFormat(virDomainCheckpointDefPtr def, int virDomainCheckpointAlignDisks(virDomainCheckpointDefPtr checkpoint); -int virDomainCheckpointRedefinePrep(virDomainPtr domain, - virDomainObjPtr vm, +int virDomainCheckpointRedefinePrep(virDomainObjPtr vm, virDomainCheckpointDefPtr *def, virDomainMomentObjPtr *checkpoint, virDomainXMLOptionPtr xmlopt, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b2d42affe3..e16069b65e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17301,7 +17301,7 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, goto cleanup; if (redefine) { - if (virDomainCheckpointRedefinePrep(domain, vm, &def, &chk, + if (virDomainCheckpointRedefinePrep(vm, &def, &chk, driver->xmlopt, &update_current) < 0) goto endjob; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 9df7840390..dafd8c8daa 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -9073,7 +9073,7 @@ testDomainCheckpointCreateXML(virDomainPtr domain, goto cleanup; if (redefine) { - if (virDomainCheckpointRedefinePrep(domain, vm, &def, &chk, + if (virDomainCheckpointRedefinePrep(vm, &def, &chk, privconn->xmlopt, &update_current) < 0) goto cleanup; -- 2.21.0

On Wed, Sep 25, 2019 at 02:54:38PM +0200, Peter Krempa wrote:
'vm' is passed in which contains the definition which contains the UUID so we don't need another parameter for this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 7 +++---- src/conf/checkpoint_conf.h | 3 +-- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

'vm' is passed in which contains the definition which contains the UUID so we don't need another parameter for this. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/snapshot_conf.c | 5 ++--- src/conf/snapshot_conf.h | 3 +-- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 61c807a71f..96ad8ca953 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -986,8 +986,7 @@ virDomainSnapshotIsExternal(virDomainMomentObjPtr snap) } int -virDomainSnapshotRedefinePrep(virDomainPtr domain, - virDomainObjPtr vm, +virDomainSnapshotRedefinePrep(virDomainObjPtr vm, virDomainSnapshotDefPtr *defptr, virDomainMomentObjPtr *snap, virDomainXMLOptionPtr xmlopt, @@ -1006,7 +1005,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, if (other) otherdef = virDomainSnapshotObjGetDef(other); check_if_stolen = other && otherdef->parent.dom; - if (virDomainSnapshotRedefineValidate(def, domain->uuid, other, xmlopt, + if (virDomainSnapshotRedefineValidate(def, vm->def->uuid, other, xmlopt, flags) < 0) { /* revert any stealing of the snapshot domain definition */ if (check_if_stolen && def->parent.dom && !otherdef->parent.dom) diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 216726fc16..17d614a7e1 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -129,8 +129,7 @@ int virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapshot, bool virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def); bool virDomainSnapshotIsExternal(virDomainMomentObjPtr snap); -int virDomainSnapshotRedefinePrep(virDomainPtr domain, - virDomainObjPtr vm, +int virDomainSnapshotRedefinePrep(virDomainObjPtr vm, virDomainSnapshotDefPtr *def, virDomainMomentObjPtr *snap, virDomainXMLOptionPtr xmlopt, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e16069b65e..ca25814bd1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15979,7 +15979,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE); if (redefine) { - if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, + if (virDomainSnapshotRedefinePrep(vm, &def, &snap, driver->xmlopt, &update_current, flags) < 0) goto endjob; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index dafd8c8daa..19b8158ed6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -8612,7 +8612,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; if (redefine) { - if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, + if (virDomainSnapshotRedefinePrep(vm, &def, &snap, privconn->xmlopt, &update_current, flags) < 0) goto cleanup; -- 2.21.0

On Wed, Sep 25, 2019 at 02:54:39PM +0200, Peter Krempa wrote:
'vm' is passed in which contains the definition which contains the UUID so we don't need another parameter for this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/snapshot_conf.c | 5 ++--- src/conf/snapshot_conf.h | 3 +-- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

The code that gets the job to refresh disk sizes was not merged yet so remove this artifact. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ca25814bd1..7da6dbce66 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17535,9 +17535,6 @@ qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint, xml = virDomainCheckpointDefFormat(chkdef, driver->caps, driver->xmlopt, format_flags); - if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE) - qemuDomainObjEndJob(driver, vm); - cleanup: virDomainObjEndAPI(&vm); return xml; -- 2.21.0

On 9/25/19 7:54 AM, Peter Krempa wrote:
The code that gets the job to refresh disk sizes was not merged yet so remove this artifact.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 3 --- 1 file changed, 3 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ca25814bd1..7da6dbce66 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17535,9 +17535,6 @@ qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint, xml = virDomainCheckpointDefFormat(chkdef, driver->caps, driver->xmlopt, format_flags);
- if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE) - qemuDomainObjEndJob(driver, vm); - cleanup: virDomainObjEndAPI(&vm); return xml;
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Move all extensive functions to a new file so that we don't just pile everything in the common files. This obviously isn't possible with straight code movement as we still need stubs in qemu_driver.c Additionally some functions e.g. for looking up a checkpoint by name were so short that moving the impl didn't make sense. Note that in the move the new file also doesn't use virQEMUMomentReparent but rather an stripped down copy. As I plan to split out snapshot code into a separate file the unification doesn't make sense any more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_checkpoint.c | 483 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_checkpoint.h | 50 ++++ src/qemu/qemu_driver.c | 382 +---------------------------- 4 files changed, 540 insertions(+), 377 deletions(-) create mode 100644 src/qemu/qemu_checkpoint.c create mode 100644 src/qemu/qemu_checkpoint.h diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 8040bf9366..e0e13fb1c3 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -68,6 +68,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_vhost_user.h \ qemu/qemu_vhost_user_gpu.c \ qemu/qemu_vhost_user_gpu.h \ + qemu/qemu_checkpoint.c \ + qemu/qemu_checkpoint.h \ $(NULL) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c new file mode 100644 index 0000000000..9e9af76871 --- /dev/null +++ b/src/qemu/qemu_checkpoint.c @@ -0,0 +1,483 @@ +/* + * qemu_checkpoint.c: checkpoint related implementation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include <sys/types.h> + +#include "qemu_checkpoint.h" +#include "qemu_capabilities.h" +#include "qemu_monitor.h" +#include "qemu_monitor_json.h" +#include "qemu_domain.h" + +#include "virerror.h" +#include "virlog.h" +#include "datatypes.h" +#include "viralloc.h" +#include "domain_conf.h" +#include "libvirt_internal.h" +#include "virxml.h" +#include "virstring.h" +#include "virdomaincheckpointobjlist.h" +#include "virdomainsnapshotobjlist.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_checkpoint"); + +/* Looks up the domain object from checkpoint and unlocks the + * driver. The returned domain object is locked and ref'd and the + * caller must call virDomainObjEndAPI() on it. */ +virDomainObjPtr +qemuDomObjFromCheckpoint(virDomainCheckpointPtr checkpoint) +{ + return qemuDomainObjFromDomain(checkpoint->domain); +} + + +/* Looks up checkpoint object from VM and name */ +virDomainMomentObjPtr +qemuCheckpointObjFromName(virDomainObjPtr vm, + const char *name) +{ + virDomainMomentObjPtr chk = NULL; + chk = virDomainCheckpointFindByName(vm->checkpoints, name); + if (!chk) + virReportError(VIR_ERR_NO_DOMAIN_CHECKPOINT, + _("no domain checkpoint with matching name '%s'"), + name); + + return chk; +} + + +/* Looks up checkpoint object from VM and checkpointPtr */ +virDomainMomentObjPtr +qemuCheckpointObjFromCheckpoint(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint) +{ + return qemuCheckpointObjFromName(vm, checkpoint->name); +} + + +/* Called inside job lock */ +static int +qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps, + virDomainObjPtr vm, + virDomainCheckpointDefPtr def) +{ + int ret = -1; + size_t i; + char *xml = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + + /* Easiest way to clone inactive portion of vm->def is via + * conversion in and back out of xml. */ + if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, + vm->def, priv->origCPU, + true, true)) || + !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, + priv->qemuCaps, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) + goto cleanup; + + if (virDomainCheckpointAlignDisks(def) < 0) + goto cleanup; + + for (i = 0; i < def->ndisks; i++) { + virDomainCheckpointDiskDefPtr disk = &def->disks[i]; + + if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) + continue; + + if (vm->def->disks[i]->src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("checkpoint for disk %s unsupported " + "for storage type %s"), + disk->name, + virStorageFileFormatTypeToString( + vm->def->disks[i]->src->format)); + goto cleanup; + } + } + + ret = 0; + + cleanup: + VIR_FREE(xml); + return ret; +} + +static int +qemuDomainCheckpointAddActions(virDomainObjPtr vm, + virJSONValuePtr actions, + virDomainMomentObjPtr old_current, + virDomainCheckpointDefPtr def) +{ + size_t i, j; + virDomainCheckpointDefPtr olddef; + virDomainMomentObjPtr parent; + bool search_parents; + + for (i = 0; i < def->ndisks; i++) { + virDomainCheckpointDiskDef *disk = &def->disks[i]; + const char *node; + + if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) + continue; + node = qemuDomainDiskNodeFormatLookup(vm, disk->name); + if (qemuMonitorJSONTransactionAdd(actions, + "block-dirty-bitmap-add", + "s:node", node, + "s:name", disk->bitmap, + "b:persistent", true, + NULL) < 0) + return -1; + + /* We only want one active bitmap for a disk along the + * checkpoint chain, then later differential backups will + * merge the bitmaps (only one active) between the bounding + * checkpoint and the leaf checkpoint. If the same disks are + * involved in each checkpoint, this search terminates in one + * iteration; but it is also possible to have to search + * further than the immediate parent to find another + * checkpoint with a bitmap on the same disk. */ + search_parents = true; + for (parent = old_current; search_parents && parent; + parent = virDomainCheckpointFindByName(vm->checkpoints, + olddef->parent.parent_name)) { + olddef = virDomainCheckpointObjGetDef(parent); + for (j = 0; j < olddef->ndisks; j++) { + virDomainCheckpointDiskDef *disk2; + + disk2 = &olddef->disks[j]; + if (STRNEQ(disk->name, disk2->name) || + disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) + continue; + if (qemuMonitorJSONTransactionAdd(actions, + "block-dirty-bitmap-disable", + "s:node", node, + "s:name", disk2->bitmap, + NULL) < 0) + return -1; + search_parents = false; + break; + } + } + } + return 0; +} + + +virDomainCheckpointPtr +qemuCheckpointCreateXML(virDomainPtr domain, + virDomainObjPtr vm, + const char *xmlDesc, + unsigned int flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virDomainMomentObjPtr chk = NULL; + virDomainCheckpointPtr checkpoint = NULL; + bool update_current = true; + bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE; + unsigned int parse_flags = 0; + virDomainMomentObjPtr other = NULL; + virQEMUDriverConfigPtr cfg = NULL; + virCapsPtr caps = NULL; + virJSONValuePtr actions = NULL; + int ret; + VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL; + + virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE, NULL); + /* TODO: VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE */ + + if (redefine) { + parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE; + update_current = false; + } + + if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot create checkpoint while snapshot exists")); + goto cleanup; + } + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu binary lacks persistent bitmaps support")); + goto cleanup; + } + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot create checkpoint for inactive domain")); + goto cleanup; + } + + if (!(def = virDomainCheckpointDefParseString(xmlDesc, caps, driver->xmlopt, + priv->qemuCaps, parse_flags))) + goto cleanup; + /* Unlike snapshots, the RNG schema already ensured a sane filename. */ + + /* We are going to modify the domain below. */ + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (redefine) { + if (virDomainCheckpointRedefinePrep(vm, &def, &chk, + driver->xmlopt, + &update_current) < 0) + goto endjob; + } else if (qemuDomainCheckpointPrepare(driver, caps, vm, def) < 0) { + goto endjob; + } + + if (!chk) { + if (!(chk = virDomainCheckpointAssignDef(vm->checkpoints, def))) + goto endjob; + + def = NULL; + } + + other = virDomainCheckpointGetCurrent(vm->checkpoints); + if (other) { + if (!redefine && + VIR_STRDUP(chk->def->parent_name, other->def->name) < 0) + goto endjob; + if (update_current) { + virDomainCheckpointSetCurrent(vm->checkpoints, NULL); + if (qemuDomainCheckpointWriteMetadata(vm, other, + driver->caps, driver->xmlopt, + cfg->checkpointDir) < 0) + goto endjob; + } + } + + /* actually do the checkpoint */ + if (redefine) { + /* XXX Should we validate that the redefined checkpoint even + * makes sense, such as checking that qemu-img recognizes the + * checkpoint bitmap name in at least one of the domain's disks? */ + } else { + if (!(actions = virJSONValueNewArray())) + goto endjob; + if (qemuDomainCheckpointAddActions(vm, actions, other, + virDomainCheckpointObjGetDef(chk)) < 0) + goto endjob; + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorTransaction(priv->mon, &actions); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) + goto endjob; + } + + /* If we fail after this point, there's not a whole lot we can do; + * we've successfully created the checkpoint, so we have to go + * forward the best we can. + */ + checkpoint = virGetDomainCheckpoint(domain, chk->def->name); + + endjob: + if (checkpoint) { + if (update_current) + virDomainCheckpointSetCurrent(vm->checkpoints, chk); + if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { + /* if writing of metadata fails, error out rather than trying + * to silently carry on without completing the checkpoint */ + virObjectUnref(checkpoint); + checkpoint = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to save metadata for checkpoint %s"), + chk->def->name); + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + } else { + virDomainCheckpointLinkParent(vm->checkpoints, chk); + } + } else if (chk) { + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + } + + qemuDomainObjEndJob(driver, vm); + + cleanup: + virJSONValueFree(actions); + virDomainObjEndAPI(&vm); + virObjectUnref(caps); + virObjectUnref(cfg); + return checkpoint; +} + + +char * +qemuCheckpointGetXMLDesc(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint, + unsigned int flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + virDomainMomentObjPtr chk = NULL; + virDomainCheckpointDefPtr chkdef; + unsigned int format_flags; + + virCheckFlags(VIR_DOMAIN_CHECKPOINT_XML_SECURE | + VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN, NULL); + + if (!(chk = qemuCheckpointObjFromCheckpoint(vm, checkpoint))) + return NULL; + + chkdef = virDomainCheckpointObjGetDef(chk); + + format_flags = virDomainCheckpointFormatConvertXMLFlags(flags); + return virDomainCheckpointDefFormat(chkdef, driver->caps, driver->xmlopt, + format_flags); +} + + +struct virQEMUCheckpointReparent { + const char *dir; + virDomainMomentObjPtr parent; + virDomainObjPtr vm; + virCapsPtr caps; + virDomainXMLOptionPtr xmlopt; + int err; +}; + + +static int +qemuCheckpointReparentChildren(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainMomentObjPtr moment = payload; + struct virQEMUCheckpointReparent *rep = data; + + if (rep->err < 0) + return 0; + + VIR_FREE(moment->def->parent_name); + + if (rep->parent->def && + VIR_STRDUP(moment->def->parent_name, rep->parent->def->name) < 0) { + rep->err = -1; + return 0; + } + + rep->err = qemuDomainCheckpointWriteMetadata(rep->vm, moment, rep->caps, + rep->xmlopt, rep->dir); + return 0; +} + + +int +qemuCheckpointDelete(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint, + unsigned int flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + virDomainMomentObjPtr chk = NULL; + virQEMUMomentRemove rem; + struct virQEMUCheckpointReparent rep; + bool metadata_only = !!(flags & VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY); + + virCheckFlags(VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN | + VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY | + VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY, -1); + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + return -1; + + if (!metadata_only) { + /* Until qemu-img supports offline bitmap deletion, we are stuck + * with requiring a running guest */ + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot delete checkpoint for inactive domain")); + goto endjob; + } + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu binary lacks persistent bitmaps support")); + goto endjob; + } + } + + if (!(chk = qemuCheckpointObjFromCheckpoint(vm, checkpoint))) + goto endjob; + + if (flags & (VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN | + VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY)) { + rem.driver = driver; + rem.vm = vm; + rem.metadata_only = metadata_only; + rem.err = 0; + rem.current = virDomainCheckpointGetCurrent(vm->checkpoints); + rem.found = false; + rem.momentDiscard = qemuDomainCheckpointDiscard; + virDomainMomentForEachDescendant(chk, qemuDomainMomentDiscardAll, + &rem); + if (rem.err < 0) + goto endjob; + if (rem.found) { + virDomainCheckpointSetCurrent(vm->checkpoints, chk); + if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) { + if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to set checkpoint '%s' as current"), + chk->def->name); + virDomainCheckpointSetCurrent(vm->checkpoints, NULL); + goto endjob; + } + } + } + } else if (chk->nchildren) { + rep.dir = cfg->checkpointDir; + rep.parent = chk->parent; + rep.vm = vm; + rep.err = 0; + rep.caps = driver->caps; + rep.xmlopt = driver->xmlopt; + virDomainMomentForEachChild(chk, qemuCheckpointReparentChildren, + &rep); + if (rep.err < 0) + goto endjob; + virDomainMomentMoveChildren(chk, chk->parent); + } + + if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) { + virDomainMomentDropChildren(chk); + ret = 0; + } else { + ret = qemuDomainCheckpointDiscard(driver, vm, chk, true, metadata_only); + } + + endjob: + qemuDomainObjEndJob(driver, vm); + return ret; +} diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h new file mode 100644 index 0000000000..0d36d718d6 --- /dev/null +++ b/src/qemu/qemu_checkpoint.h @@ -0,0 +1,50 @@ +/* + * qemu_checkpoint.h: Implementation and handling of checkpoint + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "virconftypes.h" +#include "datatypes.h" + +virDomainObjPtr +qemuDomObjFromCheckpoint(virDomainCheckpointPtr checkpoint); + +virDomainMomentObjPtr +qemuCheckpointObjFromCheckpoint(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint); + +virDomainMomentObjPtr +qemuCheckpointObjFromName(virDomainObjPtr vm, + const char *name); + +virDomainCheckpointPtr +qemuCheckpointCreateXML(virDomainPtr domain, + virDomainObjPtr vm, + const char *xmlDesc, + unsigned int flags); + + +char * +qemuCheckpointGetXMLDesc(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint, + unsigned int flags); + +int +qemuCheckpointDelete(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint, + unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7da6dbce66..7163216f69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -53,6 +53,7 @@ #include "qemu_migration_params.h" #include "qemu_blockjob.h" #include "qemu_security.h" +#include "qemu_checkpoint.h" #include "virerror.h" #include "virlog.h" @@ -195,39 +196,6 @@ qemuSnapObjFromSnapshot(virDomainObjPtr vm, return qemuSnapObjFromName(vm, snapshot->name); } -/* Looks up the domain object from checkpoint and unlocks the - * driver. The returned domain object is locked and ref'd and the - * caller must call virDomainObjEndAPI() on it. */ -static virDomainObjPtr -qemuDomObjFromCheckpoint(virDomainCheckpointPtr checkpoint) -{ - return qemuDomainObjFromDomain(checkpoint->domain); -} - - -/* Looks up checkpoint object from VM and name */ -static virDomainMomentObjPtr -qemuCheckpointObjFromName(virDomainObjPtr vm, - const char *name) -{ - virDomainMomentObjPtr chk = NULL; - chk = virDomainCheckpointFindByName(vm->checkpoints, name); - if (!chk) - virReportError(VIR_ERR_NO_DOMAIN_CHECKPOINT, - _("no domain checkpoint with matching name '%s'"), - name); - - return chk; -} - - -/* Looks up checkpoint object from VM and checkpointPtr */ -static virDomainMomentObjPtr -qemuCheckpointObjFromCheckpoint(virDomainObjPtr vm, - virDomainCheckpointPtr checkpoint) -{ - return qemuCheckpointObjFromName(vm, checkpoint->name); -} static int qemuAutostartDomain(virDomainObjPtr vm, @@ -17123,266 +17091,24 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, } -/* Called inside job lock */ -static int -qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps, - virDomainObjPtr vm, - virDomainCheckpointDefPtr def) -{ - int ret = -1; - size_t i; - char *xml = NULL; - qemuDomainObjPrivatePtr priv = vm->privateData; - - /* Easiest way to clone inactive portion of vm->def is via - * conversion in and back out of xml. */ - if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, - vm->def, priv->origCPU, - true, true)) || - !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, - priv->qemuCaps, - VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) - goto cleanup; - - if (virDomainCheckpointAlignDisks(def) < 0) - goto cleanup; - - for (i = 0; i < def->ndisks; i++) { - virDomainCheckpointDiskDefPtr disk = &def->disks[i]; - - if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) - continue; - - if (vm->def->disks[i]->src->format != VIR_STORAGE_FILE_QCOW2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("checkpoint for disk %s unsupported " - "for storage type %s"), - disk->name, - virStorageFileFormatTypeToString( - vm->def->disks[i]->src->format)); - goto cleanup; - } - } - - ret = 0; - - cleanup: - VIR_FREE(xml); - return ret; -} - -static int -qemuDomainCheckpointAddActions(virDomainObjPtr vm, - virJSONValuePtr actions, - virDomainMomentObjPtr old_current, - virDomainCheckpointDefPtr def) -{ - size_t i, j; - virDomainCheckpointDefPtr olddef; - virDomainMomentObjPtr parent; - bool search_parents; - - for (i = 0; i < def->ndisks; i++) { - virDomainCheckpointDiskDef *disk = &def->disks[i]; - const char *node; - - if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) - continue; - node = qemuDomainDiskNodeFormatLookup(vm, disk->name); - if (qemuMonitorJSONTransactionAdd(actions, - "block-dirty-bitmap-add", - "s:node", node, - "s:name", disk->bitmap, - "b:persistent", true, - NULL) < 0) - return -1; - - /* We only want one active bitmap for a disk along the - * checkpoint chain, then later differential backups will - * merge the bitmaps (only one active) between the bounding - * checkpoint and the leaf checkpoint. If the same disks are - * involved in each checkpoint, this search terminates in one - * iteration; but it is also possible to have to search - * further than the immediate parent to find another - * checkpoint with a bitmap on the same disk. */ - search_parents = true; - for (parent = old_current; search_parents && parent; - parent = virDomainCheckpointFindByName(vm->checkpoints, - olddef->parent.parent_name)) { - olddef = virDomainCheckpointObjGetDef(parent); - for (j = 0; j < olddef->ndisks; j++) { - virDomainCheckpointDiskDef *disk2; - - disk2 = &olddef->disks[j]; - if (STRNEQ(disk->name, disk2->name) || - disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) - continue; - if (qemuMonitorJSONTransactionAdd(actions, - "block-dirty-bitmap-disable", - "s:node", node, - "s:name", disk2->bitmap, - NULL) < 0) - return -1; - search_parents = false; - break; - } - } - } - return 0; -} - static virDomainCheckpointPtr qemuDomainCheckpointCreateXML(virDomainPtr domain, const char *xmlDesc, unsigned int flags) { - virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL; - char *xml = NULL; - virDomainMomentObjPtr chk = NULL; virDomainCheckpointPtr checkpoint = NULL; - bool update_current = true; - bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE; - unsigned int parse_flags = 0; - virDomainMomentObjPtr other = NULL; - virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; - qemuDomainObjPrivatePtr priv; - virJSONValuePtr actions = NULL; - int ret; - VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL; - - virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE, NULL); - /* TODO: VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE */ - - if (redefine) { - parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE; - update_current = false; - } if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; - if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot create checkpoint while snapshot exists")); - goto cleanup; - } - - priv = vm->privateData; - cfg = virQEMUDriverGetConfig(driver); - if (virDomainCheckpointCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0) goto cleanup; - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("qemu binary lacks persistent bitmaps support")); - goto cleanup; - } - - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot create checkpoint for inactive domain")); - goto cleanup; - } - - if (!(def = virDomainCheckpointDefParseString(xmlDesc, caps, driver->xmlopt, - priv->qemuCaps, parse_flags))) - goto cleanup; - /* Unlike snapshots, the RNG schema already ensured a sane filename. */ - - /* We are going to modify the domain below. */ - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - - if (redefine) { - if (virDomainCheckpointRedefinePrep(vm, &def, &chk, - driver->xmlopt, - &update_current) < 0) - goto endjob; - } else if (qemuDomainCheckpointPrepare(driver, caps, vm, def) < 0) { - goto endjob; - } - - if (!chk) { - if (!(chk = virDomainCheckpointAssignDef(vm->checkpoints, def))) - goto endjob; - - def = NULL; - } - - other = virDomainCheckpointGetCurrent(vm->checkpoints); - if (other) { - if (!redefine && - VIR_STRDUP(chk->def->parent_name, other->def->name) < 0) - goto endjob; - if (update_current) { - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - if (qemuDomainCheckpointWriteMetadata(vm, other, - driver->caps, driver->xmlopt, - cfg->checkpointDir) < 0) - goto endjob; - } - } - - /* actually do the checkpoint */ - if (redefine) { - /* XXX Should we validate that the redefined checkpoint even - * makes sense, such as checking that qemu-img recognizes the - * checkpoint bitmap name in at least one of the domain's disks? */ - } else { - if (!(actions = virJSONValueNewArray())) - goto endjob; - if (qemuDomainCheckpointAddActions(vm, actions, other, - virDomainCheckpointObjGetDef(chk)) < 0) - goto endjob; - qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorTransaction(priv->mon, &actions); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) - goto endjob; - } - - /* If we fail after this point, there's not a whole lot we can do; - * we've successfully created the checkpoint, so we have to go - * forward the best we can. - */ - checkpoint = virGetDomainCheckpoint(domain, chk->def->name); - - endjob: - if (checkpoint) { - if (update_current) - virDomainCheckpointSetCurrent(vm->checkpoints, chk); - if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps, - driver->xmlopt, - cfg->checkpointDir) < 0) { - /* if writing of metadata fails, error out rather than trying - * to silently carry on without completing the checkpoint */ - virObjectUnref(checkpoint); - checkpoint = NULL; - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to save metadata for checkpoint %s"), - chk->def->name); - virDomainCheckpointObjListRemove(vm->checkpoints, chk); - } else { - virDomainCheckpointLinkParent(vm->checkpoints, chk); - } - } else if (chk) { - virDomainCheckpointObjListRemove(vm->checkpoints, chk); - } - - qemuDomainObjEndJob(driver, vm); + checkpoint = qemuCheckpointCreateXML(domain, vm, xmlDesc, flags); cleanup: - virJSONValueFree(actions); virDomainObjEndAPI(&vm); - VIR_FREE(xml); - virObjectUnref(caps); - virObjectUnref(cfg); return checkpoint; } @@ -17511,15 +17237,8 @@ static char * qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint, unsigned int flags) { - virQEMUDriverPtr driver = checkpoint->domain->conn->privateData; virDomainObjPtr vm = NULL; char *xml = NULL; - virDomainMomentObjPtr chk = NULL; - virDomainCheckpointDefPtr chkdef; - unsigned int format_flags; - - virCheckFlags(VIR_DOMAIN_CHECKPOINT_XML_SECURE | - VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN, NULL); if (!(vm = qemuDomObjFromCheckpoint(checkpoint))) return NULL; @@ -17527,13 +17246,7 @@ qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint, if (virDomainCheckpointGetXMLDescEnsureACL(checkpoint->domain->conn, vm->def, flags) < 0) goto cleanup; - if (!(chk = qemuCheckpointObjFromCheckpoint(vm, checkpoint))) - goto cleanup; - chkdef = virDomainCheckpointObjGetDef(chk); - - format_flags = virDomainCheckpointFormatConvertXMLFlags(flags); - xml = virDomainCheckpointDefFormat(chkdef, driver->caps, driver->xmlopt, - format_flags); + xml = qemuCheckpointGetXMLDesc(vm, checkpoint, flags); cleanup: virDomainObjEndAPI(&vm); @@ -17545,108 +17258,23 @@ static int qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint, unsigned int flags) { - virQEMUDriverPtr driver = checkpoint->domain->conn->privateData; virDomainObjPtr vm = NULL; - qemuDomainObjPrivatePtr priv; int ret = -1; - virDomainMomentObjPtr chk = NULL; - virQEMUMomentRemove rem; - virQEMUMomentReparent rep; - bool metadata_only = !!(flags & VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY); - virQEMUDriverConfigPtr cfg = NULL; - - virCheckFlags(VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN | - VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY | - VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY, -1); if (!(vm = qemuDomObjFromCheckpoint(checkpoint))) return -1; - cfg = virQEMUDriverGetConfig(driver); - if (virDomainCheckpointDeleteEnsureACL(checkpoint->domain->conn, vm->def) < 0) goto cleanup; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - - priv = vm->privateData; - if (!metadata_only) { - /* Until qemu-img supports offline bitmap deletion, we are stuck - * with requiring a running guest */ - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot delete checkpoint for inactive domain")); - goto endjob; - } - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("qemu binary lacks persistent bitmaps support")); - goto endjob; - } - } - - if (!(chk = qemuCheckpointObjFromCheckpoint(vm, checkpoint))) - goto endjob; - - if (flags & (VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN | - VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY)) { - rem.driver = driver; - rem.vm = vm; - rem.metadata_only = metadata_only; - rem.err = 0; - rem.current = virDomainCheckpointGetCurrent(vm->checkpoints); - rem.found = false; - rem.momentDiscard = qemuDomainCheckpointDiscard; - virDomainMomentForEachDescendant(chk, qemuDomainMomentDiscardAll, - &rem); - if (rem.err < 0) - goto endjob; - if (rem.found) { - virDomainCheckpointSetCurrent(vm->checkpoints, chk); - if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) { - if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps, - driver->xmlopt, - cfg->checkpointDir) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to set checkpoint '%s' as current"), - chk->def->name); - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - goto endjob; - } - } - } - } else if (chk->nchildren) { - rep.dir = cfg->checkpointDir; - rep.parent = chk->parent; - rep.vm = vm; - rep.err = 0; - rep.caps = driver->caps; - rep.xmlopt = driver->xmlopt; - rep.writeMetadata = qemuDomainCheckpointWriteMetadata; - virDomainMomentForEachChild(chk, qemuDomainMomentReparentChildren, - &rep); - if (rep.err < 0) - goto endjob; - virDomainMomentMoveChildren(chk, chk->parent); - } - - if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) { - virDomainMomentDropChildren(chk); - ret = 0; - } else { - ret = qemuDomainCheckpointDiscard(driver, vm, chk, true, metadata_only); - } - - endjob: - qemuDomainObjEndJob(driver, vm); + ret = qemuCheckpointDelete(vm, checkpoint, flags); cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } + static int qemuDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result, unsigned int flags) { -- 2.21.0

On Wed, Sep 25, 2019 at 14:54:41 +0200, Peter Krempa wrote:
Move all extensive functions to a new file so that we don't just pile everything in the common files. This obviously isn't possible with straight code movement as we still need stubs in qemu_driver.c
Additionally some functions e.g. for looking up a checkpoint by name were so short that moving the impl didn't make sense.
Note that in the move the new file also doesn't use virQEMUMomentReparent but rather an stripped down copy. As I plan to split out snapshot code into a separate file the unification doesn't make sense any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_checkpoint.c | 483 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_checkpoint.h | 50 ++++ src/qemu/qemu_driver.c | 382 +---------------------------- 4 files changed, 540 insertions(+), 377 deletions(-) create mode 100644 src/qemu/qemu_checkpoint.c create mode 100644 src/qemu/qemu_checkpoint.h
[...]
+virDomainCheckpointPtr +qemuCheckpointCreateXML(virDomainPtr domain, + virDomainObjPtr vm, + const char *xmlDesc, + unsigned int flags) +{
[...]
+ endjob: + if (checkpoint) { + if (update_current) + virDomainCheckpointSetCurrent(vm->checkpoints, chk); + if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { + /* if writing of metadata fails, error out rather than trying + * to silently carry on without completing the checkpoint */ + virObjectUnref(checkpoint); + checkpoint = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to save metadata for checkpoint %s"), + chk->def->name); + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + } else { + virDomainCheckpointLinkParent(vm->checkpoints, chk); + } + } else if (chk) { + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + } + + qemuDomainObjEndJob(driver, vm); + + cleanup: + virJSONValueFree(actions); + virDomainObjEndAPI(&vm);
This is already called in qemu_driver.c so we'd try to release the object twice as we don't pass it in via a double pointer. I've deleted the above line from my private branch.
+ virObjectUnref(caps); + virObjectUnref(cfg); + return checkpoint; +}

On 9/25/19 7:54 AM, Peter Krempa wrote:
Move all extensive functions to a new file so that we don't just pile everything in the common files. This obviously isn't possible with straight code movement as we still need stubs in qemu_driver.c
Additionally some functions e.g. for looking up a checkpoint by name were so short that moving the impl didn't make sense.
Note that in the move the new file also doesn't use virQEMUMomentReparent but rather an stripped down copy. As I plan to
s/an/a/
split out snapshot code into a separate file the unification doesn't make sense any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_checkpoint.c | 483 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_checkpoint.h | 50 ++++ src/qemu/qemu_driver.c | 382 +---------------------------- 4 files changed, 540 insertions(+), 377 deletions(-) create mode 100644 src/qemu/qemu_checkpoint.c create mode 100644 src/qemu/qemu_checkpoint.h
With the double-free fix you posted in the followup,
+++ b/src/qemu/Makefile.inc.am @@ -68,6 +68,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_vhost_user.h \ qemu/qemu_vhost_user_gpu.c \ qemu/qemu_vhost_user_gpu.h \ + qemu/qemu_checkpoint.c \ + qemu/qemu_checkpoint.h \ $(NULL)
Is it worth keeping this list sorted alphabetically?
+/* Called inside job lock */ +static int +qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
Worth splitting these parameters to separate lines? (I know you're just moving code, and it's my fault they weren't split in my commit...)
+ virDomainObjPtr vm, + virDomainCheckpointDefPtr def) +{
+ +struct virQEMUCheckpointReparent { + const char *dir; + virDomainMomentObjPtr parent; + virDomainObjPtr vm; + virCapsPtr caps; + virDomainXMLOptionPtr xmlopt; + int err; +}; + + +static int +qemuCheckpointReparentChildren(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainMomentObjPtr moment = payload; + struct virQEMUCheckpointReparent *rep = data;
There's a double space here we could fix.
+int +qemuCheckpointDelete(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint, + unsigned int flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + virDomainMomentObjPtr chk = NULL; + virQEMUMomentRemove rem; + struct virQEMUCheckpointReparent rep;
You explained why you unshared virQEMUMomentReparent, but why not instead move it to be alongside virQEMUMomentRemove, which is still shared? Yes, I know it's odd that we had two callback structs, split between two different files declaring those structs, but rather than duplicating things for snapshot vs. checkpoint, keeping the two shared structs side-by-side might make more sense.
+++ b/src/qemu/qemu_driver.c
static virDomainCheckpointPtr qemuDomainCheckpointCreateXML(virDomainPtr domain, const char *xmlDesc, unsigned int flags) { - virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL;
if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup;
- if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot create checkpoint while snapshot exists")); - goto cleanup; - } - - priv = vm->privateData; - cfg = virQEMUDriverGetConfig(driver); - if (virDomainCheckpointCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0) goto cleanup;
Didn't you need to fix this separately, for security reasons? Otherwise, the code motion makes sense. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

On Thu, Sep 26, 2019 at 10:38:11 -0500, Eric Blake wrote:
On 9/25/19 7:54 AM, Peter Krempa wrote:
Move all extensive functions to a new file so that we don't just pile everything in the common files. This obviously isn't possible with straight code movement as we still need stubs in qemu_driver.c
Additionally some functions e.g. for looking up a checkpoint by name were so short that moving the impl didn't make sense.
Note that in the move the new file also doesn't use virQEMUMomentReparent but rather an stripped down copy. As I plan to
s/an/a/
split out snapshot code into a separate file the unification doesn't make sense any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_checkpoint.c | 483 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_checkpoint.h | 50 ++++ src/qemu/qemu_driver.c | 382 +---------------------------- 4 files changed, 540 insertions(+), 377 deletions(-) create mode 100644 src/qemu/qemu_checkpoint.c create mode 100644 src/qemu/qemu_checkpoint.h
With the double-free fix you posted in the followup,
+++ b/src/qemu/Makefile.inc.am @@ -68,6 +68,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_vhost_user.h \ qemu/qemu_vhost_user_gpu.c \ qemu/qemu_vhost_user_gpu.h \ + qemu/qemu_checkpoint.c \ + qemu/qemu_checkpoint.h \ $(NULL)
Is it worth keeping this list sorted alphabetically?
+/* Called inside job lock */ +static int +qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps,
Worth splitting these parameters to separate lines? (I know you're just moving code, and it's my fault they weren't split in my commit...)
+ virDomainObjPtr vm, + virDomainCheckpointDefPtr def) +{
+ +struct virQEMUCheckpointReparent { + const char *dir; + virDomainMomentObjPtr parent; + virDomainObjPtr vm; + virCapsPtr caps; + virDomainXMLOptionPtr xmlopt; + int err; +}; + + +static int +qemuCheckpointReparentChildren(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainMomentObjPtr moment = payload; + struct virQEMUCheckpointReparent *rep = data;
There's a double space here we could fix.
+int +qemuCheckpointDelete(virDomainObjPtr vm, + virDomainCheckpointPtr checkpoint, + unsigned int flags) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + virDomainMomentObjPtr chk = NULL; + virQEMUMomentRemove rem; + struct virQEMUCheckpointReparent rep;
You explained why you unshared virQEMUMomentReparent, but why not instead move it to be alongside virQEMUMomentRemove, which is still shared? Yes, I know it's odd that we had two callback structs, split between two different files declaring those structs, but rather than duplicating things for snapshot vs. checkpoint, keeping the two shared structs side-by-side might make more sense.
I plan also to split everything snapshot related out so I thought of getting completely rid of the code sharing here since it's not significant.
+++ b/src/qemu/qemu_driver.c
static virDomainCheckpointPtr qemuDomainCheckpointCreateXML(virDomainPtr domain, const char *xmlDesc, unsigned int flags) { - virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL;
if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup;
- if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot create checkpoint while snapshot exists")); - goto cleanup; - } - - priv = vm->privateData; - cfg = virQEMUDriverGetConfig(driver); - if (virDomainCheckpointCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0) goto cleanup;
Didn't you need to fix this separately, for security reasons?
Oops, I didn't notice this one. I'll post it separately probably just to have a commit to reference.

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 185 ++++++++++++++++++++++++++++++++++--- src/qemu/qemu_checkpoint.h | 5 + src/qemu/qemu_domain.c | 162 +------------------------------- src/qemu/qemu_domain.h | 15 --- 4 files changed, 180 insertions(+), 187 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 9e9af76871..22f0ffb26e 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -76,6 +76,169 @@ qemuCheckpointObjFromCheckpoint(virDomainObjPtr vm, } +static int +qemuCheckpointWriteMetadata(virDomainObjPtr vm, + virDomainMomentObjPtr checkpoint, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + const char *checkpointDir) +{ + unsigned int flags = VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE; + virDomainCheckpointDefPtr def = virDomainCheckpointObjGetDef(checkpoint); + VIR_AUTOFREE(char *) newxml = NULL; + VIR_AUTOFREE(char *) chkDir = NULL; + VIR_AUTOFREE(char *) chkFile = NULL; + + newxml = virDomainCheckpointDefFormat(def, caps, xmlopt, flags); + if (newxml == NULL) + return -1; + + if (virAsprintf(&chkDir, "%s/%s", checkpointDir, vm->def->name) < 0) + return -1; + if (virFileMakePath(chkDir) < 0) { + virReportSystemError(errno, _("cannot create checkpoint directory '%s'"), + chkDir); + return -1; + } + + if (virAsprintf(&chkFile, "%s/%s.xml", chkDir, def->parent.name) < 0) + return -1; + + return virXMLSaveFile(chkFile, NULL, "checkpoint-edit", newxml); +} + + +static int +qemuDomainCheckpointDiscard(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMomentObjPtr chk, + bool update_parent, + bool metadata_only) +{ + virDomainMomentObjPtr parent = NULL; + virDomainMomentObjPtr moment; + virDomainCheckpointDefPtr parentdef = NULL; + size_t i, j; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOFREE(char *) chkFile = NULL; + + if (!metadata_only && !virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot remove checkpoint from inactive domain")); + return -1; + } + + if (virAsprintf(&chkFile, "%s/%s/%s.xml", cfg->checkpointDir, + vm->def->name, chk->def->name) < 0) + return -1; + + if (!metadata_only) { + qemuDomainObjPrivatePtr priv = vm->privateData; + bool success = true; + bool search_parents; + virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk); + + qemuDomainObjEnterMonitor(driver, vm); + parent = virDomainCheckpointFindByName(vm->checkpoints, + chk->def->parent_name); + for (i = 0; i < chkdef->ndisks; i++) { + virDomainCheckpointDiskDef *disk = &chkdef->disks[i]; + const char *node; + + if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) + continue; + + node = qemuDomainDiskNodeFormatLookup(vm, disk->name); + /* If any ancestor checkpoint has a bitmap for the same + * disk, then this bitmap must be merged to the + * ancestor. */ + search_parents = true; + for (moment = parent; + search_parents && moment; + moment = virDomainCheckpointFindByName(vm->checkpoints, + parentdef->parent.parent_name)) { + parentdef = virDomainCheckpointObjGetDef(moment); + for (j = 0; j < parentdef->ndisks; j++) { + virDomainCheckpointDiskDef *disk2; + VIR_AUTOPTR(virJSONValue) arr = NULL; + + disk2 = &parentdef->disks[j]; + if (STRNEQ(disk->name, disk2->name) || + disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) + continue; + search_parents = false; + + arr = virJSONValueNewArray(); + if (!arr || + virJSONValueArrayAppendString(arr, disk->bitmap) < 0) { + success = false; + break; + } + if (chk == virDomainCheckpointGetCurrent(vm->checkpoints) && + qemuMonitorEnableBitmap(priv->mon, node, + disk2->bitmap) < 0) { + success = false; + break; + } + if (qemuMonitorMergeBitmaps(priv->mon, node, + disk2->bitmap, &arr) < 0) { + success = false; + break; + } + } + } + if (qemuMonitorDeleteBitmap(priv->mon, node, disk->bitmap) < 0) { + success = false; + break; + } + } + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !success) + return -1; + } + + if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) { + virDomainCheckpointSetCurrent(vm->checkpoints, NULL); + if (update_parent && parent) { + virDomainCheckpointSetCurrent(vm->checkpoints, parent); + if (qemuCheckpointWriteMetadata(vm, parent, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { + VIR_WARN("failed to set parent checkpoint '%s' as current", + chk->def->parent_name); + virDomainCheckpointSetCurrent(vm->checkpoints, NULL); + } + } + } + + if (unlink(chkFile) < 0) + VIR_WARN("Failed to unlink %s", chkFile); + if (update_parent) + virDomainMomentDropParent(chk); + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + + return 0; +} + + +int +qemuDomainCheckpointDiscardAllMetadata(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUMomentRemove rem = { + .driver = driver, + .vm = vm, + .metadata_only = true, + .momentDiscard = qemuDomainCheckpointDiscard, + }; + + virDomainCheckpointForEach(vm->checkpoints, qemuDomainMomentDiscardAll, + &rem); + virDomainCheckpointObjListRemoveAll(vm->checkpoints); + + return rem.err; +} + + /* Called inside job lock */ static int qemuDomainCheckpointPrepare(virQEMUDriverPtr driver, virCapsPtr caps, @@ -267,9 +430,9 @@ qemuCheckpointCreateXML(virDomainPtr domain, goto endjob; if (update_current) { virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - if (qemuDomainCheckpointWriteMetadata(vm, other, - driver->caps, driver->xmlopt, - cfg->checkpointDir) < 0) + if (qemuCheckpointWriteMetadata(vm, other, + driver->caps, driver->xmlopt, + cfg->checkpointDir) < 0) goto endjob; } } @@ -301,9 +464,9 @@ qemuCheckpointCreateXML(virDomainPtr domain, if (checkpoint) { if (update_current) virDomainCheckpointSetCurrent(vm->checkpoints, chk); - if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps, - driver->xmlopt, - cfg->checkpointDir) < 0) { + if (qemuCheckpointWriteMetadata(vm, chk, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { /* if writing of metadata fails, error out rather than trying * to silently carry on without completing the checkpoint */ virObjectUnref(checkpoint); @@ -384,8 +547,8 @@ qemuCheckpointReparentChildren(void *payload, return 0; } - rep->err = qemuDomainCheckpointWriteMetadata(rep->vm, moment, rep->caps, - rep->xmlopt, rep->dir); + rep->err = qemuCheckpointWriteMetadata(rep->vm, moment, rep->caps, + rep->xmlopt, rep->dir); return 0; } @@ -445,9 +608,9 @@ qemuCheckpointDelete(virDomainObjPtr vm, if (rem.found) { virDomainCheckpointSetCurrent(vm->checkpoints, chk); if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) { - if (qemuDomainCheckpointWriteMetadata(vm, chk, driver->caps, - driver->xmlopt, - cfg->checkpointDir) < 0) { + if (qemuCheckpointWriteMetadata(vm, chk, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to set checkpoint '%s' as current"), chk->def->name); diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h index 0d36d718d6..4cea4bcdb4 100644 --- a/src/qemu/qemu_checkpoint.h +++ b/src/qemu/qemu_checkpoint.h @@ -20,6 +20,7 @@ #include "virconftypes.h" #include "datatypes.h" +#include "qemu_conf.h" virDomainObjPtr qemuDomObjFromCheckpoint(virDomainCheckpointPtr checkpoint); @@ -32,6 +33,10 @@ virDomainMomentObjPtr qemuCheckpointObjFromName(virDomainObjPtr vm, const char *name); +int +qemuDomainCheckpointDiscardAllMetadata(virQEMUDriverPtr driver, + virDomainObjPtr vm); + virDomainCheckpointPtr qemuCheckpointCreateXML(virDomainPtr domain, virDomainObjPtr vm, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e8e895d9aa..19fa5420e7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -35,6 +35,7 @@ #include "qemu_slirp.h" #include "qemu_extdevice.h" #include "qemu_blockjob.h" +#include "qemu_checkpoint.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" @@ -9600,36 +9601,6 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, return ret; } -int -qemuDomainCheckpointWriteMetadata(virDomainObjPtr vm, - virDomainMomentObjPtr checkpoint, - virCapsPtr caps, - virDomainXMLOptionPtr xmlopt, - const char *checkpointDir) -{ - unsigned int flags = VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE; - virDomainCheckpointDefPtr def = virDomainCheckpointObjGetDef(checkpoint); - VIR_AUTOFREE(char *) newxml = NULL; - VIR_AUTOFREE(char *) chkDir = NULL; - VIR_AUTOFREE(char *) chkFile = NULL; - - newxml = virDomainCheckpointDefFormat(def, caps, xmlopt, flags); - if (newxml == NULL) - return -1; - - if (virAsprintf(&chkDir, "%s/%s", checkpointDir, vm->def->name) < 0) - return -1; - if (virFileMakePath(chkDir) < 0) { - virReportSystemError(errno, _("cannot create checkpoint directory '%s'"), - chkDir); - return -1; - } - - if (virAsprintf(&chkFile, "%s/%s.xml", chkDir, def->parent.name) < 0) - return -1; - - return virXMLSaveFile(chkFile, NULL, "checkpoint-edit", newxml); -} /* The domain is expected to be locked and inactive. Return -1 on normal * failure, 1 if we skipped a disk due to try_all. */ @@ -9827,137 +9798,6 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, } -/* Discard one checkpoint (or its metadata), without reparenting any children. */ -int -qemuDomainCheckpointDiscard(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMomentObjPtr chk, - bool update_parent, - bool metadata_only) -{ - virDomainMomentObjPtr parent = NULL; - virDomainMomentObjPtr moment; - virDomainCheckpointDefPtr parentdef = NULL; - size_t i, j; - VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); - VIR_AUTOFREE(char *) chkFile = NULL; - - if (!metadata_only && !virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot remove checkpoint from inactive domain")); - return -1; - } - - if (virAsprintf(&chkFile, "%s/%s/%s.xml", cfg->checkpointDir, - vm->def->name, chk->def->name) < 0) - return -1; - - if (!metadata_only) { - qemuDomainObjPrivatePtr priv = vm->privateData; - bool success = true; - bool search_parents; - virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk); - - qemuDomainObjEnterMonitor(driver, vm); - parent = virDomainCheckpointFindByName(vm->checkpoints, - chk->def->parent_name); - for (i = 0; i < chkdef->ndisks; i++) { - virDomainCheckpointDiskDef *disk = &chkdef->disks[i]; - const char *node; - - if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) - continue; - - node = qemuDomainDiskNodeFormatLookup(vm, disk->name); - /* If any ancestor checkpoint has a bitmap for the same - * disk, then this bitmap must be merged to the - * ancestor. */ - search_parents = true; - for (moment = parent; - search_parents && moment; - moment = virDomainCheckpointFindByName(vm->checkpoints, - parentdef->parent.parent_name)) { - parentdef = virDomainCheckpointObjGetDef(moment); - for (j = 0; j < parentdef->ndisks; j++) { - virDomainCheckpointDiskDef *disk2; - VIR_AUTOPTR(virJSONValue) arr = NULL; - - disk2 = &parentdef->disks[j]; - if (STRNEQ(disk->name, disk2->name) || - disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) - continue; - search_parents = false; - - arr = virJSONValueNewArray(); - if (!arr || - virJSONValueArrayAppendString(arr, disk->bitmap) < 0) { - success = false; - break; - } - if (chk == virDomainCheckpointGetCurrent(vm->checkpoints) && - qemuMonitorEnableBitmap(priv->mon, node, - disk2->bitmap) < 0) { - success = false; - break; - } - if (qemuMonitorMergeBitmaps(priv->mon, node, - disk2->bitmap, &arr) < 0) { - success = false; - break; - } - } - } - if (qemuMonitorDeleteBitmap(priv->mon, node, disk->bitmap) < 0) { - success = false; - break; - } - } - if (qemuDomainObjExitMonitor(driver, vm) < 0 || !success) - return -1; - } - - if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) { - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - if (update_parent && parent) { - virDomainCheckpointSetCurrent(vm->checkpoints, parent); - if (qemuDomainCheckpointWriteMetadata(vm, parent, driver->caps, - driver->xmlopt, - cfg->checkpointDir) < 0) { - VIR_WARN("failed to set parent checkpoint '%s' as current", - chk->def->parent_name); - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - } - } - } - - if (unlink(chkFile) < 0) - VIR_WARN("Failed to unlink %s", chkFile); - if (update_parent) - virDomainMomentDropParent(chk); - virDomainCheckpointObjListRemove(vm->checkpoints, chk); - - return 0; -} - -int -qemuDomainCheckpointDiscardAllMetadata(virQEMUDriverPtr driver, - virDomainObjPtr vm) -{ - virQEMUMomentRemove rem = { - .driver = driver, - .vm = vm, - .metadata_only = true, - .momentDiscard = qemuDomainCheckpointDiscard, - }; - - virDomainCheckpointForEach(vm->checkpoints, qemuDomainMomentDiscardAll, - &rem); - virDomainCheckpointObjListRemoveAll(vm->checkpoints); - - return rem.err; -} - - static void qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, virDomainObjPtr vm) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c740bb955a..f45da882a8 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -792,21 +792,6 @@ int qemuDomainMomentDiscardAll(void *payload, int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, virDomainObjPtr vm); -int qemuDomainCheckpointWriteMetadata(virDomainObjPtr vm, - virDomainMomentObjPtr checkpoint, - virCapsPtr caps, - virDomainXMLOptionPtr xmlopt, - const char *checkpointDir); - -int qemuDomainCheckpointDiscard(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMomentObjPtr chk, - bool update_current, - bool metadata_only); - -int qemuDomainCheckpointDiscardAllMetadata(virQEMUDriverPtr driver, - virDomainObjPtr vm); - void qemuDomainRemoveInactive(virQEMUDriverPtr driver, virDomainObjPtr vm); -- 2.21.0

On 9/25/19 7:54 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 185 ++++++++++++++++++++++++++++++++++--- src/qemu/qemu_checkpoint.h | 5 + src/qemu/qemu_domain.c | 162 +------------------------------- src/qemu/qemu_domain.h | 15 --- 4 files changed, 180 insertions(+), 187 deletions(-)
Thanks for splitting the code motion into two patches. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

There's nothing that uses it directly now. Also not allowing direct use will promote our layering. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7163216f69..5852df2a53 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -47,7 +47,6 @@ #include "qemu_hostdev.h" #include "qemu_hotplug.h" #include "qemu_monitor.h" -#include "qemu_monitor_json.h" #include "qemu_process.h" #include "qemu_migration.h" #include "qemu_migration_params.h" -- 2.21.0

On 9/25/19 7:54 AM, Peter Krempa wrote:
There's nothing that uses it directly now. Also not allowing direct use will promote our layering.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7163216f69..5852df2a53 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -47,7 +47,6 @@ #include "qemu_hostdev.h" #include "qemu_hotplug.h" #include "qemu_monitor.h" -#include "qemu_monitor_json.h" #include "qemu_process.h" #include "qemu_migration.h" #include "qemu_migration_params.h"
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (3)
-
Eric Blake
-
Ján Tomko
-
Peter Krempa