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.  Use VIR_STEAL_PTR when appropriate in the
affected lines.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
Eric Blake 2019-03-18 16:13:50 -05:00
parent 02c4e24db7
commit 55c2ab3e2b
4 changed files with 43 additions and 30 deletions

View File

@ -461,8 +461,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,
@ -472,7 +474,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 "
@ -481,15 +483,14 @@ 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;
VIR_STEAL_PTR(def->dom, otherdef->dom);
}
}
}
@ -914,7 +915,9 @@ virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def)
bool
virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap)
{
return virDomainSnapshotDefIsExternal(snap->def);
virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snap);
return virDomainSnapshotDefIsExternal(def);
}
int
@ -928,6 +931,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
{
virDomainSnapshotDefPtr def = *defptr;
virDomainSnapshotObjPtr other;
virDomainSnapshotDefPtr otherdef;
bool check_if_stolen;
/* Prevent circular chains */
@ -945,15 +949,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);
@ -963,14 +968,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;
def->dom = NULL;
}
if (check_if_stolen && def->dom && !otherdef->dom)
VIR_STEAL_PTR(otherdef->dom, def->dom);
return -1;
}
if (other) {
@ -982,9 +986,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;
*defptr = NULL;
virDomainSnapshotDefFree(otherdef);
VIR_STEAL_PTR(other->def, *defptr);
*snap = other;
}

View File

@ -168,8 +168,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);
}
@ -229,7 +230,7 @@ static void virDomainSnapshotObjFree(virDomainSnapshotObjPtr snapshot)
VIR_DEBUG("obj=%p", snapshot);
virDomainSnapshotDefFree(snapshot->def);
virDomainSnapshotDefFree(virDomainSnapshotObjGetDef(snapshot));
VIR_FREE(snapshot);
}
@ -315,15 +316,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;
}

View File

@ -76,4 +76,11 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots,
virDomainSnapshotPtr **snaps,
unsigned int flags);
/* Access the snapshot-specific definition from a given list member. */
static inline virDomainSnapshotDefPtr
virDomainSnapshotObjGetDef(virDomainSnapshotObjPtr obj)
{
return obj->def;
}
#endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJLIST_H */

View File

@ -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);