Re: [libvirt] [PATCH 3/4] snapshot: populate new XML info for qemu snapshots

Now that the XML supports listing internal snapshots, it is worth always populating the <memory> and <disks> element to match. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Always parse disk info and set memory info. --- src/qemu/qemu_driver.c | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8e8e00c..d5cea59 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11073,8 +11073,10 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainSnapshotDefPtr def = NULL; bool update_current = true; - unsigned int parse_flags = 0; + unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; virDomainSnapshotObjPtr other = NULL; + int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; + int align_match = true; virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | @@ -11098,8 +11100,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, update_current = false; if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; - if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) - parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; qemuDriverLock(driver); virUUIDFormat(domain->uuid, uuidstr); @@ -11120,6 +11120,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, _("cannot halt after transient domain snapshot")); goto cleanup; } + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) || + !virDomainObjIsActive(vm)) + parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE; if (!(def = virDomainSnapshotDefParseString(xmlDesc, driver->caps, QEMU_EXPECTED_VIRT_TYPES, @@ -11160,6 +11163,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } /* Check that any replacement is compatible */ + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && + def->state != VIR_DOMAIN_DISK_SNAPSHOT) { + virReportError(VIR_ERR_INVALID_ARG, + _("disk-only flag for snapshot %s requires " + "disk-snapshot state"),

+ def->name); + goto cleanup; + + } if (def->dom && memcmp(def->dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) { virReportError(VIR_ERR_INVALID_ARG, @@ -11209,10 +11221,13 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, other->def = NULL; snap = other; } - if (def->state == VIR_DOMAIN_DISK_SNAPSHOT && def->dom) { - if (virDomainSnapshotAlignDisks(def, - VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL, - false) < 0) + if (def->dom) { + if (def->state == VIR_DOMAIN_DISK_SNAPSHOT) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + } + if (virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0) goto cleanup; } } else { @@ -11231,13 +11246,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, "implemented yet")); goto cleanup; } - if (virDomainSnapshotAlignDisks(def, - VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL, - false) < 0) - goto cleanup; - if (qemuDomainSnapshotDiskPrepare(vm, def, &flags) < 0) + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + if (virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0 || + qemuDomainSnapshotDiskPrepare(vm, def, &flags) < 0) goto cleanup; def->state = VIR_DOMAIN_DISK_SNAPSHOT; + def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; } else { /* In a perfect world, we would allow qemu to tell us this. * The problem is that qemu only does this check @@ -11248,9 +11264,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * the boot device. This is probably a bug in qemu, but we'll * work around it here for now. */ - if (!qemuDomainSnapshotIsAllowed(vm)) + if (!qemuDomainSnapshotIsAllowed(vm) || + virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0) goto cleanup; def->state = virDomainObjGetState(vm, NULL); + def->memory = (def->state == VIR_DOMAIN_SHUTOFF ? + VIR_DOMAIN_SNAPSHOT_LOCATION_NONE : + VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL); } }