mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-08-28 11:31:16 +00:00
snapshot: Don't leak moment obj list metaroot to callers
virDomainSnapshotFindByName(list, NULL) should return NULL, rather than the internal-use-only metaroot. Most existing callers pass in a non-NULL name; the few external callers that don't are immediately calling virDomainMomentSetParent (which indeed needs the metaroot rather than NULL if the parent name is NULL); but as the leaky abstraction is ugly, it is worth instead making virDomainMomentSetParent static and adding a new function for resolving the parent link of a brand new moment within its list. The existing external uses of virDomainMomentSetParent always succeed (either the new moment has parent_name of NULL to become a new root, or has parent_name set to a strdup of the previous current moment); hence, our new function does not need a return value (but it still has a VIR_WARN in case future uses break our assumptions about failure being impossible). Missed when commit02c4e24d
refactored things to attempt to remove direct metaroot manipulations out of the qemu and test drivers into internal-only details, and made more obvious when commitdc8d3dc6
factored it out into a separate file. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
parent
d5572f62e3
commit
ceb1019257
@ -151,7 +151,7 @@ virDomainMomentDropChildren(virDomainMomentObjPtr moment)
|
|||||||
|
|
||||||
|
|
||||||
/* Add @moment to @parent's list of children. */
|
/* Add @moment to @parent's list of children. */
|
||||||
void
|
static void
|
||||||
virDomainMomentSetParent(virDomainMomentObjPtr moment,
|
virDomainMomentSetParent(virDomainMomentObjPtr moment,
|
||||||
virDomainMomentObjPtr parent)
|
virDomainMomentObjPtr parent)
|
||||||
{
|
{
|
||||||
@ -162,6 +162,27 @@ virDomainMomentSetParent(virDomainMomentObjPtr moment,
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/* Add @moment to the appropriate parent's list of children. The
|
||||||
|
* caller must ensure that moment->def->parent_name is either NULL
|
||||||
|
* (for a new root) or set to an existing moment already in the
|
||||||
|
* list. */
|
||||||
|
void
|
||||||
|
virDomainMomentLinkParent(virDomainMomentObjListPtr moments,
|
||||||
|
virDomainMomentObjPtr moment)
|
||||||
|
{
|
||||||
|
virDomainMomentObjPtr parent;
|
||||||
|
|
||||||
|
parent = virDomainMomentFindByName(moments, moment->def->parent_name);
|
||||||
|
if (!parent) {
|
||||||
|
parent = &moments->metaroot;
|
||||||
|
if (moment->def->parent_name)
|
||||||
|
VIR_WARN("moment %s lacks parent %s", moment->def->name,
|
||||||
|
moment->def->parent_name);
|
||||||
|
}
|
||||||
|
virDomainMomentSetParent(moment, parent);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* Take all children of @from and convert them into children of @to. */
|
/* Take all children of @from and convert them into children of @to. */
|
||||||
void
|
void
|
||||||
virDomainMomentMoveChildren(virDomainMomentObjPtr from,
|
virDomainMomentMoveChildren(virDomainMomentObjPtr from,
|
||||||
@ -390,7 +411,9 @@ virDomainMomentObjPtr
|
|||||||
virDomainMomentFindByName(virDomainMomentObjListPtr moments,
|
virDomainMomentFindByName(virDomainMomentObjListPtr moments,
|
||||||
const char *name)
|
const char *name)
|
||||||
{
|
{
|
||||||
return name ? virHashLookup(moments->objs, name) : &moments->metaroot;
|
if (name)
|
||||||
|
return virHashLookup(moments->objs, name);
|
||||||
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@ -484,9 +507,12 @@ virDomainMomentSetRelations(void *payload,
|
|||||||
|
|
||||||
parent = virDomainMomentFindByName(curr->moments, obj->def->parent_name);
|
parent = virDomainMomentFindByName(curr->moments, obj->def->parent_name);
|
||||||
if (!parent) {
|
if (!parent) {
|
||||||
curr->err = -1;
|
|
||||||
parent = &curr->moments->metaroot;
|
parent = &curr->moments->metaroot;
|
||||||
VIR_WARN("moment %s lacks parent", obj->def->name);
|
if (obj->def->parent_name) {
|
||||||
|
curr->err = -1;
|
||||||
|
VIR_WARN("moment %s lacks parent %s", obj->def->name,
|
||||||
|
obj->def->parent_name);
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
tmp = parent;
|
tmp = parent;
|
||||||
while (tmp && tmp->def) {
|
while (tmp && tmp->def) {
|
||||||
|
@ -60,8 +60,8 @@ void virDomainMomentDropParent(virDomainMomentObjPtr moment);
|
|||||||
void virDomainMomentDropChildren(virDomainMomentObjPtr moment);
|
void virDomainMomentDropChildren(virDomainMomentObjPtr moment);
|
||||||
void virDomainMomentMoveChildren(virDomainMomentObjPtr from,
|
void virDomainMomentMoveChildren(virDomainMomentObjPtr from,
|
||||||
virDomainMomentObjPtr to);
|
virDomainMomentObjPtr to);
|
||||||
void virDomainMomentSetParent(virDomainMomentObjPtr moment,
|
void virDomainMomentLinkParent(virDomainMomentObjListPtr moments,
|
||||||
virDomainMomentObjPtr parent);
|
virDomainMomentObjPtr moment);
|
||||||
|
|
||||||
virDomainMomentObjListPtr virDomainMomentObjListNew(void);
|
virDomainMomentObjListPtr virDomainMomentObjListNew(void);
|
||||||
void virDomainMomentObjListFree(virDomainMomentObjListPtr moments);
|
void virDomainMomentObjListFree(virDomainMomentObjListPtr moments);
|
||||||
|
@ -223,6 +223,15 @@ virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/* Populate parent link of a given snapshot. */
|
||||||
|
void
|
||||||
|
virDomainSnapshotLinkParent(virDomainSnapshotObjListPtr snapshots,
|
||||||
|
virDomainMomentObjPtr snap)
|
||||||
|
{
|
||||||
|
return virDomainMomentLinkParent(snapshots->base, snap);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* Populate parent link and child count of all snapshots, with all
|
/* Populate parent link and child count of all snapshots, with all
|
||||||
* assigned defs having relations starting as 0/NULL. Return 0 on
|
* assigned defs having relations starting as 0/NULL. Return 0 on
|
||||||
* success, -1 if a parent is missing or if a circular relationship
|
* success, -1 if a parent is missing or if a circular relationship
|
||||||
|
@ -51,6 +51,8 @@ void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots);
|
|||||||
int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
|
int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
|
||||||
virHashIterator iter,
|
virHashIterator iter,
|
||||||
void *data);
|
void *data);
|
||||||
|
void virDomainSnapshotLinkParent(virDomainSnapshotObjListPtr snapshots,
|
||||||
|
virDomainMomentObjPtr moment);
|
||||||
int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots);
|
int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots);
|
||||||
int virDomainSnapshotCheckCycles(virDomainSnapshotObjListPtr snapshots,
|
int virDomainSnapshotCheckCycles(virDomainSnapshotObjListPtr snapshots,
|
||||||
virDomainSnapshotDefPtr def,
|
virDomainSnapshotDefPtr def,
|
||||||
|
@ -978,7 +978,6 @@ virDomainMomentDropParent;
|
|||||||
virDomainMomentForEachChild;
|
virDomainMomentForEachChild;
|
||||||
virDomainMomentForEachDescendant;
|
virDomainMomentForEachDescendant;
|
||||||
virDomainMomentMoveChildren;
|
virDomainMomentMoveChildren;
|
||||||
virDomainMomentSetParent;
|
|
||||||
|
|
||||||
|
|
||||||
# conf/virdomainobjlist.h
|
# conf/virdomainobjlist.h
|
||||||
@ -1007,6 +1006,7 @@ virDomainSnapshotFindByName;
|
|||||||
virDomainSnapshotForEach;
|
virDomainSnapshotForEach;
|
||||||
virDomainSnapshotGetCurrent;
|
virDomainSnapshotGetCurrent;
|
||||||
virDomainSnapshotGetCurrentName;
|
virDomainSnapshotGetCurrentName;
|
||||||
|
virDomainSnapshotLinkParent;
|
||||||
virDomainSnapshotObjListFree;
|
virDomainSnapshotObjListFree;
|
||||||
virDomainSnapshotObjListGetNames;
|
virDomainSnapshotObjListGetNames;
|
||||||
virDomainSnapshotObjListNew;
|
virDomainSnapshotObjListNew;
|
||||||
|
@ -15540,7 +15540,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
|
|||||||
bool update_current = true;
|
bool update_current = true;
|
||||||
bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
|
bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
|
||||||
unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
|
unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
|
||||||
virDomainMomentObjPtr other = NULL;
|
|
||||||
int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
|
int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
|
||||||
bool align_match = true;
|
bool align_match = true;
|
||||||
virQEMUDriverConfigPtr cfg = NULL;
|
virQEMUDriverConfigPtr cfg = NULL;
|
||||||
@ -15807,9 +15806,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
|
|||||||
snap->def->name);
|
snap->def->name);
|
||||||
virDomainSnapshotObjListRemove(vm->snapshots, snap);
|
virDomainSnapshotObjListRemove(vm->snapshots, snap);
|
||||||
} else {
|
} else {
|
||||||
other = virDomainSnapshotFindByName(vm->snapshots,
|
virDomainSnapshotLinkParent(vm->snapshots, snap);
|
||||||
snap->def->parent_name);
|
|
||||||
virDomainMomentSetParent(snap, other);
|
|
||||||
}
|
}
|
||||||
} else if (snap) {
|
} else if (snap) {
|
||||||
virDomainSnapshotObjListRemove(vm->snapshots, snap);
|
virDomainSnapshotObjListRemove(vm->snapshots, snap);
|
||||||
|
@ -7663,12 +7663,9 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
|
|||||||
cleanup:
|
cleanup:
|
||||||
if (vm) {
|
if (vm) {
|
||||||
if (snapshot) {
|
if (snapshot) {
|
||||||
virDomainMomentObjPtr other;
|
|
||||||
if (update_current)
|
if (update_current)
|
||||||
virDomainSnapshotSetCurrent(vm->snapshots, snap);
|
virDomainSnapshotSetCurrent(vm->snapshots, snap);
|
||||||
other = virDomainSnapshotFindByName(vm->snapshots,
|
virDomainSnapshotLinkParent(vm->snapshots, snap);
|
||||||
snap->def->parent_name);
|
|
||||||
virDomainMomentSetParent(snap, other);
|
|
||||||
}
|
}
|
||||||
virDomainObjEndAPI(&vm);
|
virDomainObjEndAPI(&vm);
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user