12:40 a.m.
The only use for the 'current' member of virDomainSnapshotDef was with
the PARSE/FORMAT_INTERNAL flag for controlling an internal-use
<active> element marking whether a particular snapshot definition was
current, and even then, only by the qemu driver on output, and by qemu
and test driver on input. But this duplicates vm->snapshot_current,
and gets in the way of potential simplifications to have qemu store a
single file for all snapshots rather than one file per snapshot. Get
rid of the member by adding a bool* parameter during parse (ignored if
the PARSE_INTERNAL flag is not set), and by adding a new flag during
format (if FORMAT_INTERNAL is set, the value printed in <active>
depends on the new FORMAT_CURRENT). Then update the qemu driver
accordingly.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/conf/snapshot_conf.h | 6 ++--
src/conf/snapshot_conf.c | 18 ++++++----
src/conf/virdomainsnapshotobjlist.c | 4 +--
src/esx/esx_driver.c | 2 +-
src/qemu/qemu_domain.c | 18 +++++-----
src/qemu/qemu_driver.c | 51 +++++++++++++++--------------
src/test/test_driver.c | 5 ++-
src/vbox/vbox_common.c | 4 +--
src/vz/vz_driver.c | 3 +-
tests/domainsnapshotxml2xmltest.c | 5 ++-
10 files changed, 66 insertions(+), 50 deletions(-)
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 7230b9950f..b13a500af4 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -88,9 +88,6 @@ struct _virDomainSnapshotDef {
virDomainDefPtr dom;
virObjectPtr cookie;
-
- /* Internal use. */
- bool current; /* At most one snapshot in the list should have this set */
};
typedef enum {
@@ -103,6 +100,7 @@ typedef enum {
typedef enum {
VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE = 1 << 0,
VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL = 1 << 1,
+ VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT = 1 << 2,
} virDomainSnapshotFormatFlags;
unsigned int virDomainSnapshotFormatConvertXMLFlags(unsigned int flags);
@@ -110,11 +108,13 @@ unsigned int virDomainSnapshotFormatConvertXMLFlags(unsigned int
flags);
virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ bool *current,
unsigned int flags);
virDomainSnapshotDefPtr virDomainSnapshotDefParseNode(xmlDocPtr xml,
xmlNodePtr root,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ bool *current,
unsigned int flags);
void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def);
char *virDomainSnapshotDefFormat(const char *uuidstr,
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ffb1313c89..bf5fdc0647 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -184,12 +184,14 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
/* flags is bitwise-or of virDomainSnapshotParseFlags.
* If flags does not include VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE, then
- * caps are ignored.
+ * caps are ignored. If flags does not include
+ * VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL, then current is ignored.
*/
static virDomainSnapshotDefPtr
virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ bool *current,
unsigned int flags)
{
virDomainSnapshotDefPtr def = NULL;
@@ -350,7 +352,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
_("Could not find 'active' element"));
goto cleanup;
}
- def->current = active != 0;
+ *current = active != 0;
}
if (!offline && virSaveCookieParse(ctxt, &def->cookie, saveCookie)
< 0)
@@ -374,6 +376,7 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
xmlNodePtr root,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ bool *current,
unsigned int flags)
{
xmlXPathContextPtr ctxt = NULL;
@@ -391,7 +394,7 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
}
ctxt->node = root;
- def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, flags);
+ def = virDomainSnapshotDefParse(ctxt, caps, xmlopt, current, flags);
cleanup:
xmlXPathFreeContext(ctxt);
return def;
@@ -401,6 +404,7 @@ virDomainSnapshotDefPtr
virDomainSnapshotDefParseString(const char *xmlStr,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ bool *current,
unsigned int flags)
{
virDomainSnapshotDefPtr ret = NULL;
@@ -410,7 +414,7 @@ virDomainSnapshotDefParseString(const char *xmlStr,
if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)")))) {
xmlKeepBlanksDefault(keepBlanksDefault);
ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml),
- caps, xmlopt, flags);
+ caps, xmlopt, current, flags);
xmlFreeDoc(xml);
}
xmlKeepBlanksDefault(keepBlanksDefault);
@@ -849,7 +853,8 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
goto error;
if (flags & VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL)
- virBufferAsprintf(buf, "<active>%d</active>\n",
def->current);
+ virBufferAsprintf(buf, "<active>%d</active>\n",
+ !!(flags & VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT));
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</domainsnapshot>\n");
@@ -875,7 +880,8 @@ virDomainSnapshotDefFormat(const char *uuidstr,
virBuffer buf = VIR_BUFFER_INITIALIZER;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE |
- VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL, NULL);
+ VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL |
+ VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT, NULL);
if (virDomainSnapshotDefFormatInternal(&buf, uuidstr, def, caps,
xmlopt, flags) < 0)
return NULL;
diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index e2f2110108..be44bdde71 100644
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -108,7 +108,8 @@ virDomainSnapshotObjListParse(const char *xmlStr,
virDomainSnapshotDefPtr def;
virDomainSnapshotObjPtr snap;
- def = virDomainSnapshotDefParseNode(xml, nodes[i], caps, xmlopt, flags);
+ def = virDomainSnapshotDefParseNode(xml, nodes[i], caps, xmlopt, NULL,
+ flags);
if (!def)
goto cleanup;
if (!(snap = virDomainSnapshotAssignDef(snapshots, def))) {
@@ -134,7 +135,6 @@ virDomainSnapshotObjListParse(const char *xmlStr,
_("no snapshot matching current='%s'"),
current);
goto cleanup;
}
- (*current_snap)->def->current = true;
}
ret = 0;
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c016f8051f..1c6a2dcb71 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4102,7 +4102,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char
*xmlDesc,
return NULL;
def = virDomainSnapshotDefParseString(xmlDesc, priv->caps,
- priv->xmlopt, 0);
+ priv->xmlopt, NULL, 0);
if (!def)
return NULL;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 86e80391e1..f8387339f0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8458,11 +8458,14 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
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;
+ if (vm->current_snapshot == snapshot)
+ flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT;
virUUIDFormat(vm->def->uuid, uuidstr);
- newxml = virDomainSnapshotDefFormat(
- uuidstr, snapshot->def, caps, xmlopt,
- VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE | VIR_DOMAIN_SNAPSHOT_FORMAT_INTERNAL);
+ newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, caps, xmlopt,
+ flags);
if (newxml == NULL)
return -1;
@@ -8612,6 +8615,7 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
goto cleanup;
if (snap == vm->current_snapshot) {
+ vm->current_snapshot = NULL;
if (update_parent && snap->def->parent) {
parentsnap = virDomainSnapshotFindByName(vm->snapshots,
snap->def->parent);
@@ -8619,18 +8623,16 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
VIR_WARN("missing parent snapshot matching name '%s'",
snap->def->parent);
} else {
- parentsnap->def->current = true;
+ vm->current_snapshot = 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);
- parentsnap->def->current = false;
- parentsnap = NULL;
+ vm->current_snapshot = NULL;
}
}
}
- vm->current_snapshot = parentsnap;
}
if (unlink(snapFile) < 0)
@@ -8656,7 +8658,7 @@ int qemuDomainSnapshotDiscardAll(void *payload,
virQEMUSnapRemovePtr curr = data;
int err;
- if (snap->def->current)
+ if (curr->vm->current_snapshot == snap)
curr->current = true;
err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false,
curr->metadata_only);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6e504dd17c..7e0e76a31a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -419,6 +419,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
virDomainSnapshotDefPtr def = NULL;
virDomainSnapshotObjPtr snap = NULL;
virDomainSnapshotObjPtr current = NULL;
+ bool cur;
unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE |
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL);
@@ -465,7 +466,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
}
def = virDomainSnapshotDefParseString(xmlStr, caps,
- qemu_driver->xmlopt,
+ qemu_driver->xmlopt, &cur,
flags);
if (def == NULL) {
/* Nothing we can do here, skip this one */
@@ -480,7 +481,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
snap = virDomainSnapshotAssignDef(vm->snapshots, def);
if (snap == NULL) {
virDomainSnapshotDefFree(def);
- } else if (snap->def->current) {
+ } else if (cur) {
current = snap;
if (!vm->current_snapshot)
vm->current_snapshot = snap;
@@ -15661,6 +15662,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
virDomainSnapshotObjPtr snap = NULL;
virDomainSnapshotPtr snapshot = NULL;
virDomainSnapshotDefPtr def = NULL;
+ virDomainSnapshotObjPtr current = NULL;
bool update_current = true;
bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
@@ -15722,7 +15724,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE;
if (!(def = virDomainSnapshotDefParseString(xmlDesc, caps, driver->xmlopt,
- parse_flags)))
+ NULL, parse_flags)))
goto cleanup;
/* reject snapshot names containing slashes or starting with dot as
@@ -15856,19 +15858,17 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
def = NULL;
}
- if (update_current)
- snap->def->current = true;
- if (vm->current_snapshot) {
+ current = vm->current_snapshot;
+ if (current) {
if (!redefine &&
- VIR_STRDUP(snap->def->parent, vm->current_snapshot->def->name)
< 0)
+ VIR_STRDUP(snap->def->parent, current->def->name) < 0)
goto endjob;
if (update_current) {
- vm->current_snapshot->def->current = false;
- if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
+ vm->current_snapshot = NULL;
+ if (qemuDomainSnapshotWriteMetadata(vm, current,
driver->caps, driver->xmlopt,
cfg->snapshotDir) < 0)
goto endjob;
- vm->current_snapshot = NULL;
}
}
@@ -15914,6 +15914,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
endjob:
if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
+ if (update_current)
+ vm->current_snapshot = snap;
if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps,
driver->xmlopt,
cfg->snapshotDir) < 0) {
@@ -15925,9 +15927,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
_("unable to save metadata for snapshot %s"),
snap->def->name);
virDomainSnapshotObjListRemove(vm->snapshots, snap);
+ vm->current_snapshot = NULL;
} else {
- if (update_current)
- vm->current_snapshot = snap;
other = virDomainSnapshotFindByName(vm->snapshots,
snap->def->parent);
snap->parent = other;
@@ -16347,6 +16348,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virDomainObjPtr vm = NULL;
int ret = -1;
virDomainSnapshotObjPtr snap = NULL;
+ virDomainSnapshotObjPtr current = NULL;
virObjectEventPtr event = NULL;
virObjectEventPtr event2 = NULL;
int detail;
@@ -16441,14 +16443,13 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
}
}
-
- if (vm->current_snapshot) {
- vm->current_snapshot->def->current = false;
- if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
+ current = vm->current_snapshot;
+ if (current) {
+ vm->current_snapshot = NULL;
+ if (qemuDomainSnapshotWriteMetadata(vm, current,
driver->caps, driver->xmlopt,
cfg->snapshotDir) < 0)
goto endjob;
- vm->current_snapshot = NULL;
/* XXX Should we restore vm->current_snapshot after this point
* in the failure cases where we know there was no change? */
}
@@ -16458,7 +16459,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
*
* XXX Should domain snapshots track live xml rather
* than inactive xml? */
- snap->def->current = true;
+ vm->current_snapshot = snap;
if (snap->def->dom) {
config = virDomainDefCopy(snap->def->dom, caps,
driver->xmlopt, NULL, true);
@@ -16729,14 +16730,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
cleanup:
if (ret == 0) {
+ vm->current_snapshot = snap;
if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps,
driver->xmlopt,
- cfg->snapshotDir) < 0)
+ cfg->snapshotDir) < 0) {
+ vm->current_snapshot = NULL;
ret = -1;
- else
- vm->current_snapshot = snap;
+ }
} else if (snap) {
- snap->def->current = false;
+ vm->current_snapshot = NULL;
}
if (ret == 0 && config && vm->persistent &&
!(ret = virDomainSaveConfig(cfg->configDir, driver->caps,
@@ -16864,19 +16866,18 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
if (rem.err < 0)
goto endjob;
if (rem.current) {
+ vm->current_snapshot = snap;
if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
- snap->def->current = true;
if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps,
driver->xmlopt,
cfg->snapshotDir) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to set snapshot '%s' as
current"),
snap->def->name);
- snap->def->current = false;
+ vm->current_snapshot = NULL;
goto endjob;
}
}
- vm->current_snapshot = snap;
}
} else if (snap->nchildren) {
rep.cfg = cfg;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e1ad9382e0..ca480c1b21 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -819,6 +819,7 @@ testParseDomainSnapshots(testDriverPtr privconn,
int ret = -1;
testDomainNamespaceDefPtr nsdata = domobj->def->namespaceData;
xmlNodePtr *nodes = nsdata->snap_nodes;
+ bool cur;
for (i = 0; i < nsdata->num_snap_nodes; i++) {
virDomainSnapshotObjPtr snap;
@@ -831,6 +832,7 @@ testParseDomainSnapshots(testDriverPtr privconn,
def = virDomainSnapshotDefParseNode(ctxt->doc, node,
privconn->caps,
privconn->xmlopt,
+ &cur,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL |
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
@@ -842,7 +844,7 @@ testParseDomainSnapshots(testDriverPtr privconn,
goto error;
}
- if (def->current) {
+ if (cur) {
if (domobj->current_snapshot) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("more than one snapshot claims to be
active"));
@@ -6363,6 +6365,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
if (!(def = virDomainSnapshotDefParseString(xmlDesc,
privconn->caps,
privconn->xmlopt,
+ NULL,
parse_flags)))
goto cleanup;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index ac7e02eed6..2ca12a28e5 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -5502,7 +5502,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT, NULL);
if (!(def = virDomainSnapshotDefParseString(xmlDesc, data->caps,
- data->xmlopt,
+ data->xmlopt, NULL,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE)))
goto cleanup;
@@ -6948,7 +6948,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
}
def = virDomainSnapshotDefParseString(defXml,
data->caps,
- data->xmlopt,
+ data->xmlopt, NULL,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
if (!def) {
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 1bf6daf9b0..eba366dd2c 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -2618,7 +2618,8 @@ vzDomainSnapshotCreateXML(virDomainPtr domain,
goto cleanup;
if (!(def = virDomainSnapshotDefParseString(xmlDesc, driver->caps,
- driver->xmlopt, parse_flags)))
+ driver->xmlopt, NULL,
+ parse_flags)))
goto cleanup;
if (def->ndisks > 0) {
diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c
index 9eb71780fc..9f7f98585f 100644
--- a/tests/domainsnapshotxml2xmltest.c
+++ b/tests/domainsnapshotxml2xmltest.c
@@ -80,6 +80,7 @@ testCompareXMLToXMLFiles(const char *inxml,
virDomainSnapshotDefPtr def = NULL;
unsigned int parseflags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
unsigned int formatflags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE;
+ bool cur;
if (internal) {
parseflags |= VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL;
@@ -96,9 +97,11 @@ testCompareXMLToXMLFiles(const char *inxml,
goto cleanup;
if (!(def = virDomainSnapshotDefParseString(inXmlData, driver.caps,
- driver.xmlopt,
+ driver.xmlopt, &cur,
parseflags)))
goto cleanup;
+ if (cur)
+ formatflags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT;
if (!(actual = virDomainSnapshotDefFormat(uuid, def, driver.caps,
driver.xmlopt,
--
2.20.1