The following virsh command was triggering a use-after-free: $ virsh -c test:///default ' snapshot-create-as test s1 snapshot-create-as test s2 snapshot-delete --children-only test s1 snapshot-current --name test' Domain snapshot s1 created Domain snapshot s2 created Domain snapshot s1 children deleted error: name in virGetDomainSnapshot must not be NULL I got lucky on that run - although the error message is quite unexpected. On other runs, I was able to get a core dump, and valgrind confirms there is a definitive problem. The culprit? We were inconsistent about whether we set vm->current_snapshot, snap->def->current, or both when updating how the current snapshot was being tracked. As a result, deletion did not see that snapshot s2 was previously current, and failed to update vm->current_snapshot, so that the next API using the current snapshot failed because it referenced stale memory for the now-gone s2 (instead of the intended s1). The test driver code was copied from the qemu code (which DOES track both pieces of state everywhere), but was purposefully simplified because the test driver does not have to write persistent snapshot state to the file system. But when you realize that the only reason snap->def->current needs to exist is when writing out one file per snapshot for qemu, it's just as easy to state that the test driver never has to mess with the field (rather than chasing down which places forgot to set the field), and have vm->current_snapshot be the sole source of truth in the test driver. Ideally, I'd get rid of the 'current' member in virDomainSnapshotDef, as well as the 'current_snapshot' member in virDomainDef, and instead track the current member in virDomainSnapshotObjList, coupled with writing ALL snapshot state for qemu in a single file (where I can use <snapshots current='...'> as a wrapper, rather than VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL to output <current>1</current> XML on a per-snapshot file basis). But that's a bigger change, so for now I'm just patching things to avoid the test driver segfault. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/test/test_driver.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0bb48b4364..bd0a14114e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1,7 +1,7 @@ /* * test_driver.c: A "mock" hypervisor for use by application unit tests * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -6441,7 +6441,7 @@ testDomainSnapshotDiscardAll(void *payload, virDomainSnapshotObjPtr snap = payload; testSnapRemoveDataPtr curr = data; - if (snap->def->current) + if (curr->vm->current_snapshot == snap) curr->current = true; virDomainSnapshotObjListRemove(curr->vm->snapshots, snap); return 0; @@ -6507,11 +6507,8 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virDomainSnapshotForEachDescendant(snap, testDomainSnapshotDiscardAll, &rem); - if (rem.current) { - if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) - snap->def->current = true; + if (rem.current) vm->current_snapshot = snap; - } } else if (snap->nchildren) { testSnapReparentData rep; rep.parent = snap->parent; @@ -6539,12 +6536,9 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, if (snap->def->parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); - if (!parentsnap) { + if (!parentsnap) VIR_WARN("missing parent snapshot matching name '%s'", snap->def->parent); - } else { - parentsnap->def->current = true; - } } vm->current_snapshot = parentsnap; } @@ -6623,12 +6617,9 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } - if (vm->current_snapshot) { - vm->current_snapshot->def->current = false; + if (vm->current_snapshot) vm->current_snapshot = NULL; - } - snap->def->current = true; config = virDomainDefCopy(snap->def->dom, privconn->caps, privconn->xmlopt, NULL, true); if (!config) -- 2.20.1

Upcoming patches want to add virDomainCheckpoint that behaves very similarly to virDomainSnapshot; the easiest way to share common code is to give both classes a common base class. Thanks to the accessor functions in the previous patch, we have very few changes required outside of datatypes.[ch]. The subclass does have to use a dummy field, though, to satisfy virobject's insistence on size differentiation for type safety. Note that virClassNew() supports a NULL dispose method for a class that has nothing to clean up, but VIR_CLASS_NEW has no easy way to register such a class without a #define hack. I promised my teenage daughter Evelyn that I'd give her credit for her contribution to this commit. I asked her "What would be a good name for a base class for DomainSnapshot and DomainCheckpoint". After explaining what a base class was (using the classic OOB Square and Circle inherit from Shape), she came up with "DomainMoment", which is way better than my initial thought of "DomainPointInTime" or "DomainPIT". Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/datatypes.h | 26 ++++++-- src/datatypes.c | 110 ++++++++++++++++++++-------------- src/libvirt-domain-snapshot.c | 2 +- 3 files changed, 88 insertions(+), 50 deletions(-) diff --git a/src/datatypes.h b/src/datatypes.h index a66dfbe983..70d947657b 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -31,6 +31,7 @@ extern virClassPtr virConnectClass; extern virClassPtr virDomainClass; +extern virClassPtr virDomainMomentClass; extern virClassPtr virDomainSnapshotClass; extern virClassPtr virInterfaceClass; extern virClassPtr virNetworkClass; @@ -668,27 +669,42 @@ struct _virStream { virFreeCallback ff; }; +/** + * _virDomainMoment + * + * Internal abstract structure serving as a base class to a named + * point in time object related to a domain + */ +typedef struct _virDomainMoment virDomainMoment; +typedef virDomainMoment *virDomainMomentPtr; +struct _virDomainMoment { + virObject parent; + char *name; + virDomainPtr domain; +}; + /** * _virDomainSnapshot * * Internal structure associated with a domain snapshot */ struct _virDomainSnapshot { - virObject parent; - char *name; - virDomainPtr domain; + virDomainMoment parent; + + /* Unused attribute to allow for subclass creation */ + bool dummy; }; static inline char * virSnapName(virDomainSnapshotPtr snapshot) { - return snapshot->name; + return snapshot->parent.name; } static inline virDomainPtr virSnapDom(virDomainSnapshotPtr snapshot) { - return snapshot->domain; + return snapshot->parent.domain; } /** diff --git a/src/datatypes.c b/src/datatypes.c index 9b92d892d5..f0cfbe11fc 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -1,7 +1,7 @@ /* * datatypes.c: management of structs for public data types * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -36,6 +36,7 @@ VIR_LOG_INIT("datatypes"); virClassPtr virConnectClass; virClassPtr virConnectCloseCallbackDataClass; virClassPtr virDomainClass; +virClassPtr virDomainMomentClass; virClassPtr virDomainSnapshotClass; virClassPtr virInterfaceClass; virClassPtr virNetworkClass; @@ -50,7 +51,8 @@ virClassPtr virStoragePoolClass; static void virConnectDispose(void *obj); static void virConnectCloseCallbackDataDispose(void *obj); static void virDomainDispose(void *obj); -static void virDomainSnapshotDispose(void *obj); +static void virDomainMomentDispose(void *obj); +#define virDomainSnapshotDispose NULL static void virInterfaceDispose(void *obj); static void virNetworkDispose(void *obj); static void virNodeDeviceDispose(void *obj); @@ -86,7 +88,8 @@ virDataTypesOnceInit(void) DECLARE_CLASS_LOCKABLE(virConnect); DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData); DECLARE_CLASS(virDomain); - DECLARE_CLASS(virDomainSnapshot); + DECLARE_CLASS(virDomainMoment); + DECLARE_CLASS_COMMON(virDomainSnapshot, virDomainMomentClass); DECLARE_CLASS(virInterface); DECLARE_CLASS(virNetwork); DECLARE_CLASS(virNodeDevice); @@ -900,6 +903,64 @@ virNWFilterBindingDispose(void *obj) } +/** + * virGetDomainMoment: + * @domain: the domain involved in a point-in-time moment + * @name: pointer to the domain moment name + * + * Allocates a new concrete subclass of a domain moment object. When + * the object is no longer needed, virObjectUnref() must be called in + * order to not leak data. + * + * Returns a pointer to the domain moment object, or NULL on error. + */ +static virDomainMomentPtr +virGetDomainMoment(virDomainPtr domain, const char *name, virClassPtr subclass) +{ + virDomainMomentPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + return NULL; + + virCheckDomainGoto(domain, error); + virCheckNonNullArgGoto(name, error); + + if (!(ret = virObjectNew(subclass))) + goto error; + if (VIR_STRDUP(ret->name, name) < 0) + goto error; + + ret->domain = virObjectRef(domain); + + return ret; + + error: + virObjectUnref(ret); + return NULL; +} + + +/** + * virDomainMomentDispose: + * @obj: the domain moment to release + * + * Unconditionally release all memory associated with a moment. + * The object must not be used once this method returns. + * + * It will also unreference the associated connection object, + * which may also be released if its ref count hits zero. + */ +static void +virDomainMomentDispose(void *obj) +{ + virDomainMomentPtr moment = obj; + VIR_DEBUG("release moment %p %s", moment, moment->name); + + VIR_FREE(moment->name); + virObjectUnref(moment->domain); +} + + /** * virGetDomainSnapshot: * @domain: the domain to snapshot @@ -913,47 +974,8 @@ virNWFilterBindingDispose(void *obj) virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, const char *name) { - virDomainSnapshotPtr ret = NULL; - - if (virDataTypesInitialize() < 0) - return NULL; - - virCheckDomainGoto(domain, error); - virCheckNonNullArgGoto(name, error); - - if (!(ret = virObjectNew(virDomainSnapshotClass))) - goto error; - if (VIR_STRDUP(ret->name, name) < 0) - goto error; - - ret->domain = virObjectRef(domain); - - return ret; - - error: - virObjectUnref(ret); - return NULL; -} - - -/** - * virDomainSnapshotDispose: - * @obj: the domain snapshot to release - * - * Unconditionally release all memory associated with a snapshot. - * The snapshot object must not be used once this method returns. - * - * It will also unreference the associated connection object, - * which may also be released if its ref count hits zero. - */ -static void -virDomainSnapshotDispose(void *obj) -{ - virDomainSnapshotPtr snapshot = obj; - VIR_DEBUG("release snapshot %p %s", snapshot, snapshot->name); - - VIR_FREE(snapshot->name); - virObjectUnref(snapshot->domain); + return (virDomainSnapshotPtr) virGetDomainMoment(domain, name, + virDomainSnapshotClass); } diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c index e1275c69b0..27fb350cc6 100644 --- a/src/libvirt-domain-snapshot.c +++ b/src/libvirt-domain-snapshot.c @@ -1206,7 +1206,7 @@ int virDomainSnapshotRef(virDomainSnapshotPtr snapshot) { VIR_DEBUG("snapshot=%p, refs=%d", snapshot, - snapshot ? snapshot->parent.u.s.refs : 0); + snapshot ? snapshot->parent.parent.u.s.refs : 0); virResetLastError(); -- 2.20.1

On 3/20/19 1:40 AM, Eric Blake wrote: [...] Even though we've restricted usage via @flags where PARSE_INTERNAL is set, should this be prefaced by: if (current) guess I'm concerned with some future cut-copy-paste error... Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John The setting of ->current_snapshot in/around calls to qemuDomainSnapshotWriteMetadata was particularly "interesting" to follow, but looks fine. [...]

On 3/20/19 1:40 AM, Eric Blake wrote: [...] [...] NIT: One of these things is not like the others ;-) bool virDomain... Slick, this is how testDomainSnapshotDiscardAll can alter it's logic. Took me until the end of the patch to find ;-)... and coverity didn't whine about one function checking return while the others don't. [...] Even though we generate this message we go ahead and update @current to @snap. Should this be an "if (current) ... else ... " ? Additionally if someone's really AFU'd, they could get more than one message; whereas, previously they'd only get one such message. Previously if this happened, then current was set to NULL, but the following will set it to the last snap declared to be current. Is that expected? If not, then perhaps the if (current) above needs to add a "current = NULL;" along with the error message. Of course that leads to the possibility of others declaring themselves current and possibly having multiple errors splatted. Only seems to matter for someone running debug or looking at debug logs since we don't fail. BTW: This is one of those current gray areas of making two changes in one patch. One change being the usage of the accessors and the other being the alteration of when this message gets splatted. [...] virDomainSnapshotSetCurrent(vm->snapshots, NULL); right? [...] virDomainSnapshotSetCurrent(vm->snapshots, snap); ? [...] The comments in qemuDomainSnapshotLoad aren't showstoppers. I assume you can answer and things will be fine. Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John

On 3/20/19 4:03 PM, Eric Blake wrote: I'm posting what I split out for the logic change (where I had fun writing the commit message); and I'm going to be bold and apply your R-b to both halves of the split rather than make you re-read it in a v2. From 1a54a57381704f09491c80a0ef54b43b7b582bb7 Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake(a)redhat.com&gt; Date: Thu, 21 Mar 2019 12:16:03 -0500 Subject: [PATCH] snapshot: Rework parse logic during libvirt startup Rework the logic in qemuDomainSnapshotLoad() to set vm->current_snapshot only once at the end of the loop, rather than repeatedly querying it during the loop, to make it easier for the next patch to use accessor functions rather than direct manipulation of vm->current_snapshot. When encountering multiple snapshots claiming to be current (based on the presence of an <active>1</active> element in the XML, which libvirt only outputs for internal use and not for any public API), this changes behavior from warning only once and running with no current snapshot, to instead warning on each duplicate and selecting the last one encountered (which is arbitrary based on readdir() ordering, but actually stands a fair chance of being the most-recently created snapshot whether by timestamp or by the propensity of humans to name things in ascending order). Note that the code in question is only run by libvirtd when it first starts, reading state from disk from the previous run into memory for this run. Since the data resides somewhere that only libvirt should be touching (typically /var/lib/libvirt/qemu/snapshot/*), it should be clean. So in the common case, the code touched here is unreachable. But if someone is actually messing with files behind libvirt's back, they deserve the change in behavior. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 13a34ef860..e49924ce1a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1,7 +1,7 @@ /* * qemu_driver.c: core driver methods for managing qemu guests * - * Copyright (C) 2006-2016 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -482,9 +482,11 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, if (snap == NULL) { virDomainSnapshotDefFree(def); } else if (cur) { + if (current) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many snapshots claiming to be current for domain %s"), + vm->def->name); current = snap; - if (!vm->current_snapshot) - vm->current_snapshot = snap; } VIR_FREE(fullpath); @@ -495,13 +497,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, _("Failed to fully read directory %s"), snapDir); - if (vm->current_snapshot != current) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Too many snapshots claiming to be current for domain %s"), - vm->def->name); - vm->current_snapshot = NULL; - } - + vm->current_snapshot = current; if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0) virReportError(VIR_ERR_INTERNAL_ERROR, _("Snapshots have inconsistent relations for domain %s"), -- 2.20.1 -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Rather than allowing a leaky abstraction where multiple drivers have to open-code operations that update the relations in a virDomainSnapshotObjList, it is better to add accessor functions so that updates to relations are maintained closer to the internals. The goal here is to avoid access to, nchildren, first_child, or sibling outside of the lowest level functions, making it easier to refactor later on. While many of the conversions are fairly straightforward, the MoveChildren refactoring can be a bit interesting to follow. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/virdomainsnapshotobj.h | 5 ++++ src/conf/virdomainsnapshotobjlist.h | 2 ++ src/conf/virdomainsnapshotobj.c | 42 +++++++++++++++++++++++++++++ src/conf/virdomainsnapshotobjlist.c | 42 ++++++++++++++++++----------- src/libvirt_private.syms | 3 +++ src/qemu/qemu_driver.c | 19 +++---------- src/test/test_driver.c | 18 +++---------- 7 files changed, 85 insertions(+), 46 deletions(-) diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainsnapshotobj.h index 957f1b2ea8..0981ea4c25 100644 --- a/src/conf/virdomainsnapshotobj.h +++ b/src/conf/virdomainsnapshotobj.h @@ -46,5 +46,10 @@ int virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot, virHashIterator iter, void *data); void virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot); +void virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot); +void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, + virDomainSnapshotObjPtr to); +void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjPtr parent); #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */ diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h index e210849441..c13a0b4026 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -55,6 +55,7 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, unsigned int flags); virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, const char *name); +int virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots); virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots); const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots); bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, @@ -63,6 +64,7 @@ void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot); bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot); +void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots); int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, void *data); diff --git a/src/conf/virdomainsnapshotobj.c b/src/conf/virdomainsnapshotobj.c index 7f92ac21d9..d6b216c7b2 100644 --- a/src/conf/virdomainsnapshotobj.c +++ b/src/conf/virdomainsnapshotobj.c @@ -121,3 +121,45 @@ virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot) snapshot->parent = NULL; snapshot->sibling = NULL; } + + +/* Update @snapshot to no longer have children. */ +void +virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot) +{ + snapshot->nchildren = 0; + snapshot->first_child = NULL; +} + + +/* Add @snapshot to @parent's list of children. */ +void +virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjPtr parent) +{ + snapshot->parent = parent; + parent->nchildren++; + snapshot->sibling = parent->first_child; + parent->first_child = snapshot; +} + + +/* Take all children of @from and convert them into children of @to. */ +void +virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, + virDomainSnapshotObjPtr to) +{ + virDomainSnapshotObjPtr child; + virDomainSnapshotObjPtr last; + + for (child = from->first_child; child; child = child->sibling) { + child->parent = to; + if (!child->sibling) + last = child; + } + to->nchildren += from->nchildren; + last->sibling = to->first_child; + to->first_child = from->first_child; + from->nchildren = 0; + from->first_child = NULL; +} diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index 1eecb89a5d..9538521ab3 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -72,7 +72,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, _("incorrect flags for bulk parse")); return -1; } - if (snapshots->metaroot.nchildren || snapshots->current) { + if (virDomainSnapshotObjListSize(snapshots)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("bulk define of snapshots only possible with " "no existing snapshot")); @@ -143,9 +143,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, if (ret < 0) { /* There were no snapshots before this call; so on error, just * blindly delete anything created before the failure. */ - virHashRemoveAll(snapshots->objs); - snapshots->metaroot.nchildren = 0; - snapshots->metaroot.first_child = NULL; + virDomainSnapshotObjListRemoveAll(snapshots); } xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); @@ -437,6 +435,14 @@ virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, } +/* Return the number of objects currently in the list */ +int +virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots) +{ + return virHashSize(snapshots->objs); +} + + /* Return the current snapshot, or NULL */ virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots) @@ -484,6 +490,15 @@ bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, return ret; } +/* Remove all snapshots tracked in the list */ +void +virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots) +{ + virHashRemoveAll(snapshots->objs); + virDomainSnapshotDropChildren(&snapshots->metaroot); +} + + int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, @@ -511,28 +526,26 @@ virDomainSnapshotSetRelations(void *payload, virDomainSnapshotObjPtr obj = payload; struct snapshot_set_relation *curr = data; virDomainSnapshotObjPtr tmp; + virDomainSnapshotObjPtr parent; - obj->parent = virDomainSnapshotFindByName(curr->snapshots, - obj->def->parent); - if (!obj->parent) { + parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->parent); + if (!parent) { curr->err = -1; - obj->parent = &curr->snapshots->metaroot; + parent = &curr->snapshots->metaroot; VIR_WARN("snapshot %s lacks parent", obj->def->name); } else { - tmp = obj->parent; + tmp = parent; while (tmp && tmp->def) { if (tmp == obj) { curr->err = -1; - obj->parent = &curr->snapshots->metaroot; + parent = &curr->snapshots->metaroot; VIR_WARN("snapshot %s in circular chain", obj->def->name); break; } tmp = tmp->parent; } } - obj->parent->nchildren++; - obj->sibling = obj->parent->first_child; - obj->parent->first_child = obj; + virDomainSnapshotSetParent(obj, parent); return 0; } @@ -545,8 +558,7 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots) { struct snapshot_set_relation act = { snapshots, 0 }; - snapshots->metaroot.nchildren = 0; - snapshots->metaroot.first_child = NULL; + virDomainSnapshotDropChildren(&snapshots->metaroot); virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act); if (act.err) snapshots->current = NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 72c5cef528..ffc1724850 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -980,9 +980,12 @@ virDomainObjListRename; # conf/virdomainsnapshotobj.h +virDomainSnapshotDropChildren; virDomainSnapshotDropParent; virDomainSnapshotForEachChild; virDomainSnapshotForEachDescendant; +virDomainSnapshotMoveChildren; +virDomainSnapshotSetParent; # conf/virdomainsnapshotobjlist.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6c71382b93..eb3d112b69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15926,10 +15926,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } else { other = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); - snap->parent = other; - other->nchildren++; - snap->sibling = other->first_child; - other->first_child = snap; + virDomainSnapshotSetParent(snap, other); } } else if (snap) { virDomainSnapshotObjListRemove(vm->snapshots, snap); @@ -16763,7 +16760,6 @@ struct _virQEMUSnapReparent { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; int err; - virDomainSnapshotObjPtr last; }; @@ -16779,7 +16775,6 @@ qemuDomainSnapshotReparentChildren(void *payload, return 0; VIR_FREE(snap->def->parent); - snap->parent = rep->parent; if (rep->parent->def && VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { @@ -16787,9 +16782,6 @@ qemuDomainSnapshotReparentChildren(void *payload, return 0; } - if (!snap->sibling) - rep->last = snap; - rep->err = qemuDomainSnapshotWriteMetadata(rep->vm, snap, rep->caps, rep->xmlopt, rep->cfg->snapshotDir); @@ -16877,7 +16869,6 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, rep.parent = snap->parent; rep.vm = vm; rep.err = 0; - rep.last = NULL; rep.caps = driver->caps; rep.xmlopt = driver->xmlopt; virDomainSnapshotForEachChild(snap, @@ -16885,15 +16876,11 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, &rep); if (rep.err < 0) goto endjob; - /* Can't modify siblings during ForEachChild, so do it now. */ - snap->parent->nchildren += snap->nchildren; - rep.last->sibling = snap->parent->first_child; - snap->parent->first_child = snap->first_child; + virDomainSnapshotMoveChildren(snap, snap->parent); } if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { - snap->nchildren = 0; - snap->first_child = NULL; + virDomainSnapshotDropChildren(snap); ret = 0; } else { ret = qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata_only); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 9cbef70f1c..d3b76bfdbd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6416,10 +6416,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotSetCurrent(vm->snapshots, snap); other = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); - snap->parent = other; - other->nchildren++; - snap->sibling = other->first_child; - other->first_child = snap; + virDomainSnapshotSetParent(snap, other); } virDomainObjEndAPI(&vm); } @@ -6454,7 +6451,6 @@ struct _testSnapReparentData { virDomainSnapshotObjPtr parent; virDomainObjPtr vm; int err; - virDomainSnapshotObjPtr last; }; static int @@ -6469,7 +6465,6 @@ testDomainSnapshotReparentChildren(void *payload, return 0; VIR_FREE(snap->def->parent); - snap->parent = rep->parent; if (rep->parent->def && VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { @@ -6477,8 +6472,6 @@ testDomainSnapshotReparentChildren(void *payload, return 0; } - if (!snap->sibling) - rep->last = snap; return 0; } @@ -6515,22 +6508,17 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, rep.parent = snap->parent; rep.vm = vm; rep.err = 0; - rep.last = NULL; virDomainSnapshotForEachChild(snap, testDomainSnapshotReparentChildren, &rep); if (rep.err < 0) goto cleanup; - /* Can't modify siblings during ForEachChild, so do it now. */ - snap->parent->nchildren += snap->nchildren; - rep.last->sibling = snap->parent->first_child; - snap->parent->first_child = snap->first_child; + virDomainSnapshotMoveChildren(snap, snap->parent); } if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { - snap->nchildren = 0; - snap->first_child = NULL; + virDomainSnapshotDropChildren(snap); } else { virDomainSnapshotDropParent(snap); if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { -- 2.20.1

