On Tue, Jun 18, 2019 at 22:47:46 -0500, Eric Blake wrote:
Create a new file for managing a list of checkpoint objects, borrowing heavily from existing virDomainSnapshotObjList paradigms.
Note that while checkpoints definitely have a use case for multiple children to a single parent (create a base snapshot, create a child snapshot, revert to the base, then create another child snapshot), it's harder to predict how checkpoints will play out with reverting to prior points in time. Thus, in initial use, we may find that in a list of checkpoints, you never have more than one child. However, as the snapshot machinery is already generic, it is easier to reuse the generic machinery that tracks relations between domain moments than it is to open-code a new list-management scheme just for checkpoints.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/checkpoint_conf.h | 7 + src/conf/domain_conf.h | 2 + src/conf/virconftypes.h | 6 + src/conf/virdomaincheckpointobjlist.h | 66 ++++++++ src/conf/virdomainobjlist.h | 7 +- src/conf/Makefile.inc.am | 2 + src/conf/checkpoint_conf.c | 87 ++++++++++ src/conf/domain_conf.c | 6 + src/conf/virdomaincheckpointobjlist.c | 219 ++++++++++++++++++++++++++ src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainobjlist.c | 11 ++ src/libvirt_private.syms | 18 +++ 12 files changed, 431 insertions(+), 2 deletions(-) create mode 100644 src/conf/virdomaincheckpointobjlist.h create mode 100644 src/conf/virdomaincheckpointobjlist.c
[...]
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index a7a34d3887..469f665648 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c
[...]
@@ -573,3 +574,89 @@ virDomainCheckpointDefFormat(virDomainCheckpointDefPtr def,
[...]
+int +virDomainCheckpointRedefinePrep(virDomainPtr domain, + virDomainObjPtr vm, + virDomainCheckpointDefPtr *defptr, + virDomainMomentObjPtr *chk, + virDomainXMLOptionPtr xmlopt, + bool *update_current)
[...]
+ + if (!def->parent.dom || + memcmp(def->parent.dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) { + virReportError(VIR_ERR_INVALID_ARG, + _("definition for checkpoint %s must use uuid %s"), + def->parent.name, uuidstr); + return -1; + } + if (virDomainCheckpointAlignDisks(def) < 0) + return -1; + + other = virDomainCheckpointFindByName(vm->checkpoints, def->parent.name); + otherdef = other ? virDomainCheckpointObjGetDef(other) : NULL;
Soo ... here you decide based on 'other'
+ if (other) {
... and here too. How about moving that here to avoid the ternary?
+ if (!virDomainDefCheckABIStability(otherdef->parent.dom, + def->parent.dom, xmlopt)) + return -1; + + if (other == virDomainCheckpointGetCurrent(vm->checkpoints)) { + *update_current = true; + virDomainCheckpointSetCurrent(vm->checkpoints, NULL); + }