[libvirt] [PATCH 08/16] snapshot: Access snapshot def directly when needed

An upcoming patch will rework virDomainSnapshotObjList to be generic for both snapshots and checkpoints; reduce the churn by adding a new accessor virDomainSnapshotObjGetDef() which returns the snapshot-specific definition even when the list is rewritten to operate only on a base class, then using it at sites that that are specific to snapshots. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/virdomainsnapshotobj.h | 6 +++++ src/conf/snapshot_conf.c | 41 +++++++++++++++++------------ src/conf/virdomainsnapshotobjlist.c | 17 +++++++----- src/qemu/qemu_domain.c | 6 ++--- 4 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainsnapshotobj.h index 0981ea4c25..8f96bfded9 100644 --- a/src/conf/virdomainsnapshotobj.h +++ b/src/conf/virdomainsnapshotobj.h @@ -52,4 +52,10 @@ void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, virDomainSnapshotObjPtr parent); +static inline virDomainSnapshotDefPtr +virDomainSnapshotObjGetDef(virDomainSnapshotObjPtr obj) +{ + return obj->def; +} + #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */ diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index c692d36bd1..aec23f111c 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -456,8 +456,10 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, } if (other) { - if ((other->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING || - other->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) != + virDomainSnapshotDefPtr otherdef = virDomainSnapshotObjGetDef(other); + + if ((otherdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING || + otherdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) != (def->state == VIR_DOMAIN_SNAPSHOT_RUNNING || def->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) { virReportError(VIR_ERR_INVALID_ARG, @@ -467,7 +469,7 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, return -1; } - if ((other->def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) != + if ((otherdef->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) != (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)) { virReportError(VIR_ERR_INVALID_ARG, _("cannot change between disk only and " @@ -476,15 +478,15 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, return -1; } - if (other->def->dom) { + if (otherdef->dom) { if (def->dom) { - if (!virDomainDefCheckABIStability(other->def->dom, + if (!virDomainDefCheckABIStability(otherdef->dom, def->dom, xmlopt)) return -1; } else { /* Transfer the domain def */ - def->dom = other->def->dom; - other->def->dom = NULL; + def->dom = otherdef->dom; + otherdef->dom = NULL; } } } @@ -909,7 +911,9 @@ virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def) bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap) { - return virDomainSnapshotDefIsExternal(snap->def); + virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snap); + + return virDomainSnapshotDefIsExternal(def); } int @@ -923,6 +927,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, { virDomainSnapshotDefPtr def = *defptr; virDomainSnapshotObjPtr other; + virDomainSnapshotDefPtr otherdef; bool check_if_stolen; /* Prevent circular chains */ @@ -940,15 +945,16 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, def->parent, def->name); return -1; } - while (other->def->parent) { - if (STREQ(other->def->parent, def->name)) { + otherdef = virDomainSnapshotObjGetDef(other); + while (otherdef->parent) { + if (STREQ(otherdef->parent, def->name)) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s would create cycle to %s"), - other->def->name, def->name); + otherdef->name, def->name); return -1; } other = virDomainSnapshotFindByName(vm->snapshots, - other->def->parent); + otherdef->parent); if (!other) { VIR_WARN("snapshots are inconsistent for %s", vm->def->name); @@ -958,12 +964,13 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, } other = virDomainSnapshotFindByName(vm->snapshots, def->name); - check_if_stolen = other && other->def->dom; + otherdef = other ? virDomainSnapshotObjGetDef(other) : NULL; + check_if_stolen = other && otherdef->dom; if (virDomainSnapshotRedefineValidate(def, domain->uuid, other, xmlopt, flags) < 0) { /* revert any stealing of the snapshot domain definition */ - if (check_if_stolen && def->dom && !other->def->dom) { - other->def->dom = def->dom; + if (check_if_stolen && def->dom && !otherdef->dom) { + otherdef->dom = def->dom; def->dom = NULL; } return -1; @@ -977,8 +984,8 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, /* Drop and rebuild the parent relationship, but keep all * child relations by reusing snap. */ virDomainSnapshotDropParent(other); - virDomainSnapshotDefFree(other->def); - other->def = def; + virDomainSnapshotDefFree(otherdef); + otherdef = def; *defptr = NULL; *snap = other; } diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index 9538521ab3..ec670ff5c2 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -171,8 +171,9 @@ virDomainSnapshotFormatOne(void *payload, virDomainSnapshotObjPtr snap = payload; struct virDomainSnapshotFormatData *data = opaque; return virDomainSnapshotDefFormatInternal(data->buf, data->uuidstr, - snap->def, data->caps, - data->xmlopt, data->flags); + virDomainSnapshotObjGetDef(snap), + data->caps, data->xmlopt, + data->flags); } @@ -230,7 +231,7 @@ static void virDomainSnapshotObjFree(virDomainSnapshotObjPtr snapshot) VIR_DEBUG("obj=%p", snapshot); - virDomainSnapshotDefFree(snapshot->def); + virDomainSnapshotDefFree(virDomainSnapshotObjGetDef(snapshot)); VIR_FREE(snapshot); } @@ -316,15 +317,17 @@ static int virDomainSnapshotObjListCopyNames(void *payload, return 0; if (data->flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) { + virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(obj); + if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) && - obj->def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF) + def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF) return 0; if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) && - obj->def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) + def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) return 0; if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) && - obj->def->state != VIR_DOMAIN_SNAPSHOT_SHUTOFF && - obj->def->state != VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) + def->state != VIR_DOMAIN_SNAPSHOT_SHUTOFF && + def->state != VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) return 0; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b1a84d3914..e693a3b122 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8460,12 +8460,12 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, char uuidstr[VIR_UUID_STRING_BUFLEN]; unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE | VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL; + virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snapshot); if (virDomainSnapshotGetCurrent(vm->snapshots) == snapshot) flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT; virUUIDFormat(vm->def->uuid, uuidstr); - newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, caps, xmlopt, - flags); + newxml = virDomainSnapshotDefFormat(uuidstr, def, caps, xmlopt, flags); if (newxml == NULL) return -1; @@ -8477,7 +8477,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, goto cleanup; } - if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, snapshot->def->name) < 0) + if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->name) < 0) goto cleanup; ret = virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml); -- 2.20.1