Bjoern Walk <bwalk(a)linux.ibm.com&gt; [2019-03-28, 08:31AM +0100]: Sorry, no morning coffee yet, the fix is in, but I still get a GCC warning: CC conf/libvirt_conf_la-virdomainmomentobjlist.lo ../../src/conf/virdomainmomentobjlist.c: In function 'virDomainMomentMoveChildren': ../../src/conf/virdomainmomentobjlist.c:178:19: error: 'last' may be used uninitialized in this function [-Werror=maybe-uninitialized] last->sibling = to->first_child; ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[5]: *** [Makefile:9650: conf/libvirt_conf_la-virdomainmomentobjlist.lo] Error 1 This fixes it: diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 92cf52dd..2e9343ff 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -165,7 +165,7 @@ virDomainMomentMoveChildren(virDomainMomentObjPtr from, virDomainMomentObjPtr to) { virDomainMomentObjPtr child; - virDomainMomentObjPtr last; + virDomainMomentObjPtr last = NULL; if (!from->first_child) return; -- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 3/28/19 8:51 AM, Eric Blake wrote: Actually, I'm worried that this may trigger other warnings. I'll post a separate message with an alternative fix proposal, and we can choose which one looks nicer. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

On 3/20/19 1:40 AM, Eric Blake wrote: [...] [...] Too much substitution ;-/... leave the old code other->def = def; Or maybe we'll need the corresponding virDomainSnapshotObjSetDef... Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John [...]

On 3/20/19 12:40 AM, Eric Blake wrote: Whoops - this tries to follow a chain back to a root, but while it updates other, it does not update otherdef. I caused a circular chain in trying to check for circular chains. Will fix, along with a testsuite addition that covers it. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Pull out the common parts of virDomainSnapshotDef that will be reused for virDomainCheckpointDef into a new base class. Adjust all callers that use the direct fields (some of it is churn that disappears when the next patch refactors virDomainSnapshotObj; oh well...). Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/moment_conf.h | 41 +++++++++ src/conf/snapshot_conf.h | 11 +-- src/conf/virconftypes.h | 3 + src/conf/Makefile.inc.am | 2 + src/conf/moment_conf.c | 40 +++++++++ src/conf/snapshot_conf.c | 128 ++++++++++++++-------------- src/conf/virdomainsnapshotobj.c | 2 +- src/conf/virdomainsnapshotobjlist.c | 21 ++--- src/esx/esx_driver.c | 16 ++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 18 ++-- src/qemu/qemu_driver.c | 50 +++++------ src/test/test_driver.c | 42 ++++----- src/vbox/vbox_common.c | 88 +++++++++---------- 14 files changed, 273 insertions(+), 191 deletions(-) create mode 100644 src/conf/moment_conf.h create mode 100644 src/conf/moment_conf.c diff --git a/src/conf/moment_conf.h b/src/conf/moment_conf.h new file mode 100644 index 0000000000..348cfe5a5c --- /dev/null +++ b/src/conf/moment_conf.h @@ -0,0 +1,41 @@ +/* + * moment_conf.h: domain snapshot/checkpoint base class + * + * Copyright (C) 2006-2019 Red Hat, Inc. + * Copyright (C) 2006-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef LIBVIRT_MOMENT_CONF_H +# define LIBVIRT_MOMENT_CONF_H + +# include "internal.h" +# include "virconftypes.h" + +/* Base class for a domain moment */ +struct _virDomainMomentDef { + /* Common portion of public XML. */ + char *name; + char *description; + char *parent; + long long creationTime; /* in seconds */ + + virDomainDefPtr dom; +}; + +void virDomainMomentDefClear(virDomainMomentDefPtr def); + +#endif /* LIBVIRT_MOMENT_CONF_H */ diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index b13a500af4..4b777e94dc 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -24,6 +24,7 @@ # include "internal.h" # include "domain_conf.h" +# include "moment_conf.h" /* Items related to snapshot state */ @@ -72,11 +73,9 @@ struct _virDomainSnapshotDiskDef { /* Stores the complete snapshot metadata */ struct _virDomainSnapshotDef { - /* Public XML. */ - char *name; - char *description; - char *parent; - long long creationTime; /* in seconds */ + virDomainMomentDef common; + + /* Additional public XML. */ int state; /* virDomainSnapshotState */ int memory; /* virDomainMemorySnapshot */ @@ -85,8 +84,6 @@ struct _virDomainSnapshotDef { size_t ndisks; /* should not exceed dom->ndisks */ virDomainSnapshotDiskDef *disks; - virDomainDefPtr dom; - virObjectPtr cookie; }; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 3265c0468a..9b9ab314e7 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -217,6 +217,9 @@ typedef virDomainMemoryDef *virDomainMemoryDefPtr; typedef struct _virDomainMemtune virDomainMemtune; typedef virDomainMemtune *virDomainMemtunePtr; +typedef struct _virDomainMomentDef virDomainMomentDef; +typedef virDomainMomentDef *virDomainMomentDefPtr; + typedef struct _virDomainNVRAMDef virDomainNVRAMDef; typedef virDomainNVRAMDef *virDomainNVRAMDefPtr; diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index 9b4d80485b..d2ff8be8fd 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -22,6 +22,8 @@ DOMAIN_CONF_SOURCES = \ conf/domain_nwfilter.h \ conf/virsavecookie.c \ conf/virsavecookie.h \ + conf/moment_conf.c \ + conf/moment_conf.h \ conf/snapshot_conf.c \ conf/snapshot_conf.h \ conf/numa_conf.c \ diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c new file mode 100644 index 0000000000..f9276a5d1e --- /dev/null +++ b/src/conf/moment_conf.c @@ -0,0 +1,40 @@ +/* + * moment_conf.c: domain snapshot/checkpoint base class + * + * Copyright (C) 2006-2019 Red Hat, Inc. + * Copyright (C) 2006-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "internal.h" +#include "moment_conf.h" +#include "domain_conf.h" +#include "virlog.h" +#include "viralloc.h" + +#define VIR_FROM_THIS VIR_FROM_DOMAIN_SNAPSHOT + +VIR_LOG_INIT("conf.moment_conf"); + +void virDomainMomentDefClear(virDomainMomentDefPtr def) +{ + VIR_FREE(def->name); + VIR_FREE(def->description); + VIR_FREE(def->parent); + virDomainDefFree(def->dom); +} diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index aec23f111c..b80b199ce4 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -87,14 +87,11 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) if (!def) return; - VIR_FREE(def->name); - VIR_FREE(def->description); - VIR_FREE(def->parent); + virDomainMomentDefClear(&def->common); VIR_FREE(def->file); for (i = 0; i < def->ndisks; i++) virDomainSnapshotDiskDefClear(&def->disks[i]); VIR_FREE(def->disks); - virDomainDefFree(def->dom); virObjectUnref(def->cookie); VIR_FREE(def); } @@ -213,28 +210,28 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, gettimeofday(&tv, NULL); - def->name = virXPathString("string(./name)", ctxt); - if (def->name == NULL) { + def->common.name = virXPathString("string(./name)", ctxt); + if (def->common.name == NULL) { if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { virReportError(VIR_ERR_XML_ERROR, "%s", _("a redefined snapshot must have a name")); goto cleanup; } - if (virAsprintf(&def->name, "%lld", (long long)tv.tv_sec) < 0) + if (virAsprintf(&def->common.name, "%lld", (long long)tv.tv_sec) < 0) goto cleanup; } - def->description = virXPathString("string(./description)", ctxt); + def->common.description = virXPathString("string(./description)", ctxt); if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { if (virXPathLongLong("string(./creationTime)", ctxt, - &def->creationTime) < 0) { + &def->common.creationTime) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing creationTime from existing snapshot")); goto cleanup; } - def->parent = virXPathString("string(./parent/name)", ctxt); + def->common.parent = virXPathString("string(./parent/name)", ctxt); state = virXPathString("string(./state)", ctxt); if (state == NULL) { @@ -270,15 +267,15 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, _("missing domain in snapshot")); goto cleanup; } - def->dom = virDomainDefParseNode(ctxt->node->doc, domainNode, - caps, xmlopt, NULL, domainflags); - if (!def->dom) + def->common.dom = virDomainDefParseNode(ctxt->node->doc, domainNode, + caps, xmlopt, NULL, domainflags); + if (!def->common.dom) goto cleanup; } else { VIR_WARN("parsing older snapshot that lacks domain"); } } else { - def->creationTime = tv.tv_sec; + def->common.creationTime = tv.tv_sec; } memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt); @@ -424,7 +421,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, /* Perform sanity checking on a redefined snapshot definition. If - * @other is non-NULL, this may include swapping def->dom from other + * @other is non-NULL, this may include swapping def->common.dom from other * into def. */ int virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, @@ -442,16 +439,17 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, virReportError(VIR_ERR_INVALID_ARG, _("disk-only flag for snapshot %s requires " "disk-snapshot state"), - def->name); + def->common.name); return -1; } - if (def->dom && memcmp(def->dom->uuid, domain_uuid, VIR_UUID_BUFLEN)) { + if (def->common.dom && memcmp(def->common.dom->uuid, domain_uuid, + VIR_UUID_BUFLEN)) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(domain_uuid, uuidstr); virReportError(VIR_ERR_INVALID_ARG, _("definition for snapshot %s must use uuid %s"), - def->name, uuidstr); + def->common.name, uuidstr); return -1; } @@ -465,7 +463,7 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, virReportError(VIR_ERR_INVALID_ARG, _("cannot change between online and offline " "snapshot state in snapshot %s"), - def->name); + def->common.name); return -1; } @@ -474,24 +472,24 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, virReportError(VIR_ERR_INVALID_ARG, _("cannot change between disk only and " "full system in snapshot %s"), - def->name); + def->common.name); return -1; } - if (otherdef->dom) { - if (def->dom) { - if (!virDomainDefCheckABIStability(otherdef->dom, - def->dom, xmlopt)) + if (otherdef->common.dom) { + if (def->common.dom) { + if (!virDomainDefCheckABIStability(otherdef->common.dom, + def->common.dom, xmlopt)) return -1; } else { /* Transfer the domain def */ - def->dom = otherdef->dom; - otherdef->dom = NULL; + def->common.dom = otherdef->common.dom; + otherdef->common.dom = NULL; } } } - if (def->dom) { + if (def->common.dom) { if (external) { align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; align_match = false; @@ -538,7 +536,7 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDefPtr def) return -1; } - if (!(origpath = virDomainDiskGetSource(def->dom->disks[i]))) { + if (!(origpath = virDomainDiskGetSource(def->common.dom->disks[i]))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot generate external snapshot name " "for disk '%s' without source"), @@ -562,7 +560,7 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDefPtr def) if ((tmp = strrchr(tmppath, '.')) && !strchr(tmp, '/')) *tmp = '\0'; - if (virAsprintf(&disk->src->path, "%s.%s", tmppath, def->name) < 0) { + if (virAsprintf(&disk->src->path, "%s.%s", tmppath, def->common.name) < 0) { VIR_FREE(tmppath); return -1; } @@ -595,7 +593,7 @@ virDomainSnapshotCompareDiskIndex(const void *a, const void *b) return diska->idx - diskb->idx; } -/* Align def->disks to def->domain. Sort the list of def->disks, +/* Align def->disks to def->common.dom. Sort the list of def->disks, * filling in any missing disks or snapshot state defaults given by * the domain, with a fallback to a passed in default. Convert paths * to disk targets for uniformity. Issue an error and return -1 if @@ -612,31 +610,31 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, size_t i; int ndisks; - if (!def->dom) { + if (!def->common.dom) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing domain in snapshot")); goto cleanup; } - if (def->ndisks > def->dom->ndisks) { + if (def->ndisks > def->common.dom->ndisks) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("too many disk snapshot requests for domain")); goto cleanup; } /* Unlikely to have a guest without disks but technically possible. */ - if (!def->dom->ndisks) { + if (!def->common.dom->ndisks) { ret = 0; goto cleanup; } - if (!(map = virBitmapNew(def->dom->ndisks))) + if (!(map = virBitmapNew(def->common.dom->ndisks))) goto cleanup; /* Double check requested disks. */ for (i = 0; i < def->ndisks; i++) { virDomainSnapshotDiskDefPtr disk = &def->disks[i]; - int idx = virDomainDiskIndexByName(def->dom, disk->name, false); + int idx = virDomainDiskIndexByName(def->common.dom, disk->name, false); int disk_snapshot; if (idx < 0) { @@ -654,7 +652,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, ignore_value(virBitmapSetBit(map, idx)); disk->idx = idx; - disk_snapshot = def->dom->disks[idx]->snapshot; + disk_snapshot = def->common.dom->disks[idx]->snapshot; if (!disk->snapshot) { if (disk_snapshot && (!require_match || @@ -682,9 +680,9 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, disk->src->path, disk->name); goto cleanup; } - if (STRNEQ(disk->name, def->dom->disks[idx]->dst)) { + if (STRNEQ(disk->name, def->common.dom->disks[idx]->dst)) { VIR_FREE(disk->name); - if (VIR_STRDUP(disk->name, def->dom->disks[idx]->dst) < 0) + if (VIR_STRDUP(disk->name, def->common.dom->disks[idx]->dst) < 0) goto cleanup; } } @@ -692,10 +690,10 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, /* Provide defaults for all remaining disks. */ ndisks = def->ndisks; if (VIR_EXPAND_N(def->disks, def->ndisks, - def->dom->ndisks - def->ndisks) < 0) + def->common.dom->ndisks - def->ndisks) < 0) goto cleanup; - for (i = 0; i < def->dom->ndisks; i++) { + for (i = 0; i < def->common.dom->ndisks; i++) { virDomainSnapshotDiskDefPtr disk; if (virBitmapIsBitSet(map, i)) @@ -703,15 +701,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, disk = &def->disks[ndisks++]; if (!(disk->src = virStorageSourceNew())) goto cleanup; - if (VIR_STRDUP(disk->name, def->dom->disks[i]->dst) < 0) + if (VIR_STRDUP(disk->name, def->common.dom->disks[i]->dst) < 0) goto cleanup; disk->idx = i; /* Don't snapshot empty drives */ - if (virStorageSourceIsEmpty(def->dom->disks[i]->src)) + if (virStorageSourceIsEmpty(def->common.dom->disks[i]->src)) disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; else - disk->snapshot = def->dom->disks[i]->snapshot; + disk->snapshot = def->common.dom->disks[i]->snapshot; disk->src->type = VIR_STORAGE_TYPE_FILE; if (!disk->snapshot) @@ -802,23 +800,23 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "<domainsnapshot>\n"); virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<name>%s</name>\n", def->name); - if (def->description) + virBufferEscapeString(buf, "<name>%s</name>\n", def->common.name); + if (def->common.description) virBufferEscapeString(buf, "<description>%s</description>\n", - def->description); + def->common.description); virBufferAsprintf(buf, "<state>%s</state>\n", virDomainSnapshotStateTypeToString(def->state)); - if (def->parent) { + if (def->common.parent) { virBufferAddLit(buf, "<parent>\n"); virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<name>%s</name>\n", def->parent); + virBufferEscapeString(buf, "<name>%s</name>\n", def->common.parent); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</parent>\n"); } virBufferAsprintf(buf, "<creationTime>%lld</creationTime>\n", - def->creationTime); + def->common.creationTime); if (def->memory) { virBufferAsprintf(buf, "<memory snapshot='%s'", @@ -838,8 +836,8 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "</disks>\n"); } - if (def->dom) { - if (virDomainDefFormatInternal(def->dom, caps, domainflags, buf, + if (def->common.dom) { + if (virDomainDefFormatInternal(def->common.dom, caps, domainflags, buf, xmlopt) < 0) goto error; } else if (uuidstr) { @@ -931,30 +929,30 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, bool check_if_stolen; /* Prevent circular chains */ - if (def->parent) { - if (STREQ(def->name, def->parent)) { + if (def->common.parent) { + if (STREQ(def->common.name, def->common.parent)) { virReportError(VIR_ERR_INVALID_ARG, _("cannot set snapshot %s as its own parent"), - def->name); + def->common.name); return -1; } - other = virDomainSnapshotFindByName(vm->snapshots, def->parent); + other = virDomainSnapshotFindByName(vm->snapshots, def->common.parent); if (!other) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s for snapshot %s not found"), - def->parent, def->name); + def->common.parent, def->common.name); return -1; } otherdef = virDomainSnapshotObjGetDef(other); - while (otherdef->parent) { - if (STREQ(otherdef->parent, def->name)) { + while (otherdef->common.parent) { + if (STREQ(otherdef->common.parent, def->common.name)) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s would create cycle to %s"), - otherdef->name, def->name); + otherdef->common.name, def->common.name); return -1; } other = virDomainSnapshotFindByName(vm->snapshots, - otherdef->parent); + otherdef->common.parent); if (!other) { VIR_WARN("snapshots are inconsistent for %s", vm->def->name); @@ -963,15 +961,15 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, } } - other = virDomainSnapshotFindByName(vm->snapshots, def->name); + other = virDomainSnapshotFindByName(vm->snapshots, def->common.name); otherdef = other ? virDomainSnapshotObjGetDef(other) : NULL; - check_if_stolen = other && otherdef->dom; + check_if_stolen = other && otherdef->common.dom; if (virDomainSnapshotRedefineValidate(def, domain->uuid, other, xmlopt, flags) < 0) { /* revert any stealing of the snapshot domain definition */ - if (check_if_stolen && def->dom && !otherdef->dom) { - otherdef->dom = def->dom; - def->dom = NULL; + if (check_if_stolen && def->common.dom && !otherdef->common.dom) { + otherdef->common.dom = def->common.dom; + def->common.dom = NULL; } return -1; } diff --git a/src/conf/virdomainsnapshotobj.c b/src/conf/virdomainsnapshotobj.c index d6b216c7b2..0e3ee012fc 100644 --- a/src/conf/virdomainsnapshotobj.c +++ b/src/conf/virdomainsnapshotobj.c @@ -45,7 +45,7 @@ virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot, while (child) { virDomainSnapshotObjPtr next = child->sibling; - (iter)(child, child->def->name, data); + (iter)(child, child->def->common.name, data); child = next; } diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index cd3ea569e3..58f094fa02 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -240,10 +240,10 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s { virDomainSnapshotObjPtr snap; - if (virHashLookup(snapshots->objs, def->name) != NULL) { + if (virHashLookup(snapshots->objs, def->common.name) != NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected domain snapshot %s already exists"), - def->name); + def->common.name); return NULL; } @@ -251,7 +251,7 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s return NULL; snap->def = def; - if (virHashAddEntry(snapshots->objs, snap->def->name, snap) < 0) { + if (virHashAddEntry(snapshots->objs, snap->def->common.name, snap) < 0) { VIR_FREE(snap); return NULL; } @@ -354,7 +354,7 @@ static int virDomainSnapshotObjListCopyNames(void *payload, return 0; if (data->names && data->count < data->maxnames && - VIR_STRDUP(data->names[data->count], obj->def->name) < 0) { + VIR_STRDUP(data->names[data->count], obj->def->common.name) < 0) { data->error = true; return 0; } @@ -475,7 +475,7 @@ const char * virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots) { if (snapshots->current) - return snapshots->current->def->name; + return snapshots->current->def->common.name; return NULL; } @@ -485,7 +485,8 @@ bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, const char *name) { - return snapshots->current && STREQ(snapshots->current->def->name, name); + return snapshots->current && STREQ(snapshots->current->def->common.name, + name); } @@ -503,7 +504,7 @@ bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot) { bool ret = snapshots->current == snapshot; - virHashRemoveEntry(snapshots->objs, snapshot->def->name); + virHashRemoveEntry(snapshots->objs, snapshot->def->common.name); if (ret) snapshots->current = NULL; return ret; @@ -547,18 +548,18 @@ virDomainSnapshotSetRelations(void *payload, virDomainSnapshotObjPtr tmp; virDomainSnapshotObjPtr parent; - parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->parent); + parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->common.parent); if (!parent) { curr->err = -1; parent = &curr->snapshots->metaroot; - VIR_WARN("snapshot %s lacks parent", obj->def->name); + VIR_WARN("snapshot %s lacks parent", obj->def->common.name); } else { tmp = parent; while (tmp && tmp->def) { if (tmp == obj) { curr->err = -1; parent = &curr->snapshots->metaroot; - VIR_WARN("snapshot %s in circular chain", obj->def->name); + VIR_WARN("snapshot %s in circular chain", obj->def->common.name); break; } tmp = tmp->parent; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 1c6a2dcb71..5cf881ae35 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4118,7 +4118,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, priv->parsedUri->autoAnswer) < 0 || esxVI_LookupRootSnapshotTreeList(priv->primary, domain->uuid, &rootSnapshotList) < 0 || - esxVI_GetSnapshotTreeByName(rootSnapshotList, def->name, + esxVI_GetSnapshotTreeByName(rootSnapshotList, def->common.name, &snapshotTree, NULL, esxVI_Occurrence_OptionalItem) < 0) { goto cleanup; @@ -4126,12 +4126,12 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, if (snapshotTree) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Snapshot '%s' already exists"), def->name); + _("Snapshot '%s' already exists"), def->common.name); goto cleanup; } if (esxVI_CreateSnapshot_Task(priv->primary, virtualMachine->obj, - def->name, def->description, + def->common.name, def->common.description, diskOnly ? esxVI_Boolean_False : esxVI_Boolean_True, quiesce ? esxVI_Boolean_True : esxVI_Boolean_False, &task) < 0 || @@ -4148,7 +4148,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, goto cleanup; } - snapshot = virGetDomainSnapshot(domain, def->name); + snapshot = virGetDomainSnapshot(domain, def->common.name); cleanup: virDomainSnapshotDefFree(def); @@ -4189,12 +4189,12 @@ esxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, goto cleanup; } - def.name = virSnapName(snapshot); - def.description = snapshotTree->description; - def.parent = snapshotTreeParent ? snapshotTreeParent->name : NULL; + def.common.name = virSnapName(snapshot); + def.common.description = snapshotTree->description; + def.common.parent = snapshotTreeParent ? snapshotTreeParent->name : NULL; if (esxVI_DateTime_ConvertToCalendarTime(snapshotTree->createTime, - &def.creationTime) < 0) { + &def.common.creationTime) < 0) { goto cleanup; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bc02362495..000da64d0e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10859,7 +10859,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, goto error; if (snapshot) - virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); + virCommandAddArgList(cmd, "-loadvm", snapshot->def->common.name, NULL); if (def->namespaceData) { qemuDomainCmdlineDefPtr qemucmd; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e693a3b122..8e02f05c3c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8477,7 +8477,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, goto cleanup; } - if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->name) < 0) + if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->common.name) < 0) goto cleanup; ret = virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml); @@ -8573,11 +8573,11 @@ qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver, /* Prefer action on the disks in use at the time the snapshot was * created; but fall back to current definition if dealing with a * snapshot created prior to libvirt 0.9.5. */ - virDomainDefPtr def = snap->def->dom; + virDomainDefPtr def = snap->def->common.dom; if (!def) def = vm->def; - return qemuDomainSnapshotForEachQcow2Raw(driver, def, snap->def->name, + return qemuDomainSnapshotForEachQcow2Raw(driver, def, snap->def->common.name, op, try_all, def->ndisks); } @@ -8605,30 +8605,30 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); /* we continue on even in the face of error */ - qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); + qemuMonitorDeleteSnapshot(priv->mon, snap->def->common.name); ignore_value(qemuDomainObjExitMonitor(driver, vm)); } } if (virAsprintf(&snapFile, "%s/%s/%s.xml", cfg->snapshotDir, - vm->def->name, snap->def->name) < 0) + vm->def->name, snap->def->common.name) < 0) goto cleanup; if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { virDomainSnapshotSetCurrent(vm->snapshots, NULL); - if (update_parent && snap->def->parent) { + if (update_parent && snap->def->common.parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->common.parent); if (!parentsnap) { VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->parent); + snap->def->common.parent); } else { virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) { VIR_WARN("failed to set parent snapshot '%s' as current", - snap->def->parent); + snap->def->common.parent); virDomainSnapshotSetCurrent(vm->snapshots, NULL); } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eb3d112b69..ece9c9329e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14547,7 +14547,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, * create them correctly. */ for (i = 0; i < snap->def->ndisks && !reuse; i++) { snapdisk = &(snap->def->disks[i]); - defdisk = snap->def->dom->disks[snapdisk->idx]; + defdisk = snap->def->common.dom->disks[snapdisk->idx]; if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) continue; @@ -14663,7 +14663,7 @@ qemuDomainSnapshotCreateActiveInternal(virQEMUDriverPtr driver, goto cleanup; } - ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); + ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->common.name); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; if (ret < 0) @@ -15726,19 +15726,19 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, /* reject snapshot names containing slashes or starting with dot as * snapshot definitions are saved in files named by the snapshot name */ if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) { - if (strchr(def->name, '/')) { + if (strchr(def->common.name, '/')) { virReportError(VIR_ERR_XML_DETAIL, _("invalid snapshot name '%s': " "name can't contain '/'"), - def->name); + def->common.name); goto cleanup; } - if (def->name[0] == '.') { + if (def->common.name[0] == '.') { virReportError(VIR_ERR_XML_DETAIL, _("invalid snapshot name '%s': " "name can't start with '.'"), - def->name); + def->common.name); goto cleanup; } } @@ -15809,9 +15809,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * conversion in and back out of xml. */ if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU, true, true)) || - !(def->dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL, - VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) + !(def->common.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto endjob; if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) { @@ -15857,7 +15857,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, current = virDomainSnapshotGetCurrent(vm->snapshots); if (current) { if (!redefine && - VIR_STRDUP(snap->def->parent, current->def->name) < 0) + VIR_STRDUP(snap->def->common.parent, current->def->common.name) < 0) goto endjob; if (update_current) { virDomainSnapshotSetCurrent(vm->snapshots, NULL); @@ -15906,7 +15906,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * do; we've successfully taken the snapshot, and we are now running * on it, so we have to go forward the best we can */ - snapshot = virGetDomainSnapshot(domain, snap->def->name); + snapshot = virGetDomainSnapshot(domain, snap->def->common.name); endjob: if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) { @@ -15921,11 +15921,11 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, snapshot = NULL; virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to save metadata for snapshot %s"), - snap->def->name); + snap->def->common.name); virDomainSnapshotObjListRemove(vm->snapshots, snap); } else { other = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->common.parent); virDomainSnapshotSetParent(snap, other); } } else if (snap) { @@ -16135,7 +16135,7 @@ qemuDomainSnapshotLookupByName(virDomainPtr domain, if (!(snap = qemuSnapObjFromName(vm, name))) goto cleanup; - snapshot = virGetDomainSnapshot(domain, snap->def->name); + snapshot = virGetDomainSnapshot(domain, snap->def->common.name); cleanup: virDomainObjEndAPI(&vm); @@ -16185,14 +16185,14 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - if (!snap->def->parent) { + if (!snap->def->common.parent) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, _("snapshot '%s' does not have a parent"), - snap->def->name); + snap->def->common.name); goto cleanup; } - parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->parent); + parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->common.parent); cleanup: virDomainObjEndAPI(&vm); @@ -16417,10 +16417,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { - if (!snap->def->dom) { + if (!snap->def->common.dom) { virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, _("snapshot '%s' lacks domain '%s' rollback info"), - snap->def->name, vm->def->name); + snap->def->common.name, vm->def->name); goto endjob; } if (virDomainObjIsActive(vm) && @@ -16450,8 +16450,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * * XXX Should domain snapshots track live xml rather * than inactive xml? */ - if (snap->def->dom) { - config = virDomainDefCopy(snap->def->dom, caps, + if (snap->def->common.dom) { + config = virDomainDefCopy(snap->def->common.dom, caps, driver->xmlopt, NULL, true); if (!config) goto endjob; @@ -16557,7 +16557,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_START) < 0) goto endjob; - rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name); + rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->common.name); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; if (rc < 0) { @@ -16774,10 +16774,10 @@ qemuDomainSnapshotReparentChildren(void *payload, if (rep->err < 0) return 0; - VIR_FREE(snap->def->parent); + VIR_FREE(snap->def->common.parent); if (rep->parent->def && - VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { + VIR_STRDUP(snap->def->common.parent, rep->parent->def->common.name) < 0) { rep->err = -1; return 0; } @@ -16858,7 +16858,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, cfg->snapshotDir) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to set snapshot '%s' as current"), - snap->def->name); + snap->def->common.name); virDomainSnapshotSetCurrent(vm->snapshots, NULL); goto endjob; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d3b76bfdbd..0caed1de19 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6132,7 +6132,7 @@ testDomainSnapshotLookupByName(virDomainPtr domain, if (!(snap = testSnapObjFromName(vm, name))) goto cleanup; - snapshot = virGetDomainSnapshot(domain, snap->def->name); + snapshot = virGetDomainSnapshot(domain, snap->def->common.name); cleanup: virDomainObjEndAPI(&vm); @@ -6173,14 +6173,14 @@ testDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - if (!snap->def->parent) { + if (!snap->def->common.parent) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, _("snapshot '%s' does not have a parent"), - snap->def->name); + snap->def->common.name); goto cleanup; } - parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->parent); + parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->common.parent); cleanup: virDomainObjEndAPI(&vm); @@ -6207,7 +6207,7 @@ testDomainSnapshotCurrent(virDomainPtr domain, goto cleanup; } - snapshot = virGetDomainSnapshot(domain, current->def->name); + snapshot = virGetDomainSnapshot(domain, current->def->common.name); cleanup: virDomainObjEndAPI(&vm); @@ -6376,11 +6376,11 @@ testDomainSnapshotCreateXML(virDomainPtr domain, &update_current, flags) < 0) goto cleanup; } else { - if (!(def->dom = virDomainDefCopy(vm->def, - privconn->caps, - privconn->xmlopt, - NULL, - true))) + if (!(def->common.dom = virDomainDefCopy(vm->def, + privconn->caps, + privconn->xmlopt, + NULL, + true))) goto cleanup; if (testDomainSnapshotAlignDisks(vm, def, flags) < 0) @@ -6394,7 +6394,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, } if (!redefine) { - if (VIR_STRDUP(snap->def->parent, + if (VIR_STRDUP(snap->def->common.parent, virDomainSnapshotGetCurrentName(vm->snapshots)) < 0) goto cleanup; @@ -6407,7 +6407,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, } } - snapshot = virGetDomainSnapshot(domain, snap->def->name); + snapshot = virGetDomainSnapshot(domain, snap->def->common.name); cleanup: if (vm) { if (snapshot) { @@ -6415,7 +6415,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, if (update_current) virDomainSnapshotSetCurrent(vm->snapshots, snap); other = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->common.parent); virDomainSnapshotSetParent(snap, other); } virDomainObjEndAPI(&vm); @@ -6464,10 +6464,10 @@ testDomainSnapshotReparentChildren(void *payload, if (rep->err < 0) return 0; - VIR_FREE(snap->def->parent); + VIR_FREE(snap->def->common.parent); if (rep->parent->def && - VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { + VIR_STRDUP(snap->def->common.parent, rep->parent->def->common.name) < 0) { rep->err = -1; return 0; } @@ -6522,12 +6522,12 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, } else { virDomainSnapshotDropParent(snap); if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { - if (snap->def->parent) { + if (snap->def->common.parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->common.parent); if (!parentsnap) VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->parent); + snap->def->common.parent); } virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); } @@ -6588,10 +6588,10 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { - if (!snap->def->dom) { + if (!snap->def->common.dom) { virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, _("snapshot '%s' lacks domain '%s' rollback info"), - snap->def->name, vm->def->name); + snap->def->common.name, vm->def->name); goto cleanup; } if (virDomainObjIsActive(vm) && @@ -6607,7 +6607,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainSnapshotSetCurrent(vm->snapshots, NULL); - config = virDomainDefCopy(snap->def->dom, privconn->caps, + config = virDomainDefCopy(snap->def->common.dom, privconn->caps, privconn->xmlopt, NULL, true); if (!config) goto cleanup; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 2ca12a28e5..fb8fc3a10c 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -4814,7 +4814,7 @@ vboxSnapshotRedefine(virDomainPtr dom, * read-only disks are in the redefined snapshot's media registry (the disks need to * be open to query their uuid). */ - for (it = 0; it < def->dom->ndisks; it++) { + for (it = 0; it < def->common.dom->ndisks; it++) { int diskInMediaRegistry = 0; IMedium *readOnlyMedium = NULL; PRUnichar *locationUtf = NULL; @@ -4828,7 +4828,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VBOX_IID_INITIALIZE(&iid); VBOX_IID_INITIALIZE(&parentiid); diskInMediaRegistry = virVBoxSnapshotConfDiskIsInMediaRegistry(snapshotMachineDesc, - def->dom->disks[it]->src->path); + def->common.dom->disks[it]->src->path); if (diskInMediaRegistry == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to know if disk is in media registry")); @@ -4838,7 +4838,7 @@ vboxSnapshotRedefine(virDomainPtr dom, continue; /*The read only disk is not in the media registry*/ - VBOX_UTF8_TO_UTF16(def->dom->disks[it]->src->path, &locationUtf); + VBOX_UTF8_TO_UTF16(def->common.dom->disks[it]->src->path, &locationUtf); rc = gVBoxAPI.UIVirtualBox.OpenMedium(data->vboxObj, locationUtf, DeviceType_HardDisk, @@ -4911,7 +4911,7 @@ vboxSnapshotRedefine(virDomainPtr dom, readOnlyDisk->format = format; readOnlyDisk->uuid = uuid; - if (VIR_STRDUP(readOnlyDisk->location, def->dom->disks[it]->src->path) < 0) { + if (VIR_STRDUP(readOnlyDisk->location, def->common.dom->disks[it]->src->path) < 0) { VIR_FREE(readOnlyDisk); goto cleanup; } @@ -5016,12 +5016,12 @@ vboxSnapshotRedefine(virDomainPtr dom, goto cleanup; VIR_DEBUG("New snapshot UUID: %s", newSnapshotPtr->uuid); - if (VIR_STRDUP(newSnapshotPtr->name, def->name) < 0) + if (VIR_STRDUP(newSnapshotPtr->name, def->common.name) < 0) goto cleanup; - newSnapshotPtr->timeStamp = virTimeStringThen(def->creationTime * 1000); + newSnapshotPtr->timeStamp = virTimeStringThen(def->common.creationTime * 1000); - if (VIR_STRDUP(newSnapshotPtr->description, def->description) < 0) + if (VIR_STRDUP(newSnapshotPtr->description, def->common.description) < 0) goto cleanup; if (VIR_STRDUP(newSnapshotPtr->hardware, snapshotMachineDesc->hardware) < 0) @@ -5031,12 +5031,12 @@ vboxSnapshotRedefine(virDomainPtr dom, goto cleanup; /*We get the parent disk uuid from the parent disk location to correctly fill the storage controller.*/ - for (it = 0; it < def->dom->ndisks; it++) { + for (it = 0; it < def->common.dom->ndisks; it++) { char *location = NULL; const char *uuidReplacing = NULL; char *tmp = NULL; - location = def->dom->disks[it]->src->path; + location = def->common.dom->disks[it]->src->path; if (!location) goto cleanup; /*Replacing the uuid*/ @@ -5064,7 +5064,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VIR_FREE(tmp); } - if (virVBoxSnapshotConfAddSnapshotToXmlMachine(newSnapshotPtr, snapshotMachineDesc, def->parent) < 0) { + if (virVBoxSnapshotConfAddSnapshotToXmlMachine(newSnapshotPtr, snapshotMachineDesc, def->common.parent) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to add the snapshot to the machine description")); goto cleanup; @@ -5084,10 +5084,10 @@ vboxSnapshotRedefine(virDomainPtr dom, * Open the snapshot's read-write disk's full ancestry to allow opening the * read-write disk itself. */ - for (it = 0; it < def->dom->ndisks; it++) { + for (it = 0; it < def->common.dom->ndisks; it++) { char *location = NULL; - location = def->dom->disks[it]->src->path; + location = def->common.dom->disks[it]->src->path; if (!location) goto cleanup; @@ -5231,7 +5231,7 @@ vboxSnapshotRedefine(virDomainPtr dom, } } else { /*Create a "fake" disk to avoid corrupting children snapshot disks.*/ - for (it = 0; it < def->dom->ndisks; it++) { + for (it = 0; it < def->common.dom->ndisks; it++) { IMedium *medium = NULL; PRUnichar *locationUtf16 = NULL; char *parentUuid = NULL; @@ -5247,7 +5247,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VBOX_IID_INITIALIZE(&iid); VBOX_IID_INITIALIZE(&parentiid); - VBOX_UTF8_TO_UTF16(def->dom->disks[it]->src->path, &locationUtf16); + VBOX_UTF8_TO_UTF16(def->common.dom->disks[it]->src->path, &locationUtf16); rc = gVBoxAPI.UIVirtualBox.OpenMedium(data->vboxObj, locationUtf16, DeviceType_HardDisk, @@ -5396,8 +5396,8 @@ vboxSnapshotRedefine(virDomainPtr dom, * All the snapshot structure manipulation is done, we close the disks we have * previously opened. */ - for (it = 0; it < def->dom->ndisks; it++) { - char *location = def->dom->disks[it]->src->path; + for (it = 0; it < def->common.dom->ndisks; it++) { + char *location = def->common.dom->disks[it]->src->path; if (!location) goto cleanup; @@ -5516,7 +5516,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) { if (vboxSnapshotRedefine(dom, def, isCurrent) < 0) goto cleanup; - ret = virGetDomainSnapshot(dom, def->name); + ret = virGetDomainSnapshot(dom, def->common.name); goto cleanup; } } @@ -5543,14 +5543,14 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, goto cleanup; } - VBOX_UTF8_TO_UTF16(def->name, &name); + VBOX_UTF8_TO_UTF16(def->common.name, &name); if (!name) { virReportOOMError(); goto cleanup; } - if (def->description) { - VBOX_UTF8_TO_UTF16(def->description, &description); + if (def->common.description) { + VBOX_UTF8_TO_UTF16(def->common.description, &description); if (!description) { virReportOOMError(); goto cleanup; @@ -5580,7 +5580,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, goto cleanup; } - ret = virGetDomainSnapshot(dom, def->name); + ret = virGetDomainSnapshot(dom, def->common.name); cleanup: VBOX_RELEASE(progress); @@ -5984,7 +5984,7 @@ vboxSnapshotGetReadOnlyDisks(virDomainSnapshotDefPtr def, vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER; size_t i = 0, diskCount = 0, sdCount = 0; int ret = -1; - virDomainDefPtr defdom = def->dom; + virDomainDefPtr defdom = def->common.dom; if (!data->vboxObj) return ret; @@ -6223,10 +6223,10 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (!(snap = vboxDomainSnapshotGet(data, dom, machine, virSnapName(snapshot)))) goto cleanup; - if (VIR_ALLOC(def) < 0 || !(def->dom = virDomainDefNew())) + if (VIR_ALLOC(def) < 0 || !(def->common.dom = virDomainDefNew())) goto cleanup; - defdom = def->dom; - if (VIR_STRDUP(def->name, virSnapName(snapshot)) < 0) + defdom = def->common.dom; + if (VIR_STRDUP(def->common.name, virSnapName(snapshot)) < 0) goto cleanup; if (gVBoxAPI.vboxSnapshotRedefine) { @@ -6274,7 +6274,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (str16) { VBOX_UTF16_TO_UTF8(str16, &str8); VBOX_UTF16_FREE(str16); - if (VIR_STRDUP(def->description, str8) < 0) { + if (VIR_STRDUP(def->common.description, str8) < 0) { VBOX_UTF8_FREE(str8); goto cleanup; } @@ -6289,7 +6289,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, goto cleanup; } /* timestamp is in milliseconds while creationTime in seconds */ - def->creationTime = timestamp / 1000; + def->common.creationTime = timestamp / 1000; rc = gVBoxAPI.UISnapshot.GetParent(snap, &parent); if (NS_FAILED(rc)) { @@ -6308,7 +6308,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, } VBOX_UTF16_TO_UTF8(str16, &str8); VBOX_UTF16_FREE(str16); - if (VIR_STRDUP(def->parent, str8) < 0) { + if (VIR_STRDUP(def->common.parent, str8) < 0) { VBOX_UTF8_FREE(str8); goto cleanup; } @@ -6990,7 +6990,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) goto cleanup; } - isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc, def->name); + isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc, def->common.name); if (isCurrent < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to know if the snapshot is the current snapshot")); @@ -7002,8 +7002,8 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) * disks. The first thing to do is to manipulate VirtualBox API to create * differential read-write disks if the parent snapshot is not null. */ - if (def->parent != NULL) { - for (it = 0; it < def->dom->ndisks; it++) { + if (def->common.parent != NULL) { + for (it = 0; it < def->common.dom->ndisks; it++) { virVBoxSnapshotConfHardDiskPtr readOnly = NULL; IMedium *medium = NULL; PRUnichar *locationUtf16 = NULL; @@ -7023,7 +7023,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) VBOX_IID_INITIALIZE(&iid); VBOX_IID_INITIALIZE(&parentiid); readOnly = virVBoxSnapshotConfHardDiskPtrByLocation(snapshotMachineDesc, - def->dom->disks[it]->src->path); + def->common.dom->disks[it]->src->path); if (!readOnly) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot get hard disk by location")); @@ -7061,7 +7061,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) VBOX_UTF8_TO_UTF16("VDI", &formatUtf16); if (virAsprintf(&newLocationUtf8, "%sfakedisk-%s-%d.vdi", - machineLocationPath, def->parent, it) < 0) + machineLocationPath, def->common.parent, it) < 0) goto cleanup; VBOX_UTF8_TO_UTF16(newLocationUtf8, &newLocation); rc = gVBoxAPI.UIVirtualBox.CreateHardDisk(data->vboxObj, @@ -7159,15 +7159,15 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } } } else { - for (it = 0; it < def->dom->ndisks; it++) { + for (it = 0; it < def->common.dom->ndisks; it++) { const char *uuidRO = NULL; char *tmp = NULL; uuidRO = virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc, - def->dom->disks[it]->src->path); + def->common.dom->disks[it]->src->path); if (!uuidRO) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No such disk in media registry %s"), - def->dom->disks[it]->src->path); + def->common.dom->disks[it]->src->path); goto cleanup; } @@ -7212,14 +7212,14 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } } /*If the parent snapshot is not NULL, we remove the-read only disks from the media registry*/ - if (def->parent != NULL) { - for (it = 0; it < def->dom->ndisks; it++) { + if (def->common.parent != NULL) { + for (it = 0; it < def->common.dom->ndisks; it++) { const char *uuidRO = virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc, - def->dom->disks[it]->src->path); + def->common.dom->disks[it]->src->path); if (!uuidRO) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find UUID for location %s"), def->dom->disks[it]->src->path); + _("Unable to find UUID for location %s"), def->common.dom->disks[it]->src->path); goto cleanup; } if (virVBoxSnapshotConfRemoveHardDisk(snapshotMachineDesc->mediaRegistry, uuidRO) < 0) { @@ -7284,16 +7284,16 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } /*removing the snapshot*/ - if (virVBoxSnapshotConfRemoveSnapshot(snapshotMachineDesc, def->name) < 0) { + if (virVBoxSnapshotConfRemoveSnapshot(snapshotMachineDesc, def->common.name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to remove snapshot %s"), def->name); + _("Unable to remove snapshot %s"), def->common.name); goto cleanup; } if (isCurrent) { VIR_FREE(snapshotMachineDesc->currentSnapshot); - if (def->parent != NULL) { - virVBoxSnapshotConfSnapshotPtr snap = virVBoxSnapshotConfSnapshotByName(snapshotMachineDesc->snapshot, def->parent); + if (def->common.parent != NULL) { + virVBoxSnapshotConfSnapshotPtr snap = virVBoxSnapshotConfSnapshotByName(snapshotMachineDesc->snapshot, def->common.parent); if (!snap) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to get the snapshot to remove")); -- 2.20.1

Another step towards making the object list reusable for both snapshots and checkpoints: the list code only ever needs items that are in the common virDomainMomentDef base type. This undoes a lot of the churn in accessing common members added the earlier patch. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/virdomainsnapshotobj.h | 4 +- src/conf/virdomainsnapshotobj.c | 2 +- src/conf/virdomainsnapshotobjlist.c | 19 +++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 16 ++--- src/qemu/qemu_driver.c | 107 +++++++++++++++------------- src/test/test_driver.c | 52 +++++++------- 7 files changed, 105 insertions(+), 97 deletions(-) diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainsnapshotobj.h index 8f96bfded9..ed2884e976 100644 --- a/src/conf/virdomainsnapshotobj.h +++ b/src/conf/virdomainsnapshotobj.h @@ -28,7 +28,7 @@ # include "virhash.h" struct _virDomainSnapshotObj { - virDomainSnapshotDefPtr def; /* non-NULL except for metaroot */ + virDomainMomentDefPtr def; /* non-NULL except for metaroot */ virDomainSnapshotObjPtr parent; /* non-NULL except for metaroot, before virDomainSnapshotUpdateRelations, or @@ -55,7 +55,7 @@ void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, static inline virDomainSnapshotDefPtr virDomainSnapshotObjGetDef(virDomainSnapshotObjPtr obj) { - return obj->def; + return (virDomainSnapshotDefPtr) obj->def; } #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */ diff --git a/src/conf/virdomainsnapshotobj.c b/src/conf/virdomainsnapshotobj.c index 0e3ee012fc..d6b216c7b2 100644 --- a/src/conf/virdomainsnapshotobj.c +++ b/src/conf/virdomainsnapshotobj.c @@ -45,7 +45,7 @@ virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot, while (child) { virDomainSnapshotObjPtr next = child->sibling; - (iter)(child, child->def->common.name, data); + (iter)(child, child->def->name, data); child = next; } diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index 58f094fa02..a55448c887 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -249,9 +249,9 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s if (!(snap = virDomainSnapshotObjNew())) return NULL; - snap->def = def; + snap->def = &def->common; - if (virHashAddEntry(snapshots->objs, snap->def->common.name, snap) < 0) { + if (virHashAddEntry(snapshots->objs, snap->def->name, snap) < 0) { VIR_FREE(snap); return NULL; } @@ -354,7 +354,7 @@ static int virDomainSnapshotObjListCopyNames(void *payload, return 0; if (data->names && data->count < data->maxnames && - VIR_STRDUP(data->names[data->count], obj->def->common.name) < 0) { + VIR_STRDUP(data->names[data->count], obj->def->name) < 0) { data->error = true; return 0; } @@ -475,7 +475,7 @@ const char * virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots) { if (snapshots->current) - return snapshots->current->def->common.name; + return snapshots->current->def->name; return NULL; } @@ -485,8 +485,7 @@ bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, const char *name) { - return snapshots->current && STREQ(snapshots->current->def->common.name, - name); + return snapshots->current && STREQ(snapshots->current->def->name, name); } @@ -504,7 +503,7 @@ bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot) { bool ret = snapshots->current == snapshot; - virHashRemoveEntry(snapshots->objs, snapshot->def->common.name); + virHashRemoveEntry(snapshots->objs, snapshot->def->name); if (ret) snapshots->current = NULL; return ret; @@ -548,18 +547,18 @@ virDomainSnapshotSetRelations(void *payload, virDomainSnapshotObjPtr tmp; virDomainSnapshotObjPtr parent; - parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->common.parent); + parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->parent); if (!parent) { curr->err = -1; parent = &curr->snapshots->metaroot; - VIR_WARN("snapshot %s lacks parent", obj->def->common.name); + VIR_WARN("snapshot %s lacks parent", obj->def->name); } else { tmp = parent; while (tmp && tmp->def) { if (tmp == obj) { curr->err = -1; parent = &curr->snapshots->metaroot; - VIR_WARN("snapshot %s in circular chain", obj->def->common.name); + VIR_WARN("snapshot %s in circular chain", obj->def->name); break; } tmp = tmp->parent; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 000da64d0e..bc02362495 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10859,7 +10859,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, goto error; if (snapshot) - virCommandAddArgList(cmd, "-loadvm", snapshot->def->common.name, NULL); + virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); if (def->namespaceData) { qemuDomainCmdlineDefPtr qemucmd; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8e02f05c3c..3219ac5e48 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8573,11 +8573,11 @@ qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver, /* Prefer action on the disks in use at the time the snapshot was * created; but fall back to current definition if dealing with a * snapshot created prior to libvirt 0.9.5. */ - virDomainDefPtr def = snap->def->common.dom; + virDomainDefPtr def = snap->def->dom; if (!def) def = vm->def; - return qemuDomainSnapshotForEachQcow2Raw(driver, def, snap->def->common.name, + return qemuDomainSnapshotForEachQcow2Raw(driver, def, snap->def->name, op, try_all, def->ndisks); } @@ -8605,30 +8605,30 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); /* we continue on even in the face of error */ - qemuMonitorDeleteSnapshot(priv->mon, snap->def->common.name); + qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); ignore_value(qemuDomainObjExitMonitor(driver, vm)); } } if (virAsprintf(&snapFile, "%s/%s/%s.xml", cfg->snapshotDir, - vm->def->name, snap->def->common.name) < 0) + vm->def->name, snap->def->name) < 0) goto cleanup; if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { virDomainSnapshotSetCurrent(vm->snapshots, NULL); - if (update_parent && snap->def->common.parent) { + if (update_parent && snap->def->parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, - snap->def->common.parent); + snap->def->parent); if (!parentsnap) { VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->common.parent); + snap->def->parent); } else { virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) { VIR_WARN("failed to set parent snapshot '%s' as current", - snap->def->common.parent); + snap->def->parent); virDomainSnapshotSetCurrent(vm->snapshots, NULL); } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ece9c9329e..9ad7be5718 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14535,19 +14535,20 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); int ret = -1; virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); if (!(qemuImgPath = qemuFindQemuImgBinary(driver))) goto cleanup; - if (!(created = virBitmapNew(snap->def->ndisks))) + if (!(created = virBitmapNew(snapdef->ndisks))) goto cleanup; /* If reuse is true, then qemuDomainSnapshotPrepare already * ensured that the new files exist, and it was up to the user to * create them correctly. */ - for (i = 0; i < snap->def->ndisks && !reuse; i++) { - snapdisk = &(snap->def->disks[i]); - defdisk = snap->def->common.dom->disks[snapdisk->idx]; + for (i = 0; i < snapdef->ndisks && !reuse; i++) { + snapdisk = &(snapdef->disks[i]); + defdisk = snapdef->common.dom->disks[snapdisk->idx]; if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) continue; @@ -14585,8 +14586,8 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, } /* update disk definitions */ - for (i = 0; i < snap->def->ndisks; i++) { - snapdisk = &(snap->def->disks[i]); + for (i = 0; i < snapdef->ndisks; i++) { + snapdisk = &(snapdef->disks[i]); defdisk = vm->def->disks[snapdisk->idx]; if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { @@ -14612,7 +14613,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, if (ret < 0 && created) { ssize_t bit = -1; while ((bit = virBitmapNextSetBit(created, bit)) >= 0) { - snapdisk = &(snap->def->disks[bit]); + snapdisk = &(snapdef->disks[bit]); if (unlink(snapdisk->src->path) < 0) VIR_WARN("Failed to remove snapshot image '%s'", snapdisk->src->path); @@ -14635,6 +14636,7 @@ qemuDomainSnapshotCreateActiveInternal(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virObjectEventPtr event = NULL; bool resume = false; + virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); int ret = -1; if (!qemuMigrationSrcIsAllowed(driver, vm, false, 0)) @@ -14663,13 +14665,13 @@ qemuDomainSnapshotCreateActiveInternal(virQEMUDriverPtr driver, goto cleanup; } - ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->common.name); + ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; if (ret < 0) goto cleanup; - if (!(snap->def->cookie = (virObjectPtr) qemuDomainSaveCookieNew(vm))) + if (!(snapdef->cookie = (virObjectPtr) qemuDomainSaveCookieNew(vm))) goto cleanup; if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) { @@ -15181,19 +15183,20 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, qemuDomainSnapshotDiskDataPtr ret; qemuDomainSnapshotDiskDataPtr dd; char *backingStoreStr; + virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); - if (VIR_ALLOC_N(ret, snap->def->ndisks) < 0) + if (VIR_ALLOC_N(ret, snapdef->ndisks) < 0) return NULL; - for (i = 0; i < snap->def->ndisks; i++) { - if (snap->def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) + for (i = 0; i < snapdef->ndisks; i++) { + if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) continue; dd = ret + i; dd->disk = vm->def->disks[i]; - if (!(dd->src = virStorageSourceCopy(snap->def->disks[i].src, false))) + if (!(dd->src = virStorageSourceCopy(snapdef->disks[i].src, false))) goto error; if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 0) @@ -15237,7 +15240,7 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, return ret; error: - qemuDomainSnapshotDiskDataFree(ret, snap->def->ndisks, driver, vm); + qemuDomainSnapshotDiskDataFree(ret, snapdef->ndisks, driver, vm); return NULL; } @@ -15344,6 +15347,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = NULL; qemuDomainSnapshotDiskDataPtr diskdata = NULL; virErrorPtr orig_err = NULL; + virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); if (virDomainObjCheckActive(vm) < 0) return -1; @@ -15362,7 +15366,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * now either VIR_DOMAIN_SNAPSHOT_LOCATION_NONE, or * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and * qcow2 format. */ - for (i = 0; i < snap->def->ndisks; i++) { + for (i = 0; i < snapdef->ndisks; i++) { if (!diskdata[i].src) continue; @@ -15385,7 +15389,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; - for (i = 0; i < snap->def->ndisks; i++) { + for (i = 0; i < snapdef->ndisks; i++) { qemuDomainSnapshotDiskDataPtr dd = &diskdata[i]; if (!dd->src) @@ -15404,7 +15408,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, error: if (ret < 0) { orig_err = virSaveLastError(); - for (i = 0; i < snap->def->ndisks; i++) { + for (i = 0; i < snapdef->ndisks; i++) { if (!diskdata[i].src) continue; @@ -15421,7 +15425,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * stopped using them*/ bool paused = virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING; - for (i = 0; i < snap->def->ndisks; i++) { + for (i = 0; i < snapdef->ndisks; i++) { if (!diskdata[i].disk) continue; @@ -15442,7 +15446,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } cleanup: - qemuDomainSnapshotDiskDataFree(diskdata, snap->def->ndisks, driver, vm); + qemuDomainSnapshotDiskDataFree(diskdata, snapdef->ndisks, driver, vm); virJSONValueFree(actions); virObjectUnref(cfg); @@ -15466,7 +15470,8 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; char *xml = NULL; - bool memory = snap->def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); + bool memory = snapdef->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; bool memory_unlink = false; int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */ bool pmsuspended = false; @@ -15557,16 +15562,16 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU, true, true)) || - !(snap->def->cookie = (virObjectPtr) qemuDomainSaveCookieNew(vm))) + !(snapdef->cookie = (virObjectPtr) qemuDomainSaveCookieNew(vm))) goto cleanup; if (!(data = virQEMUSaveDataNew(xml, - (qemuDomainSaveCookiePtr) snap->def->cookie, + (qemuDomainSaveCookiePtr) snapdef->cookie, resume, compressed, driver->xmlopt))) goto cleanup; xml = NULL; - if ((ret = qemuDomainSaveMemory(driver, vm, snap->def->file, data, + if ((ret = qemuDomainSaveMemory(driver, vm, snapdef->file, data, compressedpath, 0, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; @@ -15641,7 +15646,7 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, VIR_FREE(compressedpath); virObjectUnref(cfg); if (memory_unlink && ret < 0) - unlink(snap->def->file); + unlink(snapdef->file); return ret; } @@ -15857,7 +15862,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, current = virDomainSnapshotGetCurrent(vm->snapshots); if (current) { if (!redefine && - VIR_STRDUP(snap->def->common.parent, current->def->common.name) < 0) + VIR_STRDUP(snap->def->parent, current->def->name) < 0) goto endjob; if (update_current) { virDomainSnapshotSetCurrent(vm->snapshots, NULL); @@ -15875,7 +15880,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * snapshot name in at least one of the domain's disks? */ } else if (virDomainObjIsActive(vm)) { if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY || - snap->def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { + virDomainSnapshotObjGetDef(snap)->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { /* external full system or disk snapshot */ if (qemuDomainSnapshotCreateActiveExternal(driver, vm, snap, flags) < 0) @@ -15906,7 +15911,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * do; we've successfully taken the snapshot, and we are now running * on it, so we have to go forward the best we can */ - snapshot = virGetDomainSnapshot(domain, snap->def->common.name); + snapshot = virGetDomainSnapshot(domain, snap->def->name); endjob: if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) { @@ -15921,11 +15926,11 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, snapshot = NULL; virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to save metadata for snapshot %s"), - snap->def->common.name); + snap->def->name); virDomainSnapshotObjListRemove(vm->snapshots, snap); } else { other = virDomainSnapshotFindByName(vm->snapshots, - snap->def->common.parent); + snap->def->parent); virDomainSnapshotSetParent(snap, other); } } else if (snap) { @@ -16135,7 +16140,7 @@ qemuDomainSnapshotLookupByName(virDomainPtr domain, if (!(snap = qemuSnapObjFromName(vm, name))) goto cleanup; - snapshot = virGetDomainSnapshot(domain, snap->def->common.name); + snapshot = virGetDomainSnapshot(domain, snap->def->name); cleanup: virDomainObjEndAPI(&vm); @@ -16185,14 +16190,14 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - if (!snap->def->common.parent) { + if (!snap->def->parent) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, _("snapshot '%s' does not have a parent"), - snap->def->common.name); + snap->def->name); goto cleanup; } - parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->common.parent); + parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->parent); cleanup: virDomainObjEndAPI(&vm); @@ -16252,7 +16257,7 @@ qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, virUUIDFormat(virSnapDom(snapshot)->uuid, uuidstr); - xml = virDomainSnapshotDefFormat(uuidstr, snap->def, + xml = virDomainSnapshotDefFormat(uuidstr, virDomainSnapshotObjGetDef(snap), driver->caps, driver->xmlopt, virDomainSnapshotFormatConvertXMLFlags(flags)); @@ -16340,6 +16345,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, int ret = -1; virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotObjPtr current = NULL; + virDomainSnapshotDefPtr snapdef; virObjectEventPtr event = NULL; virObjectEventPtr event2 = NULL; int detail; @@ -16398,10 +16404,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto endjob; + snapdef = virDomainSnapshotObjGetDef(snap); if (!vm->persistent && - snap->def->state != VIR_DOMAIN_SNAPSHOT_RUNNING && - snap->def->state != VIR_DOMAIN_SNAPSHOT_PAUSED && + snapdef->state != VIR_DOMAIN_SNAPSHOT_RUNNING && + snapdef->state != VIR_DOMAIN_SNAPSHOT_PAUSED && (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -16417,15 +16424,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { - if (!snap->def->common.dom) { + if (!snap->def->dom) { virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, _("snapshot '%s' lacks domain '%s' rollback info"), - snap->def->common.name, vm->def->name); + snap->def->name, vm->def->name); goto endjob; } if (virDomainObjIsActive(vm) && - !(snap->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING || - snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) && + !(snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING || + snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) && (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s", @@ -16450,16 +16457,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * * XXX Should domain snapshots track live xml rather * than inactive xml? */ - if (snap->def->common.dom) { - config = virDomainDefCopy(snap->def->common.dom, caps, + if (snap->def->dom) { + config = virDomainDefCopy(snap->def->dom, caps, driver->xmlopt, NULL, true); if (!config) goto endjob; } - cookie = (qemuDomainSaveCookiePtr) snap->def->cookie; + cookie = (qemuDomainSaveCookiePtr) snapdef->cookie; - switch ((virDomainSnapshotState) snap->def->state) { + switch ((virDomainSnapshotState) snapdef->state) { case VIR_DOMAIN_SNAPSHOT_RUNNING: case VIR_DOMAIN_SNAPSHOT_PAUSED: @@ -16557,7 +16564,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_START) < 0) goto endjob; - rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->common.name); + rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; if (rc < 0) { @@ -16600,7 +16607,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Touch up domain state. */ if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) && - (snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED || + (snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED || (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { /* Transitions 3, 6, 9 */ virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, @@ -16709,7 +16716,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid target domain state '%s'. Refusing " "snapshot reversion"), - virDomainSnapshotStateTypeToString(snap->def->state)); + virDomainSnapshotStateTypeToString(snapdef->state)); goto endjob; } @@ -16774,10 +16781,10 @@ qemuDomainSnapshotReparentChildren(void *payload, if (rep->err < 0) return 0; - VIR_FREE(snap->def->common.parent); + VIR_FREE(snap->def->parent); if (rep->parent->def && - VIR_STRDUP(snap->def->common.parent, rep->parent->def->common.name) < 0) { + VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { rep->err = -1; return 0; } @@ -16858,7 +16865,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, cfg->snapshotDir) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to set snapshot '%s' as current"), - snap->def->common.name); + snap->def->name); virDomainSnapshotSetCurrent(vm->snapshots, NULL); goto endjob; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0caed1de19..eac6160532 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6132,7 +6132,7 @@ testDomainSnapshotLookupByName(virDomainPtr domain, if (!(snap = testSnapObjFromName(vm, name))) goto cleanup; - snapshot = virGetDomainSnapshot(domain, snap->def->common.name); + snapshot = virGetDomainSnapshot(domain, snap->def->name); cleanup: virDomainObjEndAPI(&vm); @@ -6173,14 +6173,14 @@ testDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - if (!snap->def->common.parent) { + if (!snap->def->parent) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, _("snapshot '%s' does not have a parent"), - snap->def->common.name); + snap->def->name); goto cleanup; } - parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->common.parent); + parent = virGetDomainSnapshot(virSnapDom(snapshot), snap->def->parent); cleanup: virDomainObjEndAPI(&vm); @@ -6207,7 +6207,7 @@ testDomainSnapshotCurrent(virDomainPtr domain, goto cleanup; } - snapshot = virGetDomainSnapshot(domain, current->def->common.name); + snapshot = virGetDomainSnapshot(domain, current->def->name); cleanup: virDomainObjEndAPI(&vm); @@ -6234,8 +6234,8 @@ testDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, virUUIDFormat(virSnapDom(snapshot)->uuid, uuidstr); - xml = virDomainSnapshotDefFormat(uuidstr, snap->def, privconn->caps, - privconn->xmlopt, + xml = virDomainSnapshotDefFormat(uuidstr, virDomainSnapshotObjGetDef(snap), + privconn->caps, privconn->xmlopt, virDomainSnapshotFormatConvertXMLFlags(flags)); cleanup: @@ -6394,7 +6394,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, } if (!redefine) { - if (VIR_STRDUP(snap->def->common.parent, + if (VIR_STRDUP(snap->def->parent, virDomainSnapshotGetCurrentName(vm->snapshots)) < 0) goto cleanup; @@ -6407,7 +6407,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, } } - snapshot = virGetDomainSnapshot(domain, snap->def->common.name); + snapshot = virGetDomainSnapshot(domain, snap->def->name); cleanup: if (vm) { if (snapshot) { @@ -6415,7 +6415,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, if (update_current) virDomainSnapshotSetCurrent(vm->snapshots, snap); other = virDomainSnapshotFindByName(vm->snapshots, - snap->def->common.parent); + snap->def->parent); virDomainSnapshotSetParent(snap, other); } virDomainObjEndAPI(&vm); @@ -6464,10 +6464,10 @@ testDomainSnapshotReparentChildren(void *payload, if (rep->err < 0) return 0; - VIR_FREE(snap->def->common.parent); + VIR_FREE(snap->def->parent); if (rep->parent->def && - VIR_STRDUP(snap->def->common.parent, rep->parent->def->common.name) < 0) { + VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { rep->err = -1; return 0; } @@ -6522,12 +6522,12 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, } else { virDomainSnapshotDropParent(snap); if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { - if (snap->def->common.parent) { + if (snap->def->parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, - snap->def->common.parent); + snap->def->parent); if (!parentsnap) VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->common.parent); + snap->def->parent); } virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); } @@ -6550,6 +6550,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virObjectEventPtr event = NULL; virObjectEventPtr event2 = NULL; virDomainDefPtr config = NULL; + virDomainSnapshotDefPtr snapdef; int ret = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | @@ -6575,10 +6576,11 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) goto cleanup; + snapdef = virDomainSnapshotObjGetDef(snap); if (!vm->persistent && - snap->def->state != VIR_DOMAIN_SNAPSHOT_RUNNING && - snap->def->state != VIR_DOMAIN_SNAPSHOT_PAUSED && + snapdef->state != VIR_DOMAIN_SNAPSHOT_RUNNING && + snapdef->state != VIR_DOMAIN_SNAPSHOT_PAUSED && (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -6588,15 +6590,15 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { - if (!snap->def->common.dom) { + if (!snap->def->dom) { virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, _("snapshot '%s' lacks domain '%s' rollback info"), - snap->def->common.name, vm->def->name); + snap->def->name, vm->def->name); goto cleanup; } if (virDomainObjIsActive(vm) && - !(snap->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING || - snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) && + !(snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING || + snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) && (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s", @@ -6607,13 +6609,13 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainSnapshotSetCurrent(vm->snapshots, NULL); - config = virDomainDefCopy(snap->def->common.dom, privconn->caps, + config = virDomainDefCopy(snap->def->dom, privconn->caps, privconn->xmlopt, NULL, true); if (!config) goto cleanup; - if (snap->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING || - snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) { + if (snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING || + snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) { /* Transitions 2, 3, 5, 6, 8, 9 */ bool was_running = false; bool was_stopped = false; @@ -6672,7 +6674,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Touch up domain state. */ if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) && - (snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED || + (snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED || (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { /* Transitions 3, 6, 9 */ virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, -- 2.20.1

Now that the core of SnapshotObj is agnostic to snapshots and can be shared with upcoming checkpoint code, it is time to rename the struct and the functions specific to list operations. A later patch will shuffle which file holds the common code. This is a fairly mechanical patch. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/snapshot_conf.h | 6 +- src/conf/virconftypes.h | 6 +- src/conf/virdomainsnapshotobj.h | 38 +++++------ src/conf/virdomainsnapshotobjlist.h | 26 ++++---- src/qemu/qemu_command.h | 2 +- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_process.h | 4 +- src/conf/snapshot_conf.c | 10 +-- src/conf/virdomainsnapshotobj.c | 96 +++++++++++++------------- src/conf/virdomainsnapshotobjlist.c | 100 ++++++++++++++-------------- src/libvirt_private.syms | 12 ++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 12 ++-- src/qemu/qemu_driver.c | 82 +++++++++++------------ src/qemu/qemu_process.c | 4 +- src/test/test_driver.c | 58 ++++++++-------- src/vz/vz_driver.c | 34 +++++----- src/vz/vz_sdk.c | 4 +- 18 files changed, 251 insertions(+), 251 deletions(-) diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 4b777e94dc..20487454c9 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -154,19 +154,19 @@ int virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapshot, VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION) bool virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def); -bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap); +bool virDomainSnapshotIsExternal(virDomainMomentObjPtr snap); int virDomainSnapshotRedefinePrep(virDomainPtr domain, virDomainObjPtr vm, virDomainSnapshotDefPtr *def, - virDomainSnapshotObjPtr *snap, + virDomainMomentObjPtr *snap, virDomainXMLOptionPtr xmlopt, bool *update_current, unsigned int flags); int virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, const unsigned char *domain_uuid, - virDomainSnapshotObjPtr other, + virDomainMomentObjPtr other, virDomainXMLOptionPtr xmlopt, unsigned int flags); diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 9b9ab314e7..574815cf04 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -220,6 +220,9 @@ typedef virDomainMemtune *virDomainMemtunePtr; typedef struct _virDomainMomentDef virDomainMomentDef; typedef virDomainMomentDef *virDomainMomentDefPtr; +typedef struct _virDomainMomentObj virDomainMomentObj; +typedef virDomainMomentObj *virDomainMomentObjPtr; + typedef struct _virDomainNVRAMDef virDomainNVRAMDef; typedef virDomainNVRAMDef *virDomainNVRAMDefPtr; @@ -280,9 +283,6 @@ typedef virDomainSmartcardDef *virDomainSmartcardDefPtr; typedef struct _virDomainSnapshotDef virDomainSnapshotDef; typedef virDomainSnapshotDef *virDomainSnapshotDefPtr; -typedef struct _virDomainSnapshotObj virDomainSnapshotObj; -typedef virDomainSnapshotObj *virDomainSnapshotObjPtr; - typedef struct _virDomainSnapshotObjList virDomainSnapshotObjList; typedef virDomainSnapshotObjList *virDomainSnapshotObjListPtr; diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainsnapshotobj.h index ed2884e976..86c5f6860b 100644 --- a/src/conf/virdomainsnapshotobj.h +++ b/src/conf/virdomainsnapshotobj.h @@ -27,33 +27,33 @@ # include "virconftypes.h" # include "virhash.h" -struct _virDomainSnapshotObj { +struct _virDomainMomentObj { virDomainMomentDefPtr def; /* non-NULL except for metaroot */ - virDomainSnapshotObjPtr parent; /* non-NULL except for metaroot, before - virDomainSnapshotUpdateRelations, or - after virDomainSnapshotDropParent */ - virDomainSnapshotObjPtr sibling; /* NULL if last child of parent */ + virDomainMomentObjPtr parent; /* non-NULL except for metaroot, before + virDomainSnapshotUpdateRelations, or + after virDomainMomentDropParent */ + virDomainMomentObjPtr sibling; /* NULL if last child of parent */ size_t nchildren; - virDomainSnapshotObjPtr first_child; /* NULL if no children */ + virDomainMomentObjPtr first_child; /* NULL if no children */ }; -int virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot, - virHashIterator iter, - void *data); -int virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot, - virHashIterator iter, - void *data); -void virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot); -void virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot); -void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, - virDomainSnapshotObjPtr to); -void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, - virDomainSnapshotObjPtr parent); +int virDomainMomentForEachChild(virDomainMomentObjPtr moment, + virHashIterator iter, + void *data); +int virDomainMomentForEachDescendant(virDomainMomentObjPtr moment, + virHashIterator iter, + void *data); +void virDomainMomentDropParent(virDomainMomentObjPtr moment); +void virDomainMomentDropChildren(virDomainMomentObjPtr moment); +void virDomainMomentMoveChildren(virDomainMomentObjPtr from, + virDomainMomentObjPtr to); +void virDomainMomentSetParent(virDomainMomentObjPtr moment, + virDomainMomentObjPtr parent); static inline virDomainSnapshotDefPtr -virDomainSnapshotObjGetDef(virDomainSnapshotObjPtr obj) +virDomainSnapshotObjGetDef(virDomainMomentObjPtr obj) { return (virDomainSnapshotDefPtr) obj->def; } diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h index c7d4d265cb..1af367639d 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -27,9 +27,9 @@ # include "virdomainsnapshotobj.h" # include "virbuffer.h" -/* Filter that returns true if a given snapshot matches the filter flags */ -typedef bool (*virDomainSnapshotObjListFilter)(virDomainSnapshotObjPtr obj, - unsigned int flags); +/* Filter that returns true if a given moment matches the filter flags */ +typedef bool (*virDomainMomentObjListFilter)(virDomainMomentObjPtr obj, + unsigned int flags); virDomainSnapshotObjListPtr virDomainSnapshotObjListNew(void); void virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots); @@ -47,27 +47,27 @@ int virDomainSnapshotObjListFormat(virBufferPtr buf, virDomainXMLOptionPtr xmlopt, unsigned int flags); -virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotDefPtr def); +virDomainMomentObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotDefPtr def); int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr from, + virDomainMomentObjPtr from, char **const names, int maxnames, unsigned int flags); int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr from, + virDomainMomentObjPtr from, unsigned int flags); -virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, - const char *name); +virDomainMomentObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, + const char *name); int virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots); -virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots); +virDomainMomentObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots); const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr snapshots); bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, const char *name); void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr snapshot); + virDomainMomentObjPtr snapshot); bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr snapshot); + virDomainMomentObjPtr snapshot); void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots); int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, @@ -75,7 +75,7 @@ int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots); int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr from, + virDomainMomentObjPtr from, virDomainPtr dom, virDomainSnapshotPtr **snaps, unsigned int flags); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 077484094d..77578155e6 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -47,7 +47,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, virSecurityManagerPtr secManager, virDomainObjPtr vm, const char *migrateURI, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virNetDevVPortProfileOp vmop, bool standalone, bool enableFips, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index fb361515ba..ca24de15e5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -681,20 +681,20 @@ int qemuDomainLogAppendMessage(virQEMUDriverPtr driver, const char *qemuFindQemuImgBinary(virQEMUDriverPtr driver); int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, const char *snapshotDir); int qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, const char *op, bool try_all); int qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, bool update_current, bool metadata_only); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 3367cd3fe5..d20bd5306e 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -92,7 +92,7 @@ int qemuProcessStart(virConnectPtr conn, const char *migrateFrom, int stdin_fd, const char *stdin_path, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virNetDevVPortProfileOp vmop, unsigned int flags); @@ -125,7 +125,7 @@ int qemuProcessLaunch(virConnectPtr conn, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, qemuProcessIncomingDefPtr incoming, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virNetDevVPortProfileOp vmop, unsigned int flags); diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index b80b199ce4..938b69cf60 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -426,7 +426,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, int virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, const unsigned char *domain_uuid, - virDomainSnapshotObjPtr other, + virDomainMomentObjPtr other, virDomainXMLOptionPtr xmlopt, unsigned int flags) { @@ -907,7 +907,7 @@ virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def) } bool -virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap) +virDomainSnapshotIsExternal(virDomainMomentObjPtr snap) { virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snap); @@ -918,13 +918,13 @@ int virDomainSnapshotRedefinePrep(virDomainPtr domain, virDomainObjPtr vm, virDomainSnapshotDefPtr *defptr, - virDomainSnapshotObjPtr *snap, + virDomainMomentObjPtr *snap, virDomainXMLOptionPtr xmlopt, bool *update_current, unsigned int flags) { virDomainSnapshotDefPtr def = *defptr; - virDomainSnapshotObjPtr other; + virDomainMomentObjPtr other; virDomainSnapshotDefPtr otherdef; bool check_if_stolen; @@ -981,7 +981,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, /* Drop and rebuild the parent relationship, but keep all * child relations by reusing snap. */ - virDomainSnapshotDropParent(other); + virDomainMomentDropParent(other); virDomainSnapshotDefFree(otherdef); otherdef = def; *defptr = NULL; diff --git a/src/conf/virdomainsnapshotobj.c b/src/conf/virdomainsnapshotobj.c index d6b216c7b2..877a0a9079 100644 --- a/src/conf/virdomainsnapshotobj.c +++ b/src/conf/virdomainsnapshotobj.c @@ -33,80 +33,80 @@ VIR_LOG_INIT("conf.virdomainsnapshotobj"); -/* Run iter(data) on all direct children of snapshot, while ignoring all - * other entries in snapshots. Return the number of children +/* Run iter(data) on all direct children of moment, while ignoring all + * other entries in moments. Return the number of children * visited. No particular ordering is guaranteed. */ int -virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot, - virHashIterator iter, - void *data) +virDomainMomentForEachChild(virDomainMomentObjPtr moment, + virHashIterator iter, + void *data) { - virDomainSnapshotObjPtr child = snapshot->first_child; + virDomainMomentObjPtr child = moment->first_child; while (child) { - virDomainSnapshotObjPtr next = child->sibling; + virDomainMomentObjPtr next = child->sibling; (iter)(child, child->def->name, data); child = next; } - return snapshot->nchildren; + return moment->nchildren; } -struct snapshot_act_on_descendant { +struct moment_act_on_descendant { int number; virHashIterator iter; void *data; }; static int -virDomainSnapshotActOnDescendant(void *payload, - const void *name, - void *data) +virDomainMomentActOnDescendant(void *payload, + const void *name, + void *data) { - virDomainSnapshotObjPtr obj = payload; - struct snapshot_act_on_descendant *curr = data; + virDomainMomentObjPtr obj = payload; + struct moment_act_on_descendant *curr = data; (curr->iter)(payload, name, curr->data); - curr->number += 1 + virDomainSnapshotForEachDescendant(obj, + curr->number += 1 + virDomainMomentForEachDescendant(obj, curr->iter, curr->data); return 0; } -/* Run iter(data) on all descendants of snapshot, while ignoring all - * other entries in snapshots. Return the number of descendants +/* Run iter(data) on all descendants of moment, while ignoring all + * other entries in moments. Return the number of descendants * visited. The visit is guaranteed to be topological, but no * particular order between siblings is guaranteed. */ int -virDomainSnapshotForEachDescendant(virDomainSnapshotObjPtr snapshot, - virHashIterator iter, - void *data) +virDomainMomentForEachDescendant(virDomainMomentObjPtr moment, + virHashIterator iter, + void *data) { - struct snapshot_act_on_descendant act; + struct moment_act_on_descendant act; act.number = 0; act.iter = iter; act.data = data; - virDomainSnapshotForEachChild(snapshot, - virDomainSnapshotActOnDescendant, &act); + virDomainMomentForEachChild(moment, + virDomainMomentActOnDescendant, &act); return act.number; } -/* Prepare to reparent or delete snapshot, by removing it from its +/* Prepare to reparent or delete moment, by removing it from its * current listed parent. Note that when bulk removing all children * of a parent, it is faster to just 0 the count rather than calling * this function on each child. */ void -virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot) +virDomainMomentDropParent(virDomainMomentObjPtr moment) { - virDomainSnapshotObjPtr prev = NULL; - virDomainSnapshotObjPtr curr = NULL; + virDomainMomentObjPtr prev = NULL; + virDomainMomentObjPtr curr = NULL; - snapshot->parent->nchildren--; - curr = snapshot->parent->first_child; - while (curr != snapshot) { + moment->parent->nchildren--; + curr = moment->parent->first_child; + while (curr != moment) { if (!curr) { VIR_WARN("inconsistent snapshot relations"); return; @@ -115,42 +115,42 @@ virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot) curr = curr->sibling; } if (prev) - prev->sibling = snapshot->sibling; + prev->sibling = moment->sibling; else - snapshot->parent->first_child = snapshot->sibling; - snapshot->parent = NULL; - snapshot->sibling = NULL; + moment->parent->first_child = moment->sibling; + moment->parent = NULL; + moment->sibling = NULL; } -/* Update @snapshot to no longer have children. */ +/* Update @moment to no longer have children. */ void -virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot) +virDomainMomentDropChildren(virDomainMomentObjPtr moment) { - snapshot->nchildren = 0; - snapshot->first_child = NULL; + moment->nchildren = 0; + moment->first_child = NULL; } -/* Add @snapshot to @parent's list of children. */ +/* Add @moment to @parent's list of children. */ void -virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, - virDomainSnapshotObjPtr parent) +virDomainMomentSetParent(virDomainMomentObjPtr moment, + virDomainMomentObjPtr parent) { - snapshot->parent = parent; + moment->parent = parent; parent->nchildren++; - snapshot->sibling = parent->first_child; - parent->first_child = snapshot; + moment->sibling = parent->first_child; + parent->first_child = moment; } /* Take all children of @from and convert them into children of @to. */ void -virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, - virDomainSnapshotObjPtr to) +virDomainMomentMoveChildren(virDomainMomentObjPtr from, + virDomainMomentObjPtr to) { - virDomainSnapshotObjPtr child; - virDomainSnapshotObjPtr last; + virDomainMomentObjPtr child; + virDomainMomentObjPtr last; for (child = from->first_child; child; child = child->sibling) { child->parent = to; diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index a55448c887..8ecb131176 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -35,12 +35,12 @@ VIR_LOG_INIT("conf.virdomainsnapshotobjlist"); struct _virDomainSnapshotObjList { - /* name string -> virDomainSnapshotObj mapping + /* name string -> virDomainMomentObj mapping * for O(1), lockless lookup-by-name */ virHashTable *objs; - virDomainSnapshotObj metaroot; /* Special parent of all root snapshots */ - virDomainSnapshotObjPtr current; /* The current snapshot, if any */ + virDomainMomentObj metaroot; /* Special parent of all root snapshots */ + virDomainMomentObjPtr current; /* The current snapshot, if any */ }; @@ -62,7 +62,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, int n; size_t i; int keepBlanksDefault = xmlKeepBlanksDefault(0); - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; VIR_AUTOFREE(char *) current = NULL; @@ -168,7 +168,7 @@ virDomainSnapshotFormatOne(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) { - virDomainSnapshotObjPtr snap = payload; + virDomainMomentObjPtr snap = payload; struct virDomainSnapshotFormatData *data = opaque; return virDomainSnapshotDefFormatInternal(data->buf, data->uuidstr, virDomainSnapshotObjGetDef(snap), @@ -212,9 +212,9 @@ virDomainSnapshotObjListFormat(virBufferPtr buf, /* Snapshot Obj functions */ -static virDomainSnapshotObjPtr virDomainSnapshotObjNew(void) +static virDomainMomentObjPtr virDomainMomentObjNew(void) { - virDomainSnapshotObjPtr snapshot; + virDomainMomentObjPtr snapshot; if (VIR_ALLOC(snapshot) < 0) return NULL; @@ -224,7 +224,7 @@ static virDomainSnapshotObjPtr virDomainSnapshotObjNew(void) return snapshot; } -static void virDomainSnapshotObjFree(virDomainSnapshotObjPtr snapshot) +static void virDomainMomentObjFree(virDomainMomentObjPtr snapshot) { if (!snapshot) return; @@ -235,10 +235,10 @@ static void virDomainSnapshotObjFree(virDomainSnapshotObjPtr snapshot) VIR_FREE(snapshot); } -virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotDefPtr def) +virDomainMomentObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotDefPtr def) { - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; if (virHashLookup(snapshots->objs, def->common.name) != NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -247,7 +247,7 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s return NULL; } - if (!(snap = virDomainSnapshotObjNew())) + if (!(snap = virDomainMomentObjNew())) return NULL; snap->def = &def->common; @@ -261,7 +261,7 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s /* Snapshot Obj List functions */ static bool -virDomainSnapshotFilter(virDomainSnapshotObjPtr obj, +virDomainSnapshotFilter(virDomainMomentObjPtr obj, unsigned int flags) { virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(obj); @@ -296,9 +296,9 @@ static void virDomainSnapshotObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { - virDomainSnapshotObjPtr obj = payload; + virDomainMomentObjPtr obj = payload; - virDomainSnapshotObjFree(obj); + virDomainMomentObjFree(obj); } virDomainSnapshotObjListPtr @@ -325,21 +325,21 @@ virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots) } -struct virDomainSnapshotNameData { +struct virDomainMomentNameData { char **const names; int maxnames; unsigned int flags; int count; bool error; - virDomainSnapshotObjListFilter filter; + virDomainMomentObjListFilter filter; }; -static int virDomainSnapshotObjListCopyNames(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *opaque) +static int virDomainMomentObjListCopyNames(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) { - virDomainSnapshotObjPtr obj = payload; - struct virDomainSnapshotNameData *data = opaque; + virDomainMomentObjPtr obj = payload; + struct virDomainMomentNameData *data = opaque; if (data->error) return 0; @@ -364,13 +364,13 @@ static int virDomainSnapshotObjListCopyNames(void *payload, int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr from, + virDomainMomentObjPtr from, char **const names, int maxnames, unsigned int flags) { - struct virDomainSnapshotNameData data = { names, maxnames, flags, 0, - false, virDomainSnapshotFilter }; + struct virDomainMomentNameData data = { names, maxnames, flags, 0, + false, virDomainSnapshotFilter }; size_t i; if (!from) { @@ -414,17 +414,17 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, * simpler full hashtable visit or counter will do. */ if (from->def || (names && (flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL))) - virDomainSnapshotForEachDescendant(from, - virDomainSnapshotObjListCopyNames, - &data); + virDomainMomentForEachDescendant(from, + virDomainMomentObjListCopyNames, + &data); else if (names || data.flags) - virHashForEach(snapshots->objs, virDomainSnapshotObjListCopyNames, + virHashForEach(snapshots->objs, virDomainMomentObjListCopyNames, &data); else data.count = virHashSize(snapshots->objs); } else if (names || data.flags) { - virDomainSnapshotForEachChild(from, - virDomainSnapshotObjListCopyNames, &data); + virDomainMomentForEachChild(from, + virDomainMomentObjListCopyNames, &data); } else { data.count = from->nchildren; } @@ -440,13 +440,13 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr from, + virDomainMomentObjPtr from, unsigned int flags) { return virDomainSnapshotObjListGetNames(snapshots, from, NULL, 0, flags); } -virDomainSnapshotObjPtr +virDomainMomentObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr snapshots, const char *name) { @@ -463,7 +463,7 @@ virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots) /* Return the current snapshot, or NULL */ -virDomainSnapshotObjPtr +virDomainMomentObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots) { return snapshots->current; @@ -492,7 +492,7 @@ virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, /* Update the current snapshot, using NULL if no current remains */ void virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr snapshot) + virDomainMomentObjPtr snapshot) { snapshots->current = snapshot; } @@ -500,7 +500,7 @@ virDomainSnapshotSetCurrent(virDomainSnapshotObjListPtr snapshots, /* Remove snapshot from the list; return true if it was current */ bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr snapshot) + virDomainMomentObjPtr snapshot) { bool ret = snapshots->current == snapshot; virHashRemoveEntry(snapshots->objs, snapshot->def->name); @@ -514,7 +514,7 @@ void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots) { virHashRemoveAll(snapshots->objs); - virDomainSnapshotDropChildren(&snapshots->metaroot); + virDomainMomentDropChildren(&snapshots->metaroot); } @@ -533,19 +533,19 @@ virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, * wire up the hierarchical relations for the given snapshot. The error * indicator gets set if a parent is missing or a requested parent would * cause a circular parent chain. */ -struct snapshot_set_relation { +struct moment_set_relation { virDomainSnapshotObjListPtr snapshots; int err; }; static int -virDomainSnapshotSetRelations(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data) +virDomainMomentSetRelations(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) { - virDomainSnapshotObjPtr obj = payload; - struct snapshot_set_relation *curr = data; - virDomainSnapshotObjPtr tmp; - virDomainSnapshotObjPtr parent; + virDomainMomentObjPtr obj = payload; + struct moment_set_relation *curr = data; + virDomainMomentObjPtr tmp; + virDomainMomentObjPtr parent; parent = virDomainSnapshotFindByName(curr->snapshots, obj->def->parent); if (!parent) { @@ -564,7 +564,7 @@ virDomainSnapshotSetRelations(void *payload, tmp = tmp->parent; } } - virDomainSnapshotSetParent(obj, parent); + virDomainMomentSetParent(obj, parent); return 0; } @@ -575,10 +575,10 @@ virDomainSnapshotSetRelations(void *payload, int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots) { - struct snapshot_set_relation act = { snapshots, 0 }; + struct moment_set_relation act = { snapshots, 0 }; - virDomainSnapshotDropChildren(&snapshots->metaroot); - virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act); + virDomainMomentDropChildren(&snapshots->metaroot); + virHashForEach(snapshots->objs, virDomainMomentSetRelations, &act); if (act.err) snapshots->current = NULL; return act.err; @@ -587,7 +587,7 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots) int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots, - virDomainSnapshotObjPtr from, + virDomainMomentObjPtr from, virDomainPtr dom, virDomainSnapshotPtr **snaps, unsigned int flags) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ffc1724850..02f383fdba 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -980,12 +980,12 @@ virDomainObjListRename; # conf/virdomainsnapshotobj.h -virDomainSnapshotDropChildren; -virDomainSnapshotDropParent; -virDomainSnapshotForEachChild; -virDomainSnapshotForEachDescendant; -virDomainSnapshotMoveChildren; -virDomainSnapshotSetParent; +virDomainMomentDropChildren; +virDomainMomentDropParent; +virDomainMomentForEachChild; +virDomainMomentForEachDescendant; +virDomainMomentMoveChildren; +virDomainMomentSetParent; # conf/virdomainsnapshotobjlist.h diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bc02362495..f81d20e5f7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10632,7 +10632,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, virSecurityManagerPtr secManager, virDomainObjPtr vm, const char *migrateURI, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virNetDevVPortProfileOp vmop, bool standalone, bool enableFips, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3219ac5e48..d4ed772280 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8448,7 +8448,7 @@ qemuFindQemuImgBinary(virQEMUDriverPtr driver) int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, const char *snapshotDir) @@ -8566,7 +8566,7 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriverPtr driver, int qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, const char *op, bool try_all) { @@ -8585,14 +8585,14 @@ qemuDomainSnapshotForEachQcow2(virQEMUDriverPtr driver, int qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, bool update_parent, bool metadata_only) { char *snapFile = NULL; int ret = -1; qemuDomainObjPrivatePtr priv; - virDomainSnapshotObjPtr parentsnap = NULL; + virDomainMomentObjPtr parentsnap = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (!metadata_only) { @@ -8638,7 +8638,7 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, if (unlink(snapFile) < 0) VIR_WARN("Failed to unlink %s", snapFile); if (update_parent) - virDomainSnapshotDropParent(snap); + virDomainMomentDropParent(snap); virDomainSnapshotObjListRemove(vm->snapshots, snap); ret = 0; @@ -8654,7 +8654,7 @@ int qemuDomainSnapshotDiscardAll(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { - virDomainSnapshotObjPtr snap = payload; + virDomainMomentObjPtr snap = payload; virQEMUSnapRemovePtr curr = data; int err; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9ad7be5718..9c2245b095 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -198,11 +198,11 @@ qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot) /* Looks up snapshot object from VM and name */ -static virDomainSnapshotObjPtr +static virDomainMomentObjPtr qemuSnapObjFromName(virDomainObjPtr vm, const char *name) { - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; snap = virDomainSnapshotFindByName(vm->snapshots, name); if (!snap) virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -214,7 +214,7 @@ qemuSnapObjFromName(virDomainObjPtr vm, /* Looks up snapshot object from VM and snapshotPtr */ -static virDomainSnapshotObjPtr +static virDomainMomentObjPtr qemuSnapObjFromSnapshot(virDomainObjPtr vm, virDomainSnapshotPtr snapshot) { @@ -417,8 +417,8 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, char *xmlStr; char *fullpath; virDomainSnapshotDefPtr def = NULL; - virDomainSnapshotObjPtr snap = NULL; - virDomainSnapshotObjPtr current = NULL; + virDomainMomentObjPtr snap = NULL; + virDomainMomentObjPtr current = NULL; bool cur; unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE | VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | @@ -2140,7 +2140,7 @@ qemuDomainSnapshotCountExternal(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { - virDomainSnapshotObjPtr snap = payload; + virDomainMomentObjPtr snap = payload; int *count = data; if (virDomainSnapshotIsExternal(snap)) @@ -14513,7 +14513,7 @@ qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, static int qemuDomainSnapshotCreateInactiveInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap) + virDomainMomentObjPtr snap) { return qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-c", false); } @@ -14523,7 +14523,7 @@ qemuDomainSnapshotCreateInactiveInternal(virQEMUDriverPtr driver, static int qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, bool reuse) { size_t i; @@ -14630,7 +14630,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, static int qemuDomainSnapshotCreateActiveInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -15176,7 +15176,7 @@ qemuDomainSnapshotDiskDataFree(qemuDomainSnapshotDiskDataPtr data, static qemuDomainSnapshotDiskDataPtr qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, bool reuse) { size_t i; @@ -15333,7 +15333,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, static int qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, unsigned int flags, qemuDomainAsyncJob asyncJob) { @@ -15462,7 +15462,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, static int qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, + virDomainMomentObjPtr snap, unsigned int flags) { virObjectEventPtr event; @@ -15660,14 +15660,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL; char *xml = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virDomainSnapshotDefPtr def = NULL; - virDomainSnapshotObjPtr current = NULL; + virDomainMomentObjPtr current = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; - virDomainSnapshotObjPtr other = NULL; + virDomainMomentObjPtr other = NULL; int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; bool align_match = true; virQEMUDriverConfigPtr cfg = NULL; @@ -15931,7 +15931,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } else { other = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); - virDomainSnapshotSetParent(snap, other); + virDomainMomentSetParent(snap, other); } } else if (snap) { virDomainSnapshotObjListRemove(vm->snapshots, snap); @@ -16035,7 +16035,7 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | @@ -16065,7 +16065,7 @@ qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | @@ -16095,7 +16095,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | @@ -16126,7 +16126,7 @@ qemuDomainSnapshotLookupByName(virDomainPtr domain, unsigned int flags) { virDomainObjPtr vm; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virCheckFlags(0, NULL); @@ -16176,7 +16176,7 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr parent = NULL; virCheckFlags(0, NULL); @@ -16241,7 +16241,7 @@ qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, virQEMUDriverPtr driver = virSnapDom(snapshot)->conn->privateData; virDomainObjPtr vm = NULL; char *xml = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; virCheckFlags(VIR_DOMAIN_SNAPSHOT_XML_SECURE, NULL); @@ -16273,7 +16273,7 @@ qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, { virDomainObjPtr vm = NULL; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virCheckFlags(0, -1); @@ -16300,7 +16300,7 @@ qemuDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, { virDomainObjPtr vm = NULL; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virCheckFlags(0, -1); @@ -16328,7 +16328,7 @@ qemuDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, static int qemuDomainSnapshotRevertInactive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap) + virDomainMomentObjPtr snap) { /* Try all disks, but report failure if we skipped any. */ int ret = qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-a", true); @@ -16343,8 +16343,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virQEMUDriverPtr driver = virSnapDom(snapshot)->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; - virDomainSnapshotObjPtr current = NULL; + virDomainMomentObjPtr snap = NULL; + virDomainMomentObjPtr current = NULL; virDomainSnapshotDefPtr snapdef; virObjectEventPtr event = NULL; virObjectEventPtr event2 = NULL; @@ -16762,7 +16762,7 @@ typedef struct _virQEMUSnapReparent virQEMUSnapReparent; typedef virQEMUSnapReparent *virQEMUSnapReparentPtr; struct _virQEMUSnapReparent { virQEMUDriverConfigPtr cfg; - virDomainSnapshotObjPtr parent; + virDomainMomentObjPtr parent; virDomainObjPtr vm; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; @@ -16775,7 +16775,7 @@ qemuDomainSnapshotReparentChildren(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { - virDomainSnapshotObjPtr snap = payload; + virDomainMomentObjPtr snap = payload; virQEMUSnapReparentPtr rep = data; if (rep->err < 0) @@ -16803,7 +16803,7 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virQEMUDriverPtr driver = virSnapDom(snapshot)->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virQEMUSnapRemove rem; virQEMUSnapReparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); @@ -16834,9 +16834,9 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, external++; if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) - virDomainSnapshotForEachDescendant(snap, - qemuDomainSnapshotCountExternal, - &external); + virDomainMomentForEachDescendant(snap, + qemuDomainSnapshotCountExternal, + &external); if (external) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("deletion of %d external disk snapshots not " @@ -16852,9 +16852,9 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, rem.metadata_only = metadata_only; rem.err = 0; rem.current = false; - virDomainSnapshotForEachDescendant(snap, - qemuDomainSnapshotDiscardAll, - &rem); + virDomainMomentForEachDescendant(snap, + qemuDomainSnapshotDiscardAll, + &rem); if (rem.err < 0) goto endjob; if (rem.current) { @@ -16878,16 +16878,16 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, rep.err = 0; rep.caps = driver->caps; rep.xmlopt = driver->xmlopt; - virDomainSnapshotForEachChild(snap, - qemuDomainSnapshotReparentChildren, - &rep); + virDomainMomentForEachChild(snap, + qemuDomainSnapshotReparentChildren, + &rep); if (rep.err < 0) goto endjob; - virDomainSnapshotMoveChildren(snap, snap->parent); + virDomainMomentMoveChildren(snap, snap->parent); } if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { - virDomainSnapshotDropChildren(snap); + virDomainMomentDropChildren(snap); ret = 0; } else { ret = qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata_only); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d878079eab..dc7317b723 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6469,7 +6469,7 @@ qemuProcessLaunch(virConnectPtr conn, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, qemuProcessIncomingDefPtr incoming, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virNetDevVPortProfileOp vmop, unsigned int flags) { @@ -6882,7 +6882,7 @@ qemuProcessStart(virConnectPtr conn, const char *migrateFrom, int migrateFd, const char *migratePath, - virDomainSnapshotObjPtr snapshot, + virDomainMomentObjPtr snapshot, virNetDevVPortProfileOp vmop, unsigned int flags) { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index eac6160532..5aaaf0ab19 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -822,7 +822,7 @@ testParseDomainSnapshots(testDriverPtr privconn, bool cur; for (i = 0; i < nsdata->num_snap_nodes; i++) { - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; virDomainSnapshotDefPtr def; xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "domainsnapshot"); @@ -5946,11 +5946,11 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) * Snapshot APIs */ -static virDomainSnapshotObjPtr +static virDomainMomentObjPtr testSnapObjFromName(virDomainObjPtr vm, const char *name) { - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; snap = virDomainSnapshotFindByName(vm->snapshots, name); if (!snap) virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -5959,7 +5959,7 @@ testSnapObjFromName(virDomainObjPtr vm, return snap; } -static virDomainSnapshotObjPtr +static virDomainMomentObjPtr testSnapObjFromSnapshot(virDomainObjPtr vm, virDomainSnapshotPtr snapshot) { @@ -6042,7 +6042,7 @@ testDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | @@ -6068,7 +6068,7 @@ testDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | @@ -6094,7 +6094,7 @@ testDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | @@ -6121,7 +6121,7 @@ testDomainSnapshotLookupByName(virDomainPtr domain, unsigned int flags) { virDomainObjPtr vm; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virCheckFlags(0, NULL); @@ -6162,7 +6162,7 @@ testDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr parent = NULL; virCheckFlags(0, NULL); @@ -6193,7 +6193,7 @@ testDomainSnapshotCurrent(virDomainPtr domain, { virDomainObjPtr vm; virDomainSnapshotPtr snapshot = NULL; - virDomainSnapshotObjPtr current; + virDomainMomentObjPtr current; virCheckFlags(0, NULL); @@ -6220,7 +6220,7 @@ testDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, { virDomainObjPtr vm = NULL; char *xml = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; testDriverPtr privconn = virSnapDom(snapshot)->conn->privateData; @@ -6322,7 +6322,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, testDriverPtr privconn = domain->conn->privateData; virDomainObjPtr vm = NULL; virDomainSnapshotDefPtr def = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virObjectEventPtr event = NULL; bool update_current = true; @@ -6411,12 +6411,12 @@ testDomainSnapshotCreateXML(virDomainPtr domain, cleanup: if (vm) { if (snapshot) { - virDomainSnapshotObjPtr other; + virDomainMomentObjPtr other; if (update_current) virDomainSnapshotSetCurrent(vm->snapshots, snap); other = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); - virDomainSnapshotSetParent(snap, other); + virDomainMomentSetParent(snap, other); } virDomainObjEndAPI(&vm); } @@ -6438,7 +6438,7 @@ testDomainSnapshotDiscardAll(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { - virDomainSnapshotObjPtr snap = payload; + virDomainMomentObjPtr snap = payload; testSnapRemoveDataPtr curr = data; curr->current |= virDomainSnapshotObjListRemove(curr->vm->snapshots, snap); @@ -6448,7 +6448,7 @@ testDomainSnapshotDiscardAll(void *payload, typedef struct _testSnapReparentData testSnapReparentData; typedef testSnapReparentData *testSnapReparentDataPtr; struct _testSnapReparentData { - virDomainSnapshotObjPtr parent; + virDomainMomentObjPtr parent; virDomainObjPtr vm; int err; }; @@ -6458,7 +6458,7 @@ testDomainSnapshotReparentChildren(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { - virDomainSnapshotObjPtr snap = payload; + virDomainMomentObjPtr snap = payload; testSnapReparentDataPtr rep = data; if (rep->err < 0) @@ -6480,8 +6480,8 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; - virDomainSnapshotObjPtr parentsnap = NULL; + virDomainMomentObjPtr snap = NULL; + virDomainMomentObjPtr parentsnap = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | @@ -6498,9 +6498,9 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, testSnapRemoveData rem; rem.vm = vm; rem.current = false; - virDomainSnapshotForEachDescendant(snap, - testDomainSnapshotDiscardAll, - &rem); + virDomainMomentForEachDescendant(snap, + testDomainSnapshotDiscardAll, + &rem); if (rem.current) virDomainSnapshotSetCurrent(vm->snapshots, snap); } else if (snap->nchildren) { @@ -6508,19 +6508,19 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, rep.parent = snap->parent; rep.vm = vm; rep.err = 0; - virDomainSnapshotForEachChild(snap, - testDomainSnapshotReparentChildren, - &rep); + virDomainMomentForEachChild(snap, + testDomainSnapshotReparentChildren, + &rep); if (rep.err < 0) goto cleanup; - virDomainSnapshotMoveChildren(snap, snap->parent); + virDomainMomentMoveChildren(snap, snap->parent); } if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { - virDomainSnapshotDropChildren(snap); + virDomainMomentDropChildren(snap); } else { - virDomainSnapshotDropParent(snap); + virDomainMomentDropParent(snap); if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { if (snap->def->parent) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, @@ -6546,7 +6546,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, { testDriverPtr privconn = virSnapDom(snapshot)->conn->privateData; virDomainObjPtr vm = NULL; - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; virObjectEventPtr event = NULL; virObjectEventPtr event2 = NULL; virDomainDefPtr config = NULL; diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index eba366dd2c..06f94f5beb 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -2132,10 +2132,10 @@ static int vzDomainSetMemory(virDomainPtr domain, unsigned long memory) return ret; } -static virDomainSnapshotObjPtr +static virDomainMomentObjPtr vzSnapObjFromName(virDomainSnapshotObjListPtr snapshots, const char *name) { - virDomainSnapshotObjPtr snap = NULL; + virDomainMomentObjPtr snap = NULL; snap = virDomainSnapshotFindByName(snapshots, name); if (!snap) virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -2144,7 +2144,7 @@ vzSnapObjFromName(virDomainSnapshotObjListPtr snapshots, const char *name) return snap; } -static virDomainSnapshotObjPtr +static virDomainMomentObjPtr vzSnapObjFromSnapshot(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotPtr snapshot) { @@ -2156,8 +2156,8 @@ vzCurrentSnapshotIterator(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) { - virDomainSnapshotObjPtr snapshot = payload; - virDomainSnapshotObjPtr *current = data; + virDomainMomentObjPtr snapshot = payload; + virDomainMomentObjPtr *current = data; if (snapshot->def->current) *current = snapshot; @@ -2165,10 +2165,10 @@ vzCurrentSnapshotIterator(void *payload, return 0; } -static virDomainSnapshotObjPtr +static virDomainMomentObjPtr vzFindCurrentSnapshot(virDomainSnapshotObjListPtr snapshots) { - virDomainSnapshotObjPtr current = NULL; + virDomainMomentObjPtr current = NULL; virDomainSnapshotForEach(snapshots, vzCurrentSnapshotIterator, &current); return current; @@ -2268,7 +2268,7 @@ vzDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr dom; char *xml = NULL; - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainSnapshotObjListPtr snapshots = NULL; vzConnPtr privconn = virSnapDom(snapshot)->conn->privateData; @@ -2304,7 +2304,7 @@ static int vzDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr dom; - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; virDomainSnapshotObjListPtr snapshots = NULL; int n = -1; @@ -2339,7 +2339,7 @@ vzDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr dom; - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; virDomainSnapshotObjListPtr snapshots = NULL; int n = -1; @@ -2373,7 +2373,7 @@ vzDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr dom; - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; virDomainSnapshotObjListPtr snapshots = NULL; int n = -1; @@ -2407,7 +2407,7 @@ vzDomainSnapshotLookupByName(virDomainPtr domain, unsigned int flags) { virDomainObjPtr dom; - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; virDomainSnapshotPtr snapshot = NULL; virDomainSnapshotObjListPtr snapshots = NULL; @@ -2465,7 +2465,7 @@ static virDomainSnapshotPtr vzDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr dom; - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; virDomainSnapshotPtr parent = NULL; virDomainSnapshotObjListPtr snapshots = NULL; @@ -2505,7 +2505,7 @@ vzDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) virDomainObjPtr dom; virDomainSnapshotPtr snapshot = NULL; virDomainSnapshotObjListPtr snapshots = NULL; - virDomainSnapshotObjPtr current; + virDomainMomentObjPtr current; virCheckFlags(0, NULL); @@ -2539,7 +2539,7 @@ vzDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, unsigned int flags) virDomainObjPtr dom; int ret = -1; virDomainSnapshotObjListPtr snapshots = NULL; - virDomainSnapshotObjPtr current; + virDomainMomentObjPtr current; virCheckFlags(0, -1); @@ -2568,7 +2568,7 @@ vzDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, { virDomainObjPtr dom; int ret = -1; - virDomainSnapshotObjPtr snap; + virDomainMomentObjPtr snap; virDomainSnapshotObjListPtr snapshots = NULL; virCheckFlags(0, -1); @@ -2606,7 +2606,7 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, vzDriverPtr driver = privconn->driver; unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; virDomainSnapshotObjListPtr snapshots = NULL; - virDomainSnapshotObjPtr current; + virDomainMomentObjPtr current; bool job = false; virCheckFlags(0, NULL); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b9fd03c0d2..f5a59115d2 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4655,8 +4655,8 @@ prlsdkParseSnapshotTree(const char *treexml) xmlNodePtr root; xmlNodePtr *nodes = NULL; virDomainSnapshotDefPtr def = NULL; - virDomainSnapshotObjPtr snapshot; - virDomainSnapshotObjPtr current = NULL; + virDomainMomentObjPtr snapshot; + virDomainMomentObjPtr current = NULL; virDomainSnapshotObjListPtr snapshots = NULL; char *xmlstr = NULL; int n; -- 2.20.1

