[libvirt] [PATCH 5/8] snapshot: Factor out redefine cycle validation

The code to check whether a redefined snapshot/checkpoint XML is attempting to create a cycle in the list of moments is lengthy, and common between the two types of list. Therefore, it belongs in the shared base file. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/virdomainmomentobjlist.h | 3 +++ src/conf/virdomainsnapshotobjlist.h | 3 +++ src/conf/snapshot_conf.c | 41 ++++------------------------- src/conf/virdomainmomentobjlist.c | 40 ++++++++++++++++++++++++++++ src/conf/virdomainsnapshotobjlist.c | 9 +++++++ tests/virsh-snapshot | 2 +- 6 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/conf/virdomainmomentobjlist.h b/src/conf/virdomainmomentobjlist.h index 68f00a114f..4067e928f4 100644 --- a/src/conf/virdomainmomentobjlist.h +++ b/src/conf/virdomainmomentobjlist.h @@ -117,3 +117,6 @@ int virDomainMomentForEach(virDomainMomentObjListPtr moments, virHashIterator iter, void *data); int virDomainMomentUpdateRelations(virDomainMomentObjListPtr moments); +int virDomainMomentCheckCycles(virDomainMomentObjListPtr list, + virDomainMomentDefPtr def, + const char *domname); diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h index 26fa456730..fed8d22bc8 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -52,6 +52,9 @@ int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, void *data); int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots); +int virDomainSnapshotCheckCycles(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotDefPtr def, + const char *domname); #define VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA \ (VIR_DOMAIN_SNAPSHOT_LIST_METADATA | \ diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index daea6c616d..3521177f0b 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -966,46 +966,15 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, { virDomainSnapshotDefPtr def = *defptr; virDomainMomentObjPtr other; - virDomainSnapshotDefPtr otherdef; + virDomainSnapshotDefPtr otherdef = NULL; bool check_if_stolen; - /* Prevent circular chains */ - if (def->parent.parent_name) { - if (STREQ(def->parent.name, def->parent.parent_name)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot set snapshot %s as its own parent"), - def->parent.name); - return -1; - } - other = virDomainSnapshotFindByName(vm->snapshots, - def->parent.parent_name); - if (!other) { - virReportError(VIR_ERR_INVALID_ARG, - _("parent %s for snapshot %s not found"), - def->parent.parent_name, def->parent.name); - return -1; - } - otherdef = virDomainSnapshotObjGetDef(other); - while (otherdef->parent.parent_name) { - if (STREQ(otherdef->parent.parent_name, def->parent.name)) { - virReportError(VIR_ERR_INVALID_ARG, - _("parent %s would create cycle to %s"), - otherdef->parent.name, def->parent.name); - return -1; - } - other = virDomainSnapshotFindByName(vm->snapshots, - otherdef->parent.parent_name); - if (!other) { - VIR_WARN("snapshots are inconsistent for %s", - vm->def->name); - break; - } - otherdef = virDomainSnapshotObjGetDef(other); - } - } + if (virDomainSnapshotCheckCycles(vm->snapshots, def, vm->def->name) < 0) + return -1; other = virDomainSnapshotFindByName(vm->snapshots, def->parent.name); - otherdef = other ? virDomainSnapshotObjGetDef(other) : NULL; + if (other) + otherdef = virDomainSnapshotObjGetDef(other); check_if_stolen = other && otherdef->parent.dom; if (virDomainSnapshotRedefineValidate(def, domain->uuid, other, xmlopt, flags) < 0) { diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index f56b516343..0ea5e9af80 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -519,3 +519,43 @@ virDomainMomentUpdateRelations(virDomainMomentObjListPtr moments) moments->current = NULL; return act.err; } + + +int +virDomainMomentCheckCycles(virDomainMomentObjListPtr list, + virDomainMomentDefPtr def, + const char *domname) +{ + virDomainMomentObjPtr other; + + if (def->parent_name) { + if (STREQ(def->name, def->parent_name)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot set moment %s as its own parent"), + def->name); + return -1; + } + other = virDomainMomentFindByName(list, def->parent_name); + if (!other) { + virReportError(VIR_ERR_INVALID_ARG, + _("parent %s for moment %s not found"), + def->parent_name, def->name); + return -1; + } + while (other->def->parent_name) { + if (STREQ(other->def->parent_name, def->name)) { + virReportError(VIR_ERR_INVALID_ARG, + _("parent %s would create cycle to %s"), + other->def->name, def->name); + return -1; + } + other = virDomainMomentFindByName(list, other->def->parent_name); + if (!other) { + VIR_WARN("moments are inconsistent for domain %s", + domname); + break; + } + } + } + return 0; +} diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index 9bcc8d1036..99bc4bb0c5 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -234,6 +234,15 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots) } +int +virDomainSnapshotCheckCycles(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotDefPtr def, + const char *domname) +{ + return virDomainMomentCheckCycles(snapshots->base, &def->parent, domname); +} + + int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots, virDomainMomentObjPtr from, diff --git a/tests/virsh-snapshot b/tests/virsh-snapshot index 8eab67c9e0..510904f470 100755 --- a/tests/virsh-snapshot +++ b/tests/virsh-snapshot @@ -206,7 +206,7 @@ Metadata: yes EOF cat <<EOF > exp || fail=1 -error: invalid argument: parent s3 for snapshot s2 not found +error: invalid argument: parent s3 for moment s2 not found error: marker EOF compare exp err || fail=1 -- 2.20.1