6:36 p.m.
When asked to parse a branch snapshot XML definition, we have to
piece together the definition of the new snapshot from parts of
the branch point, as well as run some sanity checks that the
branches are compatible. This patch is rather restrictive in
what it allows; depending on effort and future needs, we may be
able to relax some of those restrictions and allow more cases.
* src/conf/snapshot_conf.c (virDomainSnapshotDefParseString):
Honor new flag.
---
src/conf/snapshot_conf.c | 112 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 106 insertions(+), 6 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 8a3146f..909a750 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -171,7 +171,7 @@ virDomainSnapshotDefPtr
virDomainSnapshotDefParseString(const char *xmlStr,
virCapsPtr caps,
unsigned int expectedVirtTypes,
- virDomainSnapshotObjListPtr snapshots ATTRIBUTE_UNUSED,
+ virDomainSnapshotObjListPtr snapshots,
unsigned int flags)
{
xmlXPathContextPtr ctxt = NULL;
@@ -181,6 +181,7 @@ virDomainSnapshotDefParseString(const char *xmlStr,
xmlNodePtr *nodes = NULL;
int i;
char *creation = NULL, *state = NULL;
+ char *branch = NULL;
struct timeval tv;
int active;
char *tmp;
@@ -188,6 +189,7 @@ virDomainSnapshotDefParseString(const char *xmlStr,
char *memorySnapshot = NULL;
char *memoryFile = NULL;
bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
+ virDomainSnapshotObjPtr other = NULL;
xml = virXMLParseCtxt(NULL, xmlStr, _("(domain_snapshot)"), &ctxt);
if (!xml) {
@@ -223,7 +225,84 @@ virDomainSnapshotDefParseString(const char *xmlStr,
def->description = virXPathString("string(./description)", ctxt);
- if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+ branch = virXPathString("string(./branch)", ctxt);
+ memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt);
+ memoryFile = virXPathString("string(./memory/@file)", ctxt);
+
+ if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH) {
+ if ((flags & (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE |
+ VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL)) ||
+ !snapshots) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unexpected parse request"));
+ goto cleanup;
+ }
+
+ /* In order to form a branch, we must find the existing
+ * snapshot, and it must be external. */
+ if (!branch) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("use of branch flag requires <branch>
element"));
+ goto cleanup;
+ }
+ other = virDomainSnapshotFindByName(snapshots, branch);
+ if (!other) {
+ virReportError(VIR_ERR_XML_ERROR, _("could not find branch
'%s'"),
+ branch);
+ goto cleanup;
+ }
+
+ if (!virDomainSnapshotIsExternal(other)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("branch '%s' is not an external
snapshot"),
+ branch);
+ goto cleanup;
+ }
+ if (!other->def->dom) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("branch '%s' lacks corresponding domain
details"),
+ branch);
+ goto cleanup;
+ }
+ if (!(def->dom = virDomainDefCopy(caps, other->def->dom, true)))
+ goto cleanup;
+
+ /* The new definition shares the same <parent>, <state>, and
+ * <domain> as what it is branching from. */
+ def->creationTime = tv.tv_sec;
+ if (other->def->parent &&
+ !(def->parent = strdup(other->def->parent))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ def->state = other->def->state;
+
+ /* Branches reuse existing memory files (with bookkeeping to
+ * only delete the file when the last snapshot reference is
+ * deleted). Thus, it is an error to explicitly request
+ * memory with a branch. */
+ if (memorySnapshot || memoryFile) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("cannot specify <memory> and <branch>
together"));
+ goto cleanup;
+ }
+ def->memory = other->def->memory;
+ if (other->def->file &&
+ !(def->file = strdup(other->def->file))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ /* Any <disk> in the XML must be consistent with the branch
+ * point, and any <disk> not in the XML will be populated to
+ * match the branch; checked below. */
+ } else if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+ if (branch) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("branch '%s' incompatible with redefine
flag"),
+ branch);
+ goto cleanup;
+ }
if (virXPathLongLong("string(./creationTime)", ctxt,
&def->creationTime) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -275,11 +354,15 @@ virDomainSnapshotDefParseString(const char *xmlStr,
VIR_WARN("parsing older snapshot that lacks domain");
}
} else {
+ if (branch) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("branch '%s' requires use of branch
flag"),
+ branch);
+ goto cleanup;
+ }
def->creationTime = tv.tv_sec;
}
- memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt);
- memoryFile = virXPathString("string(./memory/@file)", ctxt);
if (memorySnapshot) {
def->memory = virDomainSnapshotLocationTypeFromString(memorySnapshot);
if (def->memory <= 0) {
@@ -314,8 +397,10 @@ virDomainSnapshotDefParseString(const char *xmlStr,
_("memory state cannot be saved with offline
snapshot"));
goto cleanup;
}
- def->file = memoryFile;
- memoryFile = NULL;
+ if (memoryFile) {
+ def->file = memoryFile;
+ memoryFile = NULL;
+ }
if ((i = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0)
goto cleanup;
@@ -335,6 +420,20 @@ virDomainSnapshotDefParseString(const char *xmlStr,
_("unable to handle disk requests in snapshot"));
goto cleanup;
}
+ if (other) {
+ /* For now, we only allow branch snapshots of external snapshots. */
+ if (virDomainSnapshotAlignDisks(def,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
+ true) < 0)
+ goto cleanup;
+ for (i = 0; i < def->ndisks; i++)
+ if (def->disks[i].snapshot != other->def->disks[i].snapshot) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("mismatch in snapshot mode for disk
'%s'"),
+ def->disks[i].name);
+ goto cleanup;
+ }
+ }
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
if (virXPathInt("string(./active)", ctxt, &active) < 0) {
@@ -353,6 +452,7 @@ cleanup:
VIR_FREE(nodes);
VIR_FREE(memorySnapshot);
VIR_FREE(memoryFile);
+ VIR_FREE(branch);
xmlXPathFreeContext(ctxt);
if (ret == NULL)
virDomainSnapshotDefFree(def);
--
1.7.11.7