Now that we have made virDomainMomentObj sufficiently generic to support both snapshots and checkpoints, it is time to rename the file that it lives in. The split between a generic object and a list of the generic objects doesn't buy us as much, so it will be easier to stick all the moment list code in one file, with more code moving in the next patch. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- ...snapshotobj.h => virdomainmomentobjlist.h} | 26 ++++++++++--------- src/conf/virdomainsnapshotobjlist.h | 7 ++++- src/conf/Makefile.inc.am | 4 +-- ...snapshotobj.c => virdomainmomentobjlist.c} | 12 ++++----- src/libvirt_private.syms | 18 ++++++------- 5 files changed, 37 insertions(+), 30 deletions(-) rename src/conf/{virdomainsnapshotobj.h => virdomainmomentobjlist.h} (72%) rename src/conf/{virdomainsnapshotobj.c => virdomainmomentobjlist.c} (95%) diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainmomentobjlist.h similarity index 72% rename from src/conf/virdomainsnapshotobj.h rename to src/conf/virdomainmomentobjlist.h index 86c5f6860b..dceb55fca2 100644 --- a/src/conf/virdomainsnapshotobj.h +++ b/src/conf/virdomainmomentobjlist.h @@ -1,5 +1,5 @@ /* - * virdomainsnapshotobj.h: handle snapshot objects + * virdomainmomentobjlist.h: handle a tree of moment objects * (derived from snapshot_conf.h) * * Copyright (C) 2006-2019 Red Hat, Inc. @@ -20,25 +20,33 @@ * <http://www.gnu.org/licenses/>;. */ -#ifndef LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H -# define LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H +#ifndef LIBVIRT_VIRDOMAINMOMENTOBJLIST_H +# define LIBVIRT_VIRDOMAINMOMENTOBJLIST_H # include "internal.h" # include "virconftypes.h" # include "virhash.h" +/* Struct that allows tracing hierarchical relationships between + * multiple virDomainMoment objects. The opaque type + * virDomainMomentObjList then maintains both a hash of these structs + * (for quick lookup by name) and a metaroot (which is the parent of + * all user-visible roots), so that all other objects always have a + * valid parent object; the tree structure is currently maintained via + * a linked list. */ struct _virDomainMomentObj { + /* Public field */ virDomainMomentDefPtr def; /* non-NULL except for metaroot */ + /* Private fields, use accessors instead */ virDomainMomentObjPtr parent; /* non-NULL except for metaroot, before - virDomainSnapshotUpdateRelations, or + virDomainMomentUpdateRelations, or after virDomainMomentDropParent */ virDomainMomentObjPtr sibling; /* NULL if last child of parent */ size_t nchildren; virDomainMomentObjPtr first_child; /* NULL if no children */ }; - int virDomainMomentForEachChild(virDomainMomentObjPtr moment, virHashIterator iter, void *data); @@ -52,10 +60,4 @@ void virDomainMomentMoveChildren(virDomainMomentObjPtr from, void virDomainMomentSetParent(virDomainMomentObjPtr moment, virDomainMomentObjPtr parent); -static inline virDomainSnapshotDefPtr -virDomainSnapshotObjGetDef(virDomainMomentObjPtr obj) -{ - return (virDomainSnapshotDefPtr) obj->def; -} - -#endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */ +#endif /* LIBVIRT_VIRDOMAINMOMENTOBJLIST_H */ diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapshotobjlist.h index 1af367639d..c36b498751 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -24,7 +24,7 @@ # define LIBVIRT_VIRDOMAINSNAPSHOTOBJLIST_H # include "internal.h" -# include "virdomainsnapshotobj.h" +# include "virdomainmomentobjlist.h" # include "virbuffer.h" /* Filter that returns true if a given moment matches the filter flags */ @@ -80,4 +80,9 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotPtr **snaps, unsigned int flags); +static inline virDomainSnapshotDefPtr +virDomainSnapshotObjGetDef(virDomainMomentObjPtr obj) +{ + return (virDomainSnapshotDefPtr) obj->def; +} #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJLIST_H */ diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index d2ff8be8fd..6eb64db9de 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -31,8 +31,8 @@ DOMAIN_CONF_SOURCES = \ conf/virconftypes.h \ conf/virdomainobjlist.c \ conf/virdomainobjlist.h \ - conf/virdomainsnapshotobj.c \ - conf/virdomainsnapshotobj.h \ + conf/virdomainmomentobjlist.c \ + conf/virdomainmomentobjlist.h \ conf/virdomainsnapshotobjlist.c \ conf/virdomainsnapshotobjlist.h \ $(NULL) diff --git a/src/conf/virdomainsnapshotobj.c b/src/conf/virdomainmomentobjlist.c similarity index 95% rename from src/conf/virdomainsnapshotobj.c rename to src/conf/virdomainmomentobjlist.c index 877a0a9079..766d7fe2e4 100644 --- a/src/conf/virdomainsnapshotobj.c +++ b/src/conf/virdomainmomentobjlist.c @@ -1,5 +1,5 @@ /* - * virdomainsnapshotobj.c: handle snapshot objects + * virdomainmomentobjlist.c: handle snapshot/checkpoint objects * (derived from snapshot_conf.c) * * Copyright (C) 2006-2019 Red Hat, Inc. @@ -23,15 +23,15 @@ #include <config.h> #include "internal.h" -#include "virdomainsnapshotobj.h" -#include "snapshot_conf.h" -#include "virdomainsnapshotobjlist.h" +#include "virdomainmomentobjlist.h" #include "virlog.h" #include "virerror.h" +#include "virstring.h" +#include "moment_conf.h" -#define VIR_FROM_THIS VIR_FROM_DOMAIN_SNAPSHOT +#define VIR_FROM_THIS VIR_FROM_DOMAIN -VIR_LOG_INIT("conf.virdomainsnapshotobj"); +VIR_LOG_INIT("conf.virdomainmomentobjlist"); /* Run iter(data) on all direct children of moment, while ignoring all * other entries in moments. Return the number of children diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 02f383fdba..10cdfad4fd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -960,6 +960,15 @@ virChrdevFree; virChrdevOpen; +# conf/virdomainmomentobjlist.h +virDomainMomentDropChildren; +virDomainMomentDropParent; +virDomainMomentForEachChild; +virDomainMomentForEachDescendant; +virDomainMomentMoveChildren; +virDomainMomentSetParent; + + # conf/virdomainobjlist.h virDomainObjListAdd; virDomainObjListCollect; @@ -979,15 +988,6 @@ virDomainObjListRemoveLocked; virDomainObjListRename; -# conf/virdomainsnapshotobj.h -virDomainMomentDropChildren; -virDomainMomentDropParent; -virDomainMomentForEachChild; -virDomainMomentForEachDescendant; -virDomainMomentMoveChildren; -virDomainMomentSetParent; - - # conf/virdomainsnapshotobjlist.h virDomainListSnapshots; virDomainSnapshotAssignDef; -- 2.20.1

