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@xxxxxxxxxx>
> ---
> 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.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
