[libvirt] [PATCH v2 7/9] qemu: snapshot: Break out redefine preparation to shared function

--- src/conf/snapshot_conf.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/snapshot_conf.h | 7 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 131 +---------------------------------------- 4 files changed, 160 insertions(+), 129 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 45d6af4..f24aa22 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -1099,3 +1099,153 @@ virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap) { return virDomainSnapshotDefIsExternal(snap->def); } + +int +virDomainSnapshotRedefinePrep(virDomainPtr domain, + virDomainObjPtr vm, + virDomainSnapshotDefPtr *defptr, + virDomainSnapshotObjPtr *snap, + bool *update_current, + unsigned int flags) +{ + virDomainSnapshotDefPtr def = *defptr; + int ret = -1; + int align_location; + int align_match; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virDomainSnapshotObjPtr other; + + virUUIDFormat(domain->uuid, uuidstr); + + /* Prevent circular chains */ + if (def->parent) { + if (STREQ(def->name, def->parent)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot set snapshot %s as its own parent"), + def->name); + goto cleanup; + } + other = virDomainSnapshotFindByName(vm->snapshots, def->parent); + if (!other) { + virReportError(VIR_ERR_INVALID_ARG, + _("parent %s for snapshot %s not found"), + def->parent, def->name); + goto cleanup; + } + while (other->def->parent) { + if (STREQ(other->def->parent, def->name)) { + virReportError(VIR_ERR_INVALID_ARG, + _("parent %s would create cycle to %s"), + other->def->name, def->name); + goto cleanup; + } + other = virDomainSnapshotFindByName(vm->snapshots, + other->def->parent); + if (!other) { + VIR_WARN("snapshots are inconsistent for %s", + vm->def->name); + break; + } + } + } + + /* 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, + _("definition for snapshot %s must use uuid %s"), + def->name, uuidstr); + goto cleanup; + } + + other = virDomainSnapshotFindByName(vm->snapshots, def->name); + if (other) { + if ((other->def->state == VIR_DOMAIN_RUNNING || + other->def->state == VIR_DOMAIN_PAUSED) != + (def->state == VIR_DOMAIN_RUNNING || + def->state == VIR_DOMAIN_PAUSED)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot change between online and offline " + "snapshot state in snapshot %s"), + def->name); + goto cleanup; + } + + if ((other->def->state == VIR_DOMAIN_DISK_SNAPSHOT) != + (def->state == VIR_DOMAIN_DISK_SNAPSHOT)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot change between disk snapshot and " + "system checkpoint in snapshot %s"), + def->name); + goto cleanup; + } + + if (other->def->dom) { + if (def->dom) { + if (!virDomainDefCheckABIStability(other->def->dom, + def->dom)) + goto cleanup; + } else { + /* Transfer the domain def */ + def->dom = other->def->dom; + other->def->dom = NULL; + } + } + + if (def->dom) { + if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || + virDomainSnapshotDefIsExternal(def)) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + } + + if (virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0) { + /* revert stealing of the snapshot domain definition */ + if (def->dom && !other->def->dom) { + other->def->dom = def->dom; + def->dom = NULL; + } + goto cleanup; + } + } + + if (other == vm->current_snapshot) { + *update_current = true; + vm->current_snapshot = NULL; + } + + /* Drop and rebuild the parent relationship, but keep all + * child relations by reusing snap. */ + virDomainSnapshotDropParent(other); + virDomainSnapshotDefFree(other->def); + other->def = def; + *defptr = NULL; + *snap = other; + } else { + if (def->dom) { + if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || + def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + } + if (virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0) + goto cleanup; + } + } + + ret = 0; +cleanup: + return ret; +} diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 0b8d18a..e8f8179 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -176,6 +176,13 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots, bool virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def); bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap); +int virDomainSnapshotRedefinePrep(virDomainPtr domain, + virDomainObjPtr vm, + virDomainSnapshotDefPtr *def, + virDomainSnapshotObjPtr *snap, + bool *update_current, + unsigned int flags); + VIR_ENUM_DECL(virDomainSnapshotLocation) VIR_ENUM_DECL(virDomainSnapshotState) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..76fedaf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -650,6 +650,7 @@ virDomainSnapshotLocationTypeToString; virDomainSnapshotObjListGetNames; virDomainSnapshotObjListNum; virDomainSnapshotObjListRemove; +virDomainSnapshotRedefinePrep; virDomainSnapshotStateTypeFromString; virDomainSnapshotStateTypeToString; virDomainSnapshotUpdateRelations; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e7212c1..e9dcc09 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12280,7 +12280,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, char *xml = NULL; virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainSnapshotDefPtr def = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; @@ -12314,8 +12313,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (redefine) parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; - virUUIDFormat(domain->uuid, uuidstr); - if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; @@ -12384,133 +12381,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } if (redefine) { - /* Prevent circular chains */ - if (def->parent) { - if (STREQ(def->name, def->parent)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot set snapshot %s as its own parent"), - def->name); - goto cleanup; - } - other = virDomainSnapshotFindByName(vm->snapshots, def->parent); - if (!other) { - virReportError(VIR_ERR_INVALID_ARG, - _("parent %s for snapshot %s not found"), - def->parent, def->name); - goto cleanup; - } - while (other->def->parent) { - if (STREQ(other->def->parent, def->name)) { - virReportError(VIR_ERR_INVALID_ARG, - _("parent %s would create cycle to %s"), - other->def->name, def->name); - goto cleanup; - } - other = virDomainSnapshotFindByName(vm->snapshots, - other->def->parent); - if (!other) { - VIR_WARN("snapshots are inconsistent for %s", - vm->def->name); - break; - } - } - } - - /* 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, - _("definition for snapshot %s must use uuid %s"), - def->name, uuidstr); + if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, + &update_current, flags) < 0) goto cleanup; - } - - other = virDomainSnapshotFindByName(vm->snapshots, def->name); - if (other) { - if ((other->def->state == VIR_DOMAIN_RUNNING || - other->def->state == VIR_DOMAIN_PAUSED) != - (def->state == VIR_DOMAIN_RUNNING || - def->state == VIR_DOMAIN_PAUSED)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot change between online and offline " - "snapshot state in snapshot %s"), - def->name); - goto cleanup; - } - - if ((other->def->state == VIR_DOMAIN_DISK_SNAPSHOT) != - (def->state == VIR_DOMAIN_DISK_SNAPSHOT)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot change between disk snapshot and " - "system checkpoint in snapshot %s"), - def->name); - goto cleanup; - } - - if (other->def->dom) { - if (def->dom) { - if (!virDomainDefCheckABIStability(other->def->dom, - def->dom)) - goto cleanup; - } else { - /* Transfer the domain def */ - def->dom = other->def->dom; - other->def->dom = NULL; - } - } - - if (def->dom) { - if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || - virDomainSnapshotDefIsExternal(def)) { - align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; - align_match = false; - } - - if (virDomainSnapshotAlignDisks(def, align_location, - align_match) < 0) { - /* revert stealing of the snapshot domain definition */ - if (def->dom && !other->def->dom) { - other->def->dom = def->dom; - def->dom = NULL; - } - goto cleanup; - } - } - - if (other == vm->current_snapshot) { - update_current = true; - vm->current_snapshot = NULL; - } - - /* Drop and rebuild the parent relationship, but keep all - * child relations by reusing snap. */ - virDomainSnapshotDropParent(other); - virDomainSnapshotDefFree(other->def); - other->def = def; - def = NULL; - snap = other; - } else { - if (def->dom) { - if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || - def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { - align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; - align_match = false; - } - if (virDomainSnapshotAlignDisks(def, align_location, - align_match) < 0) - goto cleanup; - } - } } else { /* Easiest way to clone inactive portion of vm->def is via * conversion in and back out of xml. */ -- 1.8.3.1