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 <[email protected]> > --- > 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.
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