Prepare for introducing a bunch of new public APIs related to backup checkpoints by first introducing a new internal type and errors associated with that type. Checkpoints are modeled heavily after virDomainSnapshotPtr (both represent a point in time of the guest), although a snapshot exists with the intent of rolling back to that state, while a checkpoint exists to make it possible to create an incremental backup at a later time. Thus, it shares the common virDomainMoment base class created in the previous patches. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- include/libvirt/virterror.h | 6 +++++- src/util/virerror.c | 12 ++++++++++- include/libvirt/libvirt.h | 6 +++++- src/datatypes.h | 43 +++++++++++++++++++++++++++++++++++++ src/datatypes.c | 22 +++++++++++++++++++ src/libvirt_private.syms | 2 ++ 6 files changed, 88 insertions(+), 3 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 3c19ff5e2e..bccf3c731e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * errors raised while using the library. * - * Copyright (C) 2006-2016 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -132,6 +132,7 @@ typedef enum { VIR_FROM_LIBSSH = 66, /* Error from libssh connection transport */ VIR_FROM_RESCTRL = 67, /* Error from resource control */ VIR_FROM_FIREWALLD = 68, /* Error from firewalld */ + VIR_FROM_DOMAIN_CHECKPOINT = 69,/* Error from domain checkpoint */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST @@ -322,6 +323,9 @@ typedef enum { VIR_ERR_DEVICE_MISSING = 99, /* fail to find the desired device */ VIR_ERR_INVALID_NWFILTER_BINDING = 100, /* invalid nwfilter binding */ VIR_ERR_NO_NWFILTER_BINDING = 101, /* no nwfilter binding */ + VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 102, /* invalid domain checkpoint */ + VIR_ERR_NO_DOMAIN_CHECKPOINT = 103, /* domain checkpoint not found */ + VIR_ERR_NO_DOMAIN_BACKUP = 104, /* domain backup job id not found */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_NUMBER_LAST diff --git a/src/util/virerror.c b/src/util/virerror.c index 91a513160f..05e535d859 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1,7 +1,7 @@ /* * virerror.c: error handling and reporting code for libvirt * - * Copyright (C) 2006, 2008-2016 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Libssh transport layer", "Resource control", "FirewallD", + "Domain Checkpoint", ); @@ -1214,6 +1215,15 @@ const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = { [VIR_ERR_NO_NWFILTER_BINDING] = { N_("Network filter binding not found"), N_("Network filter binding not found: %s") }, + [VIR_ERR_INVALID_DOMAIN_CHECKPOINT] = { + N_("Invalid domain checkpoint"), + N_("Invalid domain checkpoint: %s") }, + [VIR_ERR_NO_DOMAIN_CHECKPOINT] = { + N_("Domain checkpoint not found"), + N_("Domain checkpoint not found: %s") }, + [VIR_ERR_NO_DOMAIN_BACKUP] = { + N_("Domain backup job id not found"), + N_("Domain backup job id not found: %s") }, }; diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 20e5d276a7..13de151cb6 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * virtualized domains * - * Copyright (C) 2005-2006, 2010-2014 Red Hat, Inc. + * Copyright (C) 2005-2019 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -34,6 +34,10 @@ extern "C" { # include <libvirt/libvirt-common.h> # include <libvirt/libvirt-host.h> # include <libvirt/libvirt-domain.h> +/* FIXME: Temporary hack until later patch creates new + * libvirt-domain-checkpoint.h file */ +typedef struct _virDomainCheckpoint virDomainCheckpoint; +typedef virDomainCheckpoint *virDomainCheckpointPtr; # include <libvirt/libvirt-domain-snapshot.h> # include <libvirt/libvirt-event.h> # include <libvirt/libvirt-interface.h> diff --git a/src/datatypes.h b/src/datatypes.h index 70d947657b..3d316e6739 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -32,6 +32,7 @@ extern virClassPtr virConnectClass; extern virClassPtr virDomainClass; extern virClassPtr virDomainMomentClass; +extern virClassPtr virDomainCheckpointClass; extern virClassPtr virDomainSnapshotClass; extern virClassPtr virInterfaceClass; extern virClassPtr virNetworkClass; @@ -293,6 +294,22 @@ extern virClassPtr virAdmClientClass; } \ } while (0) + +# define virCheckDomainCheckpointReturn(obj, retval) \ + do { \ + virDomainCheckpointPtr _check = (obj); \ + if (!virObjectIsClass(_check, virDomainCheckpointClass) || \ + !virObjectIsClass(virChkDom(_check), virDomainClass) || \ + !virObjectIsClass(virChkDom(_check)->conn, virConnectClass)) { \ + virReportErrorHelper(VIR_FROM_DOMAIN_CHECKPOINT, \ + VIR_ERR_INVALID_DOMAIN_CHECKPOINT, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) + # define virCheckDomainSnapshotReturn(obj, retval) \ do { \ virDomainSnapshotPtr _snap = (obj); \ @@ -683,6 +700,30 @@ struct _virDomainMoment { virDomainPtr domain; }; +/* + * _virDomainCheckpoint + * + * Internal structure associated with a domain checkpoint + */ +struct _virDomainCheckpoint { + virDomainMoment parent; + + /* Unused attribute to allow for subclass creation */ + bool dummy; +}; + +static inline char * +virChkName(virDomainCheckpointPtr checkpoint) +{ + return checkpoint->parent.name; +} + +static inline virDomainPtr +virChkDom(virDomainCheckpointPtr checkpoint) +{ + return checkpoint->parent.domain; +} + /** * _virDomainSnapshot * @@ -772,6 +813,8 @@ virNWFilterPtr virGetNWFilter(virConnectPtr conn, virNWFilterBindingPtr virGetNWFilterBinding(virConnectPtr conn, const char *portdev, const char *filtername); +virDomainCheckpointPtr virGetDomainCheckpoint(virDomainPtr domain, + const char *name); virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, const char *name); diff --git a/src/datatypes.c b/src/datatypes.c index f0cfbe11fc..ec22a7b38a 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -37,6 +37,7 @@ virClassPtr virConnectClass; virClassPtr virConnectCloseCallbackDataClass; virClassPtr virDomainClass; virClassPtr virDomainMomentClass; +virClassPtr virDomainCheckpointClass; virClassPtr virDomainSnapshotClass; virClassPtr virInterfaceClass; virClassPtr virNetworkClass; @@ -52,6 +53,7 @@ static void virConnectDispose(void *obj); static void virConnectCloseCallbackDataDispose(void *obj); static void virDomainDispose(void *obj); static void virDomainMomentDispose(void *obj); +#define virDomainCheckpointDispose NULL #define virDomainSnapshotDispose NULL static void virInterfaceDispose(void *obj); static void virNetworkDispose(void *obj); @@ -89,6 +91,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData); DECLARE_CLASS(virDomain); DECLARE_CLASS(virDomainMoment); + DECLARE_CLASS_COMMON(virDomainCheckpoint, virDomainMomentClass); DECLARE_CLASS_COMMON(virDomainSnapshot, virDomainMomentClass); DECLARE_CLASS(virInterface); DECLARE_CLASS(virNetwork); @@ -961,6 +964,25 @@ virDomainMomentDispose(void *obj) } +/** + * virGetDomainCheckpoint: + * @domain: the domain to checkpoint + * @name: pointer to the domain checkpoint name + * + * Allocates a new domain checkpoint object. When the object is no longer needed, + * virObjectUnref() must be called in order to not leak data. + * + * Returns a pointer to the domain checkpoint object, or NULL on error. + */ +virDomainCheckpointPtr +virGetDomainCheckpoint(virDomainPtr domain, + const char *name) +{ + return (virDomainCheckpointPtr) virGetDomainMoment(domain, name, + virDomainCheckpointClass); +} + + /** * virGetDomainSnapshot: * @domain: the domain to snapshot diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 10cdfad4fd..c848ba4239 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1252,10 +1252,12 @@ virConnectCloseCallbackDataClass; virConnectCloseCallbackDataGetCallback; virConnectCloseCallbackDataRegister; virConnectCloseCallbackDataUnregister; +virDomainCheckpointClass; virDomainClass; virDomainSnapshotClass; virGetConnect; virGetDomain; +virGetDomainCheckpoint; virGetDomainSnapshot; virGetInterface; virGetNetwork; -- 2.20.1

Rather than one file per snapshot, store all qemu snapshots in a single file, using the recently added bulk snapshot list operations. For now, this doesn't change how often libvirt writes a snapshot file, but it does open the door for the next patch to update the signature to qemuDomainSnapshotWriteMetadata() and call it less frequently. One of the main benefits for doing a bulk write is that you only have to do a single file system write at the end of an operation, rather than potentially 3 during virDomainSnapshotCreateXML(REDEFINE|CURRENT) (delete the old snapshot definition being redefined, rewrite the previous current snapshot to longer be current, and store the new snapshot definition) or even more during virDomainSnapshotDelete(DESCENDENTS) (a file system hit per snapshot being deleted). It also makes it perhaps a bit more feasible to roll back to earlier state if something fails horribly midway through an operation (until you write the new file, the old file is still a reliable record of what state to roll back to), compared to the current code which has to track lots of things locally; although I did not attempt to play with any patches along those lines. Another benefit of the bulk write - it's less code to maintain, and will make it easier for me to model qemu's checkpoint storage in the same way (and for checkpoints, I don't even have to worry about legacy parsing). This is a one-way upgrade - if you have snapshots created by an older libvirt, the new libvirt will correctly load those snapshots and convert to the new format. But as the new libvirt will no longer output the old format, reverting back to the old libvirt will make it appear that all snapshots have disappeared (merely hidden until you upgrade libvirt again). But then again, we've never promised that downgrading libvirt after an upgrade was supposed to work flawlessly. There is a slight chance for confusion if a user named two separate domains 'foo' and 'foo.xml'; the new scheme expects 'foo.xml' to be a regular file for the former domain, while the old scheme expected 'foo.xml' to be a directory for the latter domain; if we are worried about that, we could tweak the code to instead output new state in a file named 'foo' instead of the more typical 'foo.xml' and just key off of whether the file is regular or a directory when deciding if this is the first libvirt run after an upgrade. But I felt that the chances of a user abusing domain names like that is not worth the effort. The bulk snapshot file can be huge (over RPC, we allow <domain> to be up to 4M, and we allow up to 16k snapshots; since each each snapshot includes a <domain>, a worst-case domain would result in gigabytes of bulk data); it is no worse on overall filesystem usage than before, but now in a single file vs. a series of files it requires more memory to read in at once. I don't know if we have to worry about that in practice, but my patch does cap things to read in no more than an arbitrarily-picked 128M, which we may have to raise in the future. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 59 ++++++++------------------- src/qemu/qemu_driver.c | 93 +++++++++++++++++++++++++++++++++--------- 2 files changed, 91 insertions(+), 61 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ea7b31dab3..424f839a00 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8448,45 +8448,28 @@ qemuFindQemuImgBinary(virQEMUDriverPtr driver) int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, - virDomainMomentObjPtr snapshot, + virDomainMomentObjPtr snapshot ATTRIBUTE_UNUSED, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, const char *snapshotDir) { - char *newxml = NULL; - int ret = -1; - char *snapDir = NULL; - char *snapFile = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE | - VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL; - virDomainSnapshotDefPtr def = virDomainSnapshotObjGetDef(snapshot); + unsigned int flags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE; + VIR_AUTOFREE(char *) newxml = NULL; + VIR_AUTOFREE(char *) snapDir = NULL; + VIR_AUTOFREE(char *) snapFile = NULL; + VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if (virDomainSnapshotGetCurrent(vm->snapshots) == snapshot) - flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT; virUUIDFormat(vm->def->uuid, uuidstr); - newxml = virDomainSnapshotDefFormat(uuidstr, def, caps, xmlopt, flags); - if (newxml == NULL) + if (virDomainSnapshotObjListFormat(&buf, uuidstr, vm->snapshots, caps, + xmlopt, flags) < 0) return -1; - if (virAsprintf(&snapDir, "%s/%s", snapshotDir, vm->def->name) < 0) - goto cleanup; - if (virFileMakePath(snapDir) < 0) { - virReportSystemError(errno, _("cannot create snapshot directory '%s'"), - snapDir); - goto cleanup; - } - - if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->common.name) < 0) - goto cleanup; - - ret = virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml); + if (virAsprintf(&snapFile, "%s/%s.xml", snapshotDir, vm->def->name) < 0) + return -1; - cleanup: - VIR_FREE(snapFile); - VIR_FREE(snapDir); - VIR_FREE(newxml); - return ret; + newxml = virBufferContentAndReset(&buf); + return virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml); } /* The domain is expected to be locked and inactive. Return -1 on normal @@ -8589,7 +8572,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, bool update_parent, bool metadata_only) { - char *snapFile = NULL; int ret = -1; qemuDomainObjPrivatePtr priv; virDomainMomentObjPtr parentsnap = NULL; @@ -8610,10 +8592,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, } } - if (virAsprintf(&snapFile, "%s/%s/%s.xml", cfg->snapshotDir, - vm->def->name, snap->def->name) < 0) - goto cleanup; - if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { virDomainSnapshotSetCurrent(vm->snapshots, NULL); if (update_parent && snap->def->parent) { @@ -8635,8 +8613,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, } } - if (unlink(snapFile) < 0) - VIR_WARN("Failed to unlink %s", snapFile); if (update_parent) virDomainMomentDropParent(snap); virDomainSnapshotObjListRemove(vm->snapshots, snap); @@ -8644,7 +8620,6 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, ret = 0; cleanup: - VIR_FREE(snapFile); virObjectUnref(cfg); return ret; } @@ -8691,7 +8666,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, virDomainObjPtr vm) { virQEMUDriverConfigPtr cfg; - VIR_AUTOFREE(char *) snapDir = NULL; + VIR_AUTOFREE(char *) snapFile = NULL; cfg = virQEMUDriverGetConfig(driver); @@ -8699,12 +8674,12 @@ qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0) { VIR_WARN("unable to remove all snapshots for domain %s", vm->def->name); - } else if (virAsprintf(&snapDir, "%s/%s", cfg->snapshotDir, + } else if (virAsprintf(&snapFile, "%s/%s.xml", cfg->snapshotDir, vm->def->name) < 0) { - VIR_WARN("unable to remove snapshot directory %s/%s", + VIR_WARN("unable to remove snapshots storage %s/%s.xml", cfg->snapshotDir, vm->def->name); - } else if (rmdir(snapDir) < 0 && errno != ENOENT) { - VIR_WARN("unable to remove snapshot directory %s", snapDir); + } else if (unlink(snapFile) < 0 && errno != ENOENT) { + VIR_WARN("unable to remove snapshots storage %s", snapFile); } qemuExtDevicesCleanupHost(driver, vm->def); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9c2245b095..018d1cdc87 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -406,12 +406,14 @@ qemuSecurityInit(virQEMUDriverPtr driver) } +/* Older qemu used a series of $dir/snapshot/domainname/snap.xml + * files, instead of the modern $dir/snapshot/domainname.xml bulk + * file. Called while vm is locked. */ static int -qemuDomainSnapshotLoad(virDomainObjPtr vm, - void *data) +qemuDomainSnapshotLoadLegacy(virDomainObjPtr vm, + char *snapDir, + virCapsPtr caps) { - char *baseDir = (char *)data; - char *snapDir = NULL; DIR *dir = NULL; struct dirent *entry; char *xmlStr; @@ -424,21 +426,8 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL); int ret = -1; - virCapsPtr caps = NULL; int direrr; - virObjectLock(vm); - if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to allocate memory for " - "snapshot directory for domain %s"), - vm->def->name); - goto cleanup; - } - - if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false))) - goto cleanup; - VIR_INFO("Scanning for snapshots for domain %s in %s", vm->def->name, snapDir); @@ -503,6 +492,74 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, _("Snapshots have inconsistent relations for domain %s"), vm->def->name); + virResetLastError(); + + ret = 0; + cleanup: + VIR_DIR_CLOSE(dir); + return ret; +} + + +/* Load all snapshots associated with domain */ +static int +qemuDomainSnapshotLoad(virDomainObjPtr vm, + void *data) +{ + char *baseDir = (char *)data; + unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_PARSE_DISKS); + int ret = -1; + virCapsPtr caps = NULL; + VIR_AUTOFREE(char *) snapFile = NULL; + VIR_AUTOFREE(char *) snapDir = NULL; + VIR_AUTOFREE(char *) xmlStr = NULL; + + virObjectLock(vm); + VIR_INFO("Scanning for snapshots for domain %s in %s", vm->def->name, + baseDir); + if (virAsprintf(&snapFile, "%s/%s.xml", baseDir, vm->def->name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate memory for " + "snapshots storage for domain %s"), + vm->def->name); + goto cleanup; + } + + if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false))) + goto cleanup; + + if (virFileExists(snapFile)) { + /* State last saved by modern libvirt in single file. As each + * snapshot contains a <domain>, it can be quite large. */ + if (virFileReadAll(snapFile, 128*1024*1024*1, &xmlStr) < 0) { + /* Nothing we can do here */ + virReportSystemError(errno, + _("Failed to read snapshot file %s"), + snapFile); + goto cleanup; + } + + ret = virDomainSnapshotObjListParse(xmlStr, vm->def->uuid, + vm->snapshots, caps, + qemu_driver->xmlopt, flags); + if (ret < 0) + goto cleanup; + VIR_INFO("Read in %d snapshots for domain %s", ret, vm->def->name); + } else if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) >= 0 && + virFileExists(snapDir)) { + /* State may have been saved by earlier libvirt; if so, try to + * read it in, convert to modern style, and remove the old + * directory if successful. */ + if (qemuDomainSnapshotLoadLegacy(vm, snapDir, caps) < 0) + goto cleanup; + if (qemuDomainSnapshotWriteMetadata(vm, NULL, caps, + qemu_driver->xmlopt, baseDir) < 0) + goto cleanup; + if (virFileDeleteTree(snapDir) < 0) + VIR_WARN("Unable to remove legacy snapshot directory %s", snapDir); + } + /* FIXME: qemu keeps internal track of snapshots. We can get access * to this info via the "info snapshots" monitor command for running * domains, or via "qemu-img snapshot -l" for shutoff domains. It would @@ -516,8 +573,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, ret = 0; cleanup: - VIR_DIR_CLOSE(dir); - VIR_FREE(snapDir); virObjectUnref(caps); virObjectUnlock(vm); return ret; -- 2.20.1

On 3/20/19 5:32 PM, Eric Blake wrote: I guess proper review of this depends on the decision from 17/16. Still if the decision going forward it to have in one large file, then this patch appears to be right to me. If there's some pushback that says no we have to give the consumer a decision to keep saving single snapshot files, then this cannot be done. Still in reading this patch, I'm not sure we could keep a hybrid model, so that swings the pendulum towards let's go with the one file although the RPC limitation is a concern. Like I said in the last one - if snapshots continue to use the old way and checkpoints fall in the other direction of using one file - that's fine. Sucks to keep two models around though. Hard to know without empirical data that someone has *that many* snapshots, but I hate to assume anything at this point in the game because that bites us hard. Or at least it did with the stats data and some config w/ more disks than ever thought reasonably possible ;-). John

On 3/20/19 5:32 PM, Eric Blake wrote: This kind of information I would think would need to be heavily if not overly documented. I assume someone could say this should be "stuck behind" a forced decision by the consumer to do the conversion to the new style. What worries me most is the issues in the subsequent paragraphs that could bite someone. To some degree we could be 'stuck with' the model for snapshots, but that doesn't mean checkpoints couldn't make use of some newer functionality that stores everything in one file and not allow the storage into multiple files. This is one of those gray areas for me that from an overall architectural level that we struggle with related to dealing with our technical debt because we've guaranteed some sort of backcompat for various things "forever". I also wonder what harm it does to keep the old style around and *force* the consumer to do the deletion. Whether that's by them running rmdir on the old directory or running some libvirt command to clean out the legacy. I lean towards leaving the old stuff there - if nothing else it provides some strange sort of comfort. Maybe even rename the directory to have something like ".old" in the name. I'm OK with the choice and arguments you make for the algorithm you chose, but a ping of Daniel to make sure he's OK with it probably would help here, especially since he commented on the last pile and brought the issue to the forefront. I think this is just another thing to document. As in, if you've done this and use snapshots, then "fix your stupidity" ;-). Why anyone other that QE to test all possibilities would name a domain with ".xml" in the name suffix goes beyond common sense to me. and even more. I think when I first reviewed this I thought of the RPC limit stuff - at least w/r/t how the stats code is the other place where we've hit this limit. But for some reason I felt that perhaps some under the covers code in that code we solved something that allowed for things to work, but now I'm not so sure. @snapDir is not used So "theoretically" we'll never clean up the old way? Maybe this should survive and have a prefix of virFileExists before the unlink below? or some sort of rename for posterity (depends on the opinion about keeping the historical references). FWIW: Similar thoughts here about keeping the historical reference and then "forcing" the user to delete it themselves or provide some sort of new flag to delete the old historical files. This is where I wonder if it'd be better to keep the historical reference and then perhaps in the "if" portion of this code do a similar generation of snapDir, virFileExists (OK, Directory), and then VIR_WARN that old stuff still exists... For what's here - sure it seems to do the right thing, but let's be sure we think we're making the right decision before an R-by, fair enough? John

Rather than hard-coding the snapshot filter bit values into the generic code, add another layer of indirection: callers must map which of their public filter bits correspond to supported moment bits, then pass two separate flags (the ones translated for moment code to operate on, and the remaining ones for the filter callback to operate on). Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- I realized I don't have to mess with matching public API bits at all. But if I'm smart, the bit values I choose for checkpoints will match the internal ones supported by moments, so that only the snapshot code has to do the mapping. src/conf/virdomainmomentobjlist.h | 32 ++++++++++++++++++++++++++-- src/conf/virdomainmomentobjlist.c | 33 +++++++++++++++-------------- src/conf/virdomainsnapshotobjlist.c | 29 +++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 20 deletions(-) diff --git a/src/conf/virdomainmomentobjlist.h b/src/conf/virdomainmomentobjlist.h index 89a0df5a24..b67af24bba 100644 --- a/src/conf/virdomainmomentobjlist.h +++ b/src/conf/virdomainmomentobjlist.h @@ -70,12 +70,40 @@ void virDomainMomentObjListFree(virDomainMomentObjListPtr moments); virDomainMomentObjPtr virDomainMomentAssignDef(virDomainMomentObjListPtr moments, virDomainMomentDefPtr def); +/* Various enum bits that map to public API filters. Note that the + * values of the internal bits are not necessarily the same as the + * public ones. */ +typedef enum { + VIR_DOMAIN_MOMENT_LIST_ROOTS = (1 << 0), + VIR_DOMAIN_MOMENT_LIST_DESCENDANTS = (1 << 0), + VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL = (1 << 1), + VIR_DOMAIN_MOMENT_LIST_LEAVES = (1 << 2), + VIR_DOMAIN_MOMENT_LIST_NO_LEAVES = (1 << 3), + VIR_DOMAIN_MOMENT_LIST_METADATA = (1 << 4), + VIR_DOMAIN_MOMENT_LIST_NO_METADATA = (1 << 5), +} virDomainMomentFilters; + +# define VIR_DOMAIN_MOMENT_FILTERS_METADATA \ + (VIR_DOMAIN_MOMENT_LIST_METADATA | \ + VIR_DOMAIN_MOMENT_LIST_NO_METADATA) + +# define VIR_DOMAIN_MOMENT_FILTERS_LEAVES \ + (VIR_DOMAIN_MOMENT_LIST_LEAVES | \ + VIR_DOMAIN_MOMENT_LIST_NO_LEAVES) + +# define VIR_DOMAIN_MOMENT_FILTERS_ALL \ + (VIR_DOMAIN_MOMENT_LIST_ROOTS | \ + VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL | \ + VIR_DOMAIN_MOMENT_FILTERS_METADATA | \ + VIR_DOMAIN_MOMENT_FILTERS_LEAVES) + int virDomainMomentObjListGetNames(virDomainMomentObjListPtr moments, virDomainMomentObjPtr from, char **const names, int maxnames, - unsigned int flags, - virDomainMomentObjListFilter filter); + unsigned int moment_flags, + virDomainMomentObjListFilter filter, + unsigned int filter_flags); virDomainMomentObjPtr virDomainMomentFindByName(virDomainMomentObjListPtr moments, const char *name); int virDomainMomentObjListSize(virDomainMomentObjListPtr moments); diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 46bd3e448a..94b927746a 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -281,6 +281,7 @@ struct virDomainMomentNameData { int count; bool error; virDomainMomentObjListFilter filter; + unsigned int filter_flags; }; @@ -295,13 +296,12 @@ static int virDomainMomentObjListCopyNames(void *payload, return 0; /* Caller already sanitized flags. Filtering on DESCENDANTS was * done by choice of iteration in the caller. */ - /* TODO: Create VIR_DOMAIN_MOMENT_LIST names */ - if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren) + if ((data->flags & VIR_DOMAIN_MOMENT_LIST_LEAVES) && obj->nchildren) return 0; - if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren) + if ((data->flags & VIR_DOMAIN_MOMENT_LIST_NO_LEAVES) && !obj->nchildren) return 0; - if (!data->filter(obj, data->flags)) + if (!data->filter(obj, data->filter_flags)) return 0; if (data->names && data->count < data->maxnames && @@ -320,25 +320,26 @@ virDomainMomentObjListGetNames(virDomainMomentObjListPtr moments, char **const names, int maxnames, unsigned int flags, - virDomainMomentObjListFilter filter) + virDomainMomentObjListFilter filter, + unsigned int filter_flags) { struct virDomainMomentNameData data = { names, maxnames, flags, 0, - false, filter }; + false, filter, filter_flags }; size_t i; + virCheckFlags(VIR_DOMAIN_MOMENT_FILTERS_ALL, -1); if (!from) { /* LIST_ROOTS and LIST_DESCENDANTS have the same bit value, * but opposite semantics. Toggle here to get the correct * traversal on the metaroot. */ - /* TODO: Create VIR_DOMAIN_MOMENT_LIST names */ - flags ^= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; + flags ^= VIR_DOMAIN_MOMENT_LIST_ROOTS; from = &moments->metaroot; } /* We handle LIST_ROOT/LIST_DESCENDANTS and LIST_TOPOLOGICAL directly, * mask those bits out to determine when we must use the filter callback. */ - data.flags &= ~(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | - VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL); + data.flags &= ~(VIR_DOMAIN_MOMENT_LIST_DESCENDANTS | + VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL); /* If this common code is being used, we assume that all moments * have metadata, and thus can handle METADATA up front as an @@ -346,21 +347,21 @@ virDomainMomentObjListGetNames(virDomainMomentObjListPtr moments, * add the ability to track qcow2 internal snapshots without the * use of metadata, in which case this check should move into the * filter callback. */ - if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) == - VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) + if ((data.flags & VIR_DOMAIN_MOMENT_FILTERS_METADATA) == + VIR_DOMAIN_MOMENT_LIST_NO_METADATA) return 0; - data.flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA; + data.flags &= ~VIR_DOMAIN_MOMENT_FILTERS_METADATA; - if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) { + if (flags & VIR_DOMAIN_MOMENT_LIST_DESCENDANTS) { /* We could just always do a topological visit; but it is * possible to optimize for less stack usage and time when a * simpler full hashtable visit or counter will do. */ if (from->def || (names && - (flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL))) + (flags & VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL))) virDomainMomentForEachDescendant(from, virDomainMomentObjListCopyNames, &data); - else if (names || data.flags) + else if (names || data.flags || filter_flags) virHashForEach(moments->objs, virDomainMomentObjListCopyNames, &data); else diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index 52887add46..0e38e1585e 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -279,6 +279,31 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, int maxnames, unsigned int flags) { + /* Convert public flags into common flags */ + unsigned int moment_flags = 0; + struct { int snap_flag; int moment_flag; } map[] = { + { VIR_DOMAIN_SNAPSHOT_LIST_ROOTS, + VIR_DOMAIN_MOMENT_LIST_ROOTS, }, + { VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL, + VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL, }, + { VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, + VIR_DOMAIN_MOMENT_LIST_LEAVES, }, + { VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES, + VIR_DOMAIN_MOMENT_LIST_NO_LEAVES, }, + { VIR_DOMAIN_SNAPSHOT_LIST_METADATA, + VIR_DOMAIN_MOMENT_LIST_METADATA, }, + { VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, + VIR_DOMAIN_MOMENT_LIST_NO_METADATA, }, + }; + size_t i; + + for (i = 0; i < ARRAY_CARDINALITY(map); i++) { + if (flags & map[i].snap_flag) { + flags &= ~map[i].snap_flag; + moment_flags |= map[i].moment_flag; + } + } + /* For ease of coding the visitor, it is easier to zero each group * where all of the bits are set. */ if ((flags & VIR_DOMAIN_SNAPSHOT_FILTERS_LEAVES) == @@ -291,8 +316,8 @@ virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION) flags &= ~VIR_DOMAIN_SNAPSHOT_FILTERS_LOCATION; return virDomainMomentObjListGetNames(snapshots->base, from, names, - maxnames, flags, - virDomainSnapshotFilter); + maxnames, moment_flags, + virDomainSnapshotFilter, flags); } -- 2.20.1

