External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-31 14:07:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

snapshot: Drop virDomainSnapshotDef.current

Browse Source 
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, which involves hoisting
assignments to vm->current_snapshot to occur prior to any point where
a snapshot XML file is written (although qemu kept
vm->current_snapshot and snapshot->def_current in sync by the end of
the function, they were not always identical in the middle of
functions, so the shuffling gets a bit interesting). Later patches
will clean up some of that confusing churn to vm->current_snapshot.

Note: even if later patches refactor qemu to no longer use
FORMAT_INTERNAL for output (by storing bulk snapshot XML instead), we
will always need PARSE_INTERNAL for input (because on upgrade, a new
libvirt still has to parse XML left from a previous libvirt).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
Eric Blake 2019-03-18 22:56:19 -05:00
parent 0baf6945ed
commit f105627992
10 changed files with 71 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/conf/snapshot_conf.c
View File

@ -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;
@ -345,12 +347,17 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
}
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
if (!current) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("internal parse requested with NULL current"));
goto cleanup;
}
if (virXPathInt("string(./active)", ctxt, &active) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not find 'active' element"));
goto cleanup;
}
def->current = active != 0;
*current = active != 0;
}
if (!offline && virSaveCookieParse(ctxt, &def->cookie, saveCookie) < 0)
@ -374,6 +381,7 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
xmlNodePtr root,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
bool *current,
unsigned int flags)
{
xmlXPathContextPtr ctxt = NULL;
@ -391,7 +399,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 +409,7 @@ virDomainSnapshotDefPtr
virDomainSnapshotDefParseString(const char *xmlStr,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
bool *current,
unsigned int flags)
{
virDomainSnapshotDefPtr ret = NULL;
@ -410,7 +419,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 +858,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 +885,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;

6
src/conf/snapshot_conf.h
View File

@ -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,

4
src/conf/virdomainsnapshotobjlist.c
View File

@ -105,7 +105,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))) {
@ -131,7 +132,6 @@ virDomainSnapshotObjListParse(const char *xmlStr,
_("no snapshot matching current='%s'"), current);
goto cleanup;
}
(*current_snap)->def->current = true;
}
ret = n;

2
src/esx/esx_driver.c
View File

@ -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;

18
src/qemu/qemu_domain.c
View File

@ -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);

51
src/qemu/qemu_driver.c
View File

@ -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;

5
src/test/test_driver.c
View File

@ -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;

4
src/vbox/vbox_common.c
View File

@ -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) {

3
src/vz/vz_driver.c
View File

@ -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) {

5
tests/domainsnapshotxml2xmltest.c
View File

@ -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,