On Tue, Jun 18, 2019 at 22:47:45 -0500, Eric Blake wrote:
Add a new file checkpoint_conf.c that performs the translation to
and
from new XML describing a checkpoint. The code shares a common base
class with snapshots, since a checkpoint similarly represents the
domain state at a moment in time. Add some basic testing of round trip
XML handling through the new code.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/conf/checkpoint_conf.h | 94 +++
src/conf/virconftypes.h | 3 +
po/POTFILES | 1 +
src/conf/Makefile.inc.am | 2 +
src/conf/checkpoint_conf.c | 575 ++++++++++++++++++
src/libvirt_private.syms | 8 +
tests/Makefile.am | 9 +-
.../internal-active-invalid.xml | 53 ++
.../internal-inactive-invalid.xml | 53 ++
tests/domaincheckpointxml2xmltest.c | 223 +++++++
10 files changed, 1019 insertions(+), 2 deletions(-)
create mode 100644 src/conf/checkpoint_conf.h
create mode 100644 src/conf/checkpoint_conf.c
create mode 100644 tests/domaincheckpointxml2xmlout/internal-active-invalid.xml
create mode 100644 tests/domaincheckpointxml2xmlout/internal-inactive-invalid.xml
create mode 100644 tests/domaincheckpointxml2xmltest.c
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
new file mode 100644
index 0000000000..a7a34d3887
--- /dev/null
+++ b/src/conf/checkpoint_conf.c
@@ -0,0 +1,575 @@
[...]
+static int
+virDomainCheckpointDiskDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virDomainCheckpointDiskDefPtr def)
+{
+ int ret = -1;
+ char *checkpoint = NULL;
+ char *bitmap = NULL;
+ xmlNodePtr saved = ctxt->node;
+
+ ctxt->node = node;
+
+ def->name = virXMLPropString(node, "name");
+ if (!def->name) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing name from disk checkpoint element"));
Shouldn't schema validation catch this? If schema validation is not
enabled note that there isn't a second chance to do it.
+ goto cleanup;
+ }
[...]
+
+ if (!(def = virDomainCheckpointDefNew()))
+ return NULL;
+
+ def->parent.name = virXPathString("string(./name)", ctxt);
+ if (def->parent.name == NULL) {
+ if (flags & VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("a redefined checkpoint must have a name"));
+ goto cleanup;
+ }
+ }
+
+ def->parent.description = virXPathString("string(./description)",
ctxt);
+
+ if (flags & VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE) {
+ if (virXPathLongLong("string(./creationTime)", ctxt,
+ &def->parent.creationTime) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing creationTime from existing
checkpoint"));
While I can see the point that we do the same thing in the snapshot XML
parser, having validation intermixed with the parser is not great for
the future.
+ goto cleanup;
+ }
+
+ def->parent.parent_name = virXPathString("string(./parent/name)",
ctxt);
+
+ if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
+ int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+ VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
+ xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
ACK, this adds bunch of technical debt based on the fact that it's
mostly copied form snapshots, but I trust you will address that later.
I'd strongly prefer if we'd do schema validation unconditionally for the
new APIs though since we won't get to fix that one later.