On Wed, 2019-03-20 at 00:40 -0500, Eric Blake wrote: This causes libvirtd to crash at startup on my machine. Have a backtrace: Thread 19 (Thread 0x7fffaa4e0700 (LWP 31651)): #0 0x00007ffff72ecd31 in open64 () from /lib64/libc.so.6 #1 0x00007ffff727d3f6 in _IO_file_open () from /lib64/libc.so.6 #2 0x00007ffff727d5ad in __GI__IO_file_fopen () from /lib64/libc.so.6 #3 0x00007ffff727132d in __fopen_internal () from /lib64/libc.so.6 #4 0x00007ffff71199d8 in ?? () from /lib64/libudev.so.1 #5 0x00007ffff71146dd in ?? () from /lib64/libudev.so.1 #6 0x00007ffff71173ed in ?? () from /lib64/libudev.so.1 #7 0x00007ffff71179d9 in ?? () from /lib64/libudev.so.1 #8 0x00007ffff710be77 in udev_device_get_property_value () from /lib64/libudev.so.1 #9 0x00007fffc2e8a3ef in udevGetDeviceProperty (udev_device=udev_device@entry=0x7fff900606e0, property_key=property_key@entry=0x7fffc2ea3504 "DRIVER") at node_device/node_device_udev.c:140 #10 0x00007fffc2e8a459 in udevGetStringProperty (udev_device=udev_device@entry=0x7fff900606e0, property_key=property_key@entry=0x7fffc2ea3504 "DRIVER", value=0x7fff900f98d8) at node_device/node_device_udev.c:154 #11 0x00007fffc2e8b3ad in udevAddOneDevice (device=device@entry=0x7fff900606e0) at node_device/node_device_udev.c:1369 #12 0x00007fffc2e8d27e in udevProcessDeviceListEntry (list_entry=0x7fff9004f350, udev=0x7fff98120f00) at node_device/node_device_udev.c:1435 #13 udevEnumerateDevices (udev=0x7fff98120f00) at node_device/node_device_udev.c:1489 #14 nodeStateInitializeEnumerate (opaque=0x7fff98120f00) at node_device/node_device_udev.c:1798 #15 0x00007ffff7c5a2c2 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #16 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #17 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 18 (Thread 0x7fffaace1700 (LWP 31650)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x7fff98120640, m=m@entry=0x7fff98120600) at util/virthread.c:154 #2 0x00007fffc2e8d404 in udevEventHandleThread (opaque=<optimized out>) at node_device/node_device_udev.c:1618 #3 0x00007ffff7c5a2c2 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 17 (Thread 0x7fffab4e2700 (LWP 31607)): #0 0x00007ffff7cc1c85 in virDomainMomentAssignDef (moments=0x7fff981af8a0, def=def@entry=0x7fff98227870) at conf/virdomainmomentobjlist.c:230 #1 0x00007ffff7cc22b8 in virDomainSnapshotAssignDef (snapshots=<optimized out>, def=def@entry=0x7fff98227870) at conf/virdomainsnapshotobjlist.c:212 #2 0x00007fffc2b3dd6f in qemuDomainSnapshotLoad (vm=0x7fff982dfcc0, data=<optimized out>) at qemu/qemu_driver.c:481 #3 0x00007ffff7cbfeba in virDomainObjListHelper (payload=<optimized out>, name=<optimized out>, opaque=0x7fffab4e1960) at conf/virdomainobjlist.c:803 #4 0x00007ffff7c06200 in virHashForEach (data=<optimized out>, iter=<optimized out>, table=<optimized out>) at util/virhash.c:575 #5 virHashForEach (table=0x7fff981226c0, iter=iter@entry=0x7ffff7cbfeb0 <virDomainObjListHelper>, data=data@entry=0x7fffab4e1960) at util/virhash.c:563 #6 0x00007ffff7cc13a1 in virDomainObjListForEach (doms=0x7fff98122660, callback=callback@entry=0x7fffc2b3db40 <qemuDomainSnapshotLoad>, opaque=<optimized out>) at conf/virdomainobjlist.c:818 #7 0x00007fffc2b3ebc3 in qemuStateInitialize (privileged=true, callback=<optimized out>, opaque=<optimized out>) at qemu/qemu_driver.c:898 #8 0x00007ffff7e07fed in virStateInitialize (privileged=true, callback=0x555555578480 <daemonInhibitCallback>, opaque=0x555555605bc0) at libvirt.c:653 #9 0x00005555555784db in daemonRunStateInit (opaque=0x555555605bc0) at remote/remote_daemon.c:797 #10 0x00007ffff7c5a2c2 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #11 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #12 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 16 (Thread 0x7fffc0926700 (LWP 31606)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555663c38, m=m@entry=0x555555663c10) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x555555661a50) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 15 (Thread 0x7fffc1127700 (LWP 31605)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555663c38, m=m@entry=0x555555663c10) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x55555560bdc0) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 14 (Thread 0x7fffc1928700 (LWP 31604)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555663c38, m=m@entry=0x555555663c10) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x55555566a880) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 13 (Thread 0x7fffc2129700 (LWP 31603)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555663c38, m=m@entry=0x555555663c10) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x555555667640) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 12 (Thread 0x7fffc292a700 (LWP 31602)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555663c38, m=m@entry=0x555555663c10) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x555555653ae0) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 11 (Thread 0x7fffc3fff700 (LWP 31601)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x5555556060f8, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5afa4 in virThreadPoolWorker (opaque=opaque@entry=0x555555617ec0) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 10 (Thread 0x7fffd8ff9700 (LWP 31600)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x5555556060f8, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5afa4 in virThreadPoolWorker (opaque=opaque@entry=0x555555618090) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 9 (Thread 0x7fffd97fa700 (LWP 31599)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x5555556060f8, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5afa4 in virThreadPoolWorker (opaque=opaque@entry=0x5555556180e0) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 8 (Thread 0x7fffd9ffb700 (LWP 31598)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x5555556060f8, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5afa4 in virThreadPoolWorker (opaque=opaque@entry=0x555555618130) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 7 (Thread 0x7fffda7fc700 (LWP 31597)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x5555556060f8, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5afa4 in virThreadPoolWorker (opaque=opaque@entry=0x555555618260) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 6 (Thread 0x7fffdaffd700 (LWP 31596)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555606058, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x5555556182b0) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 5 (Thread 0x7fffdb7fe700 (LWP 31595)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555606058, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x555555618380) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 4 (Thread 0x7fffdbfff700 (LWP 31594)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555606058, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x555555618460) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 3 (Thread 0x7fffe8971700 (LWP 31593)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555606058, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x5555556185a0) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 2 (Thread 0x7fffe9172700 (LWP 31592)): #0 0x00007ffff741373c in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0 #1 0x00007ffff7c5a4f6 in virCondWait (c=c@entry=0x555555606058, m=m@entry=0x555555606030) at util/virthread.c:154 #2 0x00007ffff7c5aff3 in virThreadPoolWorker (opaque=opaque@entry=0x5555556192e0) at util/virthreadpool.c:120 #3 0x00007ffff7c5a298 in virThreadHelper (data=<optimized out>) at util/virthread.c:206 #4 0x00007ffff740d58e in start_thread () from /lib64/libpthread.so.0 #5 0x00007ffff72fc6a3 in clone () from /lib64/libc.so.6 Thread 1 (Thread 0x7ffff61c7b80 (LWP 31587)): #0 0x00007ffff72f1421 in poll () from /lib64/libc.so.6 #1 0x00007ffff7bf841b in poll (__timeout=-1, __nfds=6, __fds=<optimized out>) at /usr/include/bits/poll2.h:46 #2 virEventPollRunOnce () at util/vireventpoll.c:636 #3 0x00007ffff7bf6f71 in virEventRunDefaultImpl () at util/virevent.c:322 #4 0x00007ffff7d35f95 in virNetDaemonRun (dmn=0x555555605bc0) at rpc/virnetdaemon.c:847 #5 0x0000555555576e30 in main (argc=<optimized out>, argv=<optimized out>) at remote/remote_daemon.c:1455 If I revert back to 320a1480d0dbe77ae9da08b6ce6c3ad5e2706b63 or delete all snapshots, then it works fine. -- Andrea Bolognani / Red Hat / Virtualization