On Wed, Jul 24, 2019 at 12:55:58AM -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. > > Of note - this code intentionally differs from snapshots in that XML > schema validation is unconditional, rather than based on a public API > flag. I'm in two minds as to whether this is really a good idea. I always have a strong tendancy to prefer consistency across the APIs we expose, which would point to an opt-in flag for validation. If we did later find we need a way to skip validation, then we end up having to add a NO_VALIDATION API flag, which makes inconsitency even more glaring to app devs I understand that this can help catch mistakes in apps, but I'm not sold on it as a default, when it is easy for apps to opt-in if they want to. > Also, the redefine flag requires the <domain> sub-element to be > present, rather than catering to historical back-compat to older > versions. > > Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> > --- > src/conf/checkpoint_conf.h | 84 +++ > src/conf/virconftypes.h | 3 + > po/POTFILES | 1 + > src/conf/Makefile.inc.am | 2 + > src/conf/checkpoint_conf.c | 535 ++++++++++++++++++ > src/libvirt_private.syms | 8 + > tests/Makefile.am | 9 +- > .../internal-active-invalid.xml | 53 ++ > .../internal-inactive-invalid.xml | 53 ++ > tests/qemudomaincheckpointxml2xmltest.c | 213 +++++++ > 10 files changed, 959 insertions(+), 2 deletions(-) > create mode 100644 src/conf/checkpoint_conf.h > create mode 100644 src/conf/checkpoint_conf.c > create mode 100644 tests/qemudomaincheckpointxml2xmlout/internal-active-invalid.xml > create mode 100644 tests/qemudomaincheckpointxml2xmlout/internal-inactive-invalid.xml > create mode 100644 tests/qemudomaincheckpointxml2xmltest.c > > diff --git a/src/conf/checkpoint_conf.h b/src/conf/checkpoint_conf.h > new file mode 100644 > index 0000000000..0fac521efe > --- /dev/null > +++ b/src/conf/checkpoint_conf.h > @@ -0,0 +1,84 @@ > +/* > + * checkpoint_conf.h: domain checkpoint XML processing > + * (based on snapshot_conf.h) > + * > + * Copyright (C) 2006-2019 Red Hat, Inc. > + * Copyright (C) 2006-2008 Daniel P. Berrange > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + */ > + > +#pragma once > + > +#include "internal.h" > +#include "domain_conf.h" > +#include "moment_conf.h" > +#include "virobject.h" > + > +/* Items related to checkpoint state */ > + > +typedef enum { > + VIR_DOMAIN_CHECKPOINT_TYPE_DEFAULT = 0, > + VIR_DOMAIN_CHECKPOINT_TYPE_NONE, > + VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP, > + > + VIR_DOMAIN_CHECKPOINT_TYPE_LAST > +} virDomainCheckpointType; > + > +/* Stores disk-checkpoint information */ > +typedef struct _virDomainCheckpointDiskDef virDomainCheckpointDiskDef; > +typedef virDomainCheckpointDiskDef *virDomainCheckpointDiskDefPtr; > +struct _virDomainCheckpointDiskDef { > + char *name; /* name matching the <target dev='...' of the domain */ > + int idx; /* index within checkpoint->dom->disks that matches name */ > + int type; /* virDomainCheckpointType */ > + char *bitmap; /* bitmap name, if type is bitmap */ > + unsigned long long size; /* current checkpoint size in bytes */ > +}; > + > +/* Stores the complete checkpoint metadata */ > +struct _virDomainCheckpointDef { > + virDomainMomentDef parent; > + > + /* Additional Public XML. */ > + size_t ndisks; /* should not exceed dom->ndisks */ > + virDomainCheckpointDiskDef *disks; > +}; > + > + > +typedef enum { > + VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE = 1 << 0, > +} virDomainCheckpointParseFlags; > + > +typedef enum { > + VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE = 1 << 0, > + VIR_DOMAIN_CHECKPOINT_FORMAT_NO_DOMAIN = 1 << 1, > + VIR_DOMAIN_CHECKPOINT_FORMAT_SIZE = 1 << 2, > +} virDomainCheckpointFormatFlags; > + > +unsigned int virDomainCheckpointFormatConvertXMLFlags(unsigned int flags); > + > +virDomainCheckpointDefPtr virDomainCheckpointDefParseString(const char *xmlStr, > + virCapsPtr caps, > + virDomainXMLOptionPtr xmlopt, > + unsigned int flags); > +virDomainCheckpointDefPtr virDomainCheckpointDefNew(void); > +char *virDomainCheckpointDefFormat(virDomainCheckpointDefPtr def, > + virCapsPtr caps, > + virDomainXMLOptionPtr xmlopt, > + unsigned int flags); > +int virDomainCheckpointAlignDisks(virDomainCheckpointDefPtr checkpoint); > + > +VIR_ENUM_DECL(virDomainCheckpoint); > diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h > index b8f553f7fb..fe3f0af14f 100644 > --- a/src/conf/virconftypes.h > +++ b/src/conf/virconftypes.h > @@ -102,6 +102,9 @@ typedef virDomainBlkiotune *virDomainBlkiotunePtr; > typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; > typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; > > +typedef struct _virDomainCheckpointDef virDomainCheckpointDef; > +typedef virDomainCheckpointDef *virDomainCheckpointDefPtr; > + > typedef struct _virDomainChrDef virDomainChrDef; > typedef virDomainChrDef *virDomainChrDefPtr; > > diff --git a/po/POTFILES b/po/POTFILES > index 5d0bf43f02..2c27049d55 100644 > --- a/po/POTFILES > +++ b/po/POTFILES > @@ -15,6 +15,7 @@ src/bhyve/bhyve_monitor.c > src/bhyve/bhyve_parse_command.c > src/bhyve/bhyve_process.c > src/conf/capabilities.c > +src/conf/checkpoint_conf.c > src/conf/cpu_conf.c > src/conf/device_conf.c > src/conf/domain_addr.c > diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am > index 16a21c34f3..f656725e3a 100644 > --- a/src/conf/Makefile.inc.am > +++ b/src/conf/Makefile.inc.am > @@ -14,6 +14,8 @@ NETDEV_CONF_SOURCES = \ > DOMAIN_CONF_SOURCES = \ > conf/capabilities.c \ > conf/capabilities.h \ > + conf/checkpoint_conf.c \ > + conf/checkpoint_conf.h \ > conf/domain_addr.c \ > conf/domain_addr.h \ > conf/domain_capabilities.c \ > diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c > new file mode 100644 > index 0000000000..69c9040a43 > --- /dev/null > +++ b/src/conf/checkpoint_conf.c > @@ -0,0 +1,535 @@ > +/* > + * checkpoint_conf.c: domain checkpoint XML processing > + * > + * Copyright (C) 2006-2019 Red Hat, Inc. > + * Copyright (C) 2006-2008 Daniel P. Berrange > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + */ > + > +#include <config.h> > + > +#include "configmake.h" > +#include "internal.h" > +#include "virbitmap.h" > +#include "virbuffer.h" > +#include "datatypes.h" > +#include "domain_conf.h" > +#include "virlog.h" > +#include "viralloc.h" > +#include "checkpoint_conf.h" > +#include "virstoragefile.h" > +#include "viruuid.h" > +#include "virfile.h" > +#include "virerror.h" > +#include "virxml.h" > +#include "virstring.h" > + > +#define VIR_FROM_THIS VIR_FROM_DOMAIN_CHECKPOINT > + > +VIR_LOG_INIT("conf.checkpoint_conf"); > + > +static virClassPtr virDomainCheckpointDefClass; > +static void virDomainCheckpointDefDispose(void *obj); > + > +static int > +virDomainCheckpointOnceInit(void) > +{ > + if (!VIR_CLASS_NEW(virDomainCheckpointDef, virClassForDomainMomentDef())) > + return -1; > + > + return 0; > +} > + > +VIR_ONCE_GLOBAL_INIT(virDomainCheckpoint); > + > +VIR_ENUM_IMPL(virDomainCheckpoint, > + VIR_DOMAIN_CHECKPOINT_TYPE_LAST, > + "default", "no", "bitmap"); > + > + > +/* Checkpoint Def functions */ > +static void > +virDomainCheckpointDiskDefClear(virDomainCheckpointDiskDefPtr disk) > +{ > + VIR_FREE(disk->name); > + VIR_FREE(disk->bitmap); > +} > + > +/* Allocate a new virDomainCheckpointDef; free with virObjectUnref() */ > +virDomainCheckpointDefPtr > +virDomainCheckpointDefNew(void) > +{ > + virDomainCheckpointDefPtr def; > + > + if (virDomainCheckpointInitialize() < 0) > + return NULL; > + > + def = virObjectNew(virDomainCheckpointDefClass); > + return def; > +} > + > +static void > +virDomainCheckpointDefDispose(void *obj) > +{ > + virDomainCheckpointDefPtr def = obj; > + size_t i; > + > + for (i = 0; i < def->ndisks; i++) > + virDomainCheckpointDiskDefClear(&def->disks[i]); > + VIR_FREE(def->disks); > +} > + > +static int > +virDomainCheckpointDiskDefParseXML(xmlNodePtr node, > + xmlXPathContextPtr ctxt, > + virDomainCheckpointDiskDefPtr def) > +{ > + VIR_AUTOFREE(char *) checkpoint = NULL; > + VIR_XPATH_NODE_AUTORESTORE(ctxt); > + > + ctxt->node = node; > + > + /* Schema guarantees this is non-NULL: */ > + def->name = virXMLPropString(node, "name"); > + > + checkpoint = virXMLPropString(node, "checkpoint"); > + if (checkpoint) > + /* Schema guarantees this is in range: */ > + def->type = virDomainCheckpointTypeFromString(checkpoint); > + else > + def->type = VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP; > + > + def->bitmap = virXMLPropString(node, "bitmap"); > + > + return 0; > +} > + > +/* flags is bitwise-or of virDomainCheckpointParseFlags. If flags > + * does not include VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE, then caps > + * is ignored. > + */ > +static virDomainCheckpointDefPtr > +virDomainCheckpointDefParse(xmlXPathContextPtr ctxt, > + virCapsPtr caps, > + virDomainXMLOptionPtr xmlopt, > + unsigned int flags) > +{ > + virDomainCheckpointDefPtr ret = NULL; > + size_t i; > + int n; > + char *tmp; > + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; > + VIR_AUTOFREE(char *)creation = NULL; > + VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL; > + > + 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")); > + return NULL; > + } > + } > + > + 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")); > + return NULL; > + } > + > + 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); > + > + VIR_FREE(tmp); > + if (!domainNode) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("missing domain in checkpoint")); > + return NULL; > + } > + def->parent.dom = virDomainDefParseNode(ctxt->node->doc, domainNode, > + caps, xmlopt, NULL, > + domainflags); > + if (!def->parent.dom) > + return NULL; > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("missing domain in checkpoint redefine")); > + return NULL; > + } > + } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->parent) < 0) { > + return NULL; > + } > + > + if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0) > + return NULL; > + if (n && VIR_ALLOC_N(def->disks, n) < 0) > + return NULL; > + def->ndisks = n; > + for (i = 0; i < def->ndisks; i++) { > + if (virDomainCheckpointDiskDefParseXML(nodes[i], ctxt, > + &def->disks[i]) < 0) > + return NULL; > + } > + > + VIR_STEAL_PTR(ret, def); > + return ret; > +} > + > +static virDomainCheckpointDefPtr > +virDomainCheckpointDefParseNode(xmlDocPtr xml, > + xmlNodePtr root, > + virCapsPtr caps, > + virDomainXMLOptionPtr xmlopt, > + unsigned int flags) > +{ > + xmlXPathContextPtr ctxt = NULL; > + virDomainCheckpointDefPtr def = NULL; > + VIR_AUTOFREE(char *) schema = NULL; > + > + if (!virXMLNodeNameEqual(root, "domaincheckpoint")) { > + virReportError(VIR_ERR_XML_ERROR, "%s", _("domaincheckpoint")); > + goto cleanup; > + } > + > + /* This is a new enough API to make schema validation unconditional */ > + schema = virFileFindResource("domaincheckpoint.rng", > + abs_top_srcdir "/docs/schemas", > + PKGDATADIR "/schemas"); > + if (!schema) > + return NULL; > + if (virXMLValidateAgainstSchema(schema, xml) < 0) > + return NULL; > + > + ctxt = xmlXPathNewContext(xml); > + if (ctxt == NULL) { > + virReportOOMError(); > + goto cleanup; > + } > + > + ctxt->node = root; > + def = virDomainCheckpointDefParse(ctxt, caps, xmlopt, flags); > + cleanup: > + xmlXPathFreeContext(ctxt); > + return def; > +} > + > +virDomainCheckpointDefPtr > +virDomainCheckpointDefParseString(const char *xmlStr, > + virCapsPtr caps, > + virDomainXMLOptionPtr xmlopt, > + unsigned int flags) > +{ > + virDomainCheckpointDefPtr ret = NULL; > + xmlDocPtr xml; > + int keepBlanksDefault = xmlKeepBlanksDefault(0); > + > + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)")))) { > + xmlKeepBlanksDefault(keepBlanksDefault); > + ret = virDomainCheckpointDefParseNode(xml, xmlDocGetRootElement(xml), > + caps, xmlopt, flags); > + xmlFreeDoc(xml); > + } > + xmlKeepBlanksDefault(keepBlanksDefault); > + > + return ret; > +} > + > + > +/** > + * virDomainCheckpointDefAssignBitmapNames: > + * @def: checkpoint def object > + * > + * Generate default bitmap names for checkpoint targets. Returns 0 on > + * success, -1 on error. > + */ > +static int > +virDomainCheckpointDefAssignBitmapNames(virDomainCheckpointDefPtr def) > +{ > + size_t i; > + > + for (i = 0; i < def->ndisks; i++) { > + virDomainCheckpointDiskDefPtr disk = &def->disks[i]; > + > + if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP || > + disk->bitmap) > + continue; > + > + if (VIR_STRDUP(disk->bitmap, def->parent.name) < 0) > + return -1; > + } > + > + return 0; > +} > + > + > +static int > +virDomainCheckpointCompareDiskIndex(const void *a, const void *b) > +{ > + const virDomainCheckpointDiskDef *diska = a; > + const virDomainCheckpointDiskDef *diskb = b; > + > + /* Integer overflow shouldn't be a problem here. */ > + return diska->idx - diskb->idx; > +} > + > +/* Align def->disks to def->domain. Sort the list of def->disks, > + * filling in any missing disks with appropriate default. Convert > + * paths to disk targets for uniformity. Issue an error and return -1 > + * if any def->disks[n]->name appears more than once or does not map > + * to dom->disks. */ > +int > +virDomainCheckpointAlignDisks(virDomainCheckpointDefPtr def) > +{ > + int ret = -1; > + virBitmapPtr map = NULL; > + size_t i; > + int ndisks; > + int checkpoint_default = VIR_DOMAIN_CHECKPOINT_TYPE_NONE; > + > + if (!def->parent.dom) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("missing domain in checkpoint")); > + goto cleanup; > + } > + > + if (def->ndisks > def->parent.dom->ndisks) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("too many disk checkpoint requests for domain")); > + goto cleanup; > + } > + > + /* Unlikely to have a guest without disks but technically possible. */ > + if (!def->parent.dom->ndisks) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("domain must have at least one disk to perform " > + "checkpoints")); > + goto cleanup; > + } > + > + /* If <disks> omitted, do bitmap on all writeable disks; > + * otherwise, do nothing for omitted disks */ > + if (!def->ndisks) > + checkpoint_default = VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP; > + > + if (!(map = virBitmapNew(def->parent.dom->ndisks))) > + goto cleanup; > + > + /* Double check requested disks. */ > + for (i = 0; i < def->ndisks; i++) { > + virDomainCheckpointDiskDefPtr disk = &def->disks[i]; > + int idx = virDomainDiskIndexByName(def->parent.dom, disk->name, false); > + > + if (idx < 0) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("no disk named '%s'"), disk->name); > + goto cleanup; > + } > + > + if (virBitmapIsBitSet(map, idx)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("disk '%s' specified twice"), > + disk->name); > + goto cleanup; > + } > + if ((virStorageSourceIsEmpty(def->parent.dom->disks[idx]->src) || > + def->parent.dom->disks[idx]->src->readonly) && > + disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_NONE) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("disk '%s' is empty or readonly"), > + disk->name); > + goto cleanup; > + } > + ignore_value(virBitmapSetBit(map, idx)); > + disk->idx = idx; > + > + if (STRNEQ(disk->name, def->parent.dom->disks[idx]->dst)) { > + VIR_FREE(disk->name); > + if (VIR_STRDUP(disk->name, def->parent.dom->disks[idx]->dst) < 0) > + goto cleanup; > + } > + } > + > + /* Provide defaults for all remaining disks. */ > + ndisks = def->ndisks; > + if (VIR_EXPAND_N(def->disks, def->ndisks, > + def->parent.dom->ndisks - def->ndisks) < 0) > + goto cleanup; > + > + for (i = 0; i < def->parent.dom->ndisks; i++) { > + virDomainCheckpointDiskDefPtr disk; > + > + if (virBitmapIsBitSet(map, i)) > + continue; > + disk = &def->disks[ndisks++]; > + if (VIR_STRDUP(disk->name, def->parent.dom->disks[i]->dst) < 0) > + goto cleanup; > + disk->idx = i; > + > + /* Don't checkpoint empty or readonly drives */ > + if (virStorageSourceIsEmpty(def->parent.dom->disks[i]->src) || > + def->parent.dom->disks[i]->src->readonly) > + disk->type = VIR_DOMAIN_CHECKPOINT_TYPE_NONE; > + else > + disk->type = checkpoint_default; > + } > + > + qsort(&def->disks[0], def->ndisks, sizeof(def->disks[0]), > + virDomainCheckpointCompareDiskIndex); > + > + /* Generate default bitmap names for checkpoint */ > + if (virDomainCheckpointDefAssignBitmapNames(def) < 0) > + goto cleanup; > + > + ret = 0; > + > + cleanup: > + virBitmapFree(map); > + return ret; > +} > + > + > +/* Converts public VIR_DOMAIN_CHECKPOINT_XML_* into > + * VIR_DOMAIN_CHECKPOINT_FORMAT_* flags, and silently ignores any other > + * flags. */ > +unsigned int virDomainCheckpointFormatConvertXMLFlags(unsigned int flags) > +{ > + unsigned int formatFlags = 0; > + > + if (flags & VIR_DOMAIN_CHECKPOINT_XML_SECURE) > + formatFlags |= VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE; > + if (flags & VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN) > + formatFlags |= VIR_DOMAIN_CHECKPOINT_FORMAT_NO_DOMAIN; > + if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE) > + formatFlags |= VIR_DOMAIN_CHECKPOINT_FORMAT_SIZE; > + > + return formatFlags; > +} > + > + > +static int > +virDomainCheckpointDiskDefFormat(virBufferPtr buf, > + virDomainCheckpointDiskDefPtr disk, > + unsigned int flags) > +{ > + if (!disk->name) > + return 0; > + > + virBufferEscapeString(buf, "<disk name='%s'", disk->name); > + if (disk->type) > + virBufferAsprintf(buf, " checkpoint='%s'", > + virDomainCheckpointTypeToString(disk->type)); > + if (disk->bitmap) { > + virBufferEscapeString(buf, " bitmap='%s'", disk->bitmap); > + if (flags & VIR_DOMAIN_CHECKPOINT_FORMAT_SIZE) > + virBufferAsprintf(buf, " size='%llu'", disk->size); > + } > + virBufferAddLit(buf, "/>\n"); > + return 0; > +} > + > + > +static int > +virDomainCheckpointDefFormatInternal(virBufferPtr buf, > + virDomainCheckpointDefPtr def, > + virCapsPtr caps, > + virDomainXMLOptionPtr xmlopt, > + unsigned int flags) > +{ > + size_t i; > + unsigned int domainflags = VIR_DOMAIN_DEF_FORMAT_INACTIVE; > + > + if (flags & VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE) > + domainflags |= VIR_DOMAIN_DEF_FORMAT_SECURE; > + > + virBufferAddLit(buf, "<domaincheckpoint>\n"); > + virBufferAdjustIndent(buf, 2); > + > + virBufferEscapeString(buf, "<name>%s</name>\n", def->parent.name); > + virBufferEscapeString(buf, "<description>%s</description>\n", > + def->parent.description); > + > + if (def->parent.parent_name) { > + virBufferAddLit(buf, "<parent>\n"); > + virBufferAdjustIndent(buf, 2); > + virBufferEscapeString(buf, "<name>%s</name>\n", > + def->parent.parent_name); > + virBufferAdjustIndent(buf, -2); > + virBufferAddLit(buf, "</parent>\n"); > + } > + > + if (def->parent.creationTime) > + virBufferAsprintf(buf, "<creationTime>%lld</creationTime>\n", > + def->parent.creationTime); > + > + if (def->ndisks) { > + virBufferAddLit(buf, "<disks>\n"); > + virBufferAdjustIndent(buf, 2); > + for (i = 0; i < def->ndisks; i++) { > + if (virDomainCheckpointDiskDefFormat(buf, &def->disks[i], > + flags) < 0) > + goto error; > + } > + virBufferAdjustIndent(buf, -2); > + virBufferAddLit(buf, "</disks>\n"); > + } > + > + if (!(flags & VIR_DOMAIN_CHECKPOINT_FORMAT_NO_DOMAIN) && > + virDomainDefFormatInternal(def->parent.dom, caps, domainflags, buf, > + xmlopt) < 0) > + goto error; > + > + virBufferAdjustIndent(buf, -2); > + virBufferAddLit(buf, "</domaincheckpoint>\n"); > + > + if (virBufferCheckError(buf) < 0) > + goto error; > + > + return 0; > + > + error: > + virBufferFreeAndReset(buf); > + return -1; > +} > + > +char * > +virDomainCheckpointDefFormat(virDomainCheckpointDefPtr def, > + virCapsPtr caps, > + virDomainXMLOptionPtr xmlopt, > + unsigned int flags) > +{ > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + > + virCheckFlags(VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE | > + VIR_DOMAIN_CHECKPOINT_FORMAT_NO_DOMAIN | > + VIR_DOMAIN_CHECKPOINT_FORMAT_SIZE, NULL); > + if (virDomainCheckpointDefFormatInternal(&buf, def, caps, xmlopt, > + flags) < 0) > + return NULL; > + > + return virBufferContentAndReset(&buf); > +} > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index ff5a77b0e2..693c75b45d 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -69,6 +69,14 @@ virCapabilitiesSetHostCPU; > virCapabilitiesSetNetPrefix; > > > +# conf/checkpoint_conf.h > +virDomainCheckpointDefFormat; > +virDomainCheckpointDefNew; > +virDomainCheckpointDefParseString; > +virDomainCheckpointTypeFromString; > +virDomainCheckpointTypeToString; > + > + > # conf/cpu_conf.h > virCPUCacheModeTypeFromString; > virCPUCacheModeTypeToString; > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 9e2e57459e..544328bb18 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -276,7 +276,7 @@ endif WITH_LIBXL > > if WITH_QEMU > test_programs += qemuxml2argvtest qemuxml2xmltest \ > - qemudomainsnapshotxml2xmltest \ > + qemudomaincheckpointxml2xmltest qemudomainsnapshotxml2xmltest \ > qemumonitorjsontest qemuhotplugtest \ > qemuagenttest qemucapabilitiestest qemucaps2xmltest \ > qemumemlocktest \ > @@ -647,6 +647,11 @@ qemublocktest_LDADD = \ > $(qemu_LDADDS) \ > $(NULL) > > +qemudomaincheckpointxml2xmltest_SOURCES = \ > + qemudomaincheckpointxml2xmltest.c testutilsqemu.c testutilsqemu.h \ > + testutils.c testutils.h > +qemudomaincheckpointxml2xmltest_LDADD = $(qemu_LDADDS) > + > qemudomainsnapshotxml2xmltest_SOURCES = \ > qemudomainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ > testutils.c testutils.h > @@ -682,7 +687,7 @@ qemufirmwaretest_LDADD = $(qemu_LDADDS) > > else ! WITH_QEMU > EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c \ > - qemudomainsnapshotxml2xmltest.c \ > + qemudomaincheckpointxml2xmltest.c qemudomainsnapshotxml2xmltest.c \ > testutilsqemu.c testutilsqemu.h \ > testutilsqemuschema.c testutilsqemuschema.h \ > qemumonitorjsontest.c qemuhotplugtest.c \ > diff --git a/tests/qemudomaincheckpointxml2xmlout/internal-active-invalid.xml b/tests/qemudomaincheckpointxml2xmlout/internal-active-invalid.xml > new file mode 100644 > index 0000000000..a518c58915 > --- /dev/null > +++ b/tests/qemudomaincheckpointxml2xmlout/internal-active-invalid.xml > @@ -0,0 +1,53 @@ > +<domaincheckpoint> > + <name>1525889631</name> > + <description>Completion of updates after OS install</description> > + <parent> > + <name>1525111885</name> > + </parent> > + <creationTime>1525889631</creationTime> > + <disks> > + <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/> > + <disk name='vdb' checkpoint='no'/> > + </disks> > + <domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219136</memory> > + <currentMemory unit='KiB'>219136</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i686</emulator> > + <disk type='file' device='disk'> > + <driver name='qemu' type='qcow2'/> > + <source file='/tmp/data.img'/> > + <target dev='vda' bus='virtio'/> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> > + </disk> > + <disk type='file' device='disk'> > + <driver name='qemu' type='qcow2'/> > + <source file='/tmp/logs.img'/> > + <target dev='vdb' bus='virtio'/> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> > + </disk> > + <controller type='usb' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> > + </controller> > + <controller type='ide' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> > + </controller> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <memballoon model='none'/> > + </devices> > + </domain> > + <active>1</active> > +</domaincheckpoint> > diff --git a/tests/qemudomaincheckpointxml2xmlout/internal-inactive-invalid.xml b/tests/qemudomaincheckpointxml2xmlout/internal-inactive-invalid.xml > new file mode 100644 > index 0000000000..df14c97836 > --- /dev/null > +++ b/tests/qemudomaincheckpointxml2xmlout/internal-inactive-invalid.xml > @@ -0,0 +1,53 @@ > +<domaincheckpoint> > + <name>1525889631</name> > + <description>Completion of updates after OS install</description> > + <parent> > + <name>1525111885</name> > + </parent> > + <creationTime>1525889631</creationTime> > + <disks> > + <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/> > + <disk name='vdb' checkpoint='no'/> > + </disks> > + <domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219136</memory> > + <currentMemory unit='KiB'>219136</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-i686</emulator> > + <disk type='file' device='disk'> > + <driver name='qemu' type='qcow2'/> > + <source file='/tmp/data.img'/> > + <target dev='vda' bus='virtio'/> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> > + </disk> > + <disk type='file' device='disk'> > + <driver name='qemu' type='qcow2'/> > + <source file='/tmp/logs.img'/> > + <target dev='vdb' bus='virtio'/> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> > + </disk> > + <controller type='usb' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> > + </controller> > + <controller type='ide' index='0'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> > + </controller> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <memballoon model='none'/> > + </devices> > + </domain> > + <active>0</active> > +</domaincheckpoint> > diff --git a/tests/qemudomaincheckpointxml2xmltest.c b/tests/qemudomaincheckpointxml2xmltest.c > new file mode 100644 > index 0000000000..8a7c0922c7 > --- /dev/null > +++ b/tests/qemudomaincheckpointxml2xmltest.c > @@ -0,0 +1,213 @@ > +#include <config.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > +#include <string.h> > + > +#include <sys/types.h> > +#include <fcntl.h> > + > +#include "testutils.h" > + > +#ifdef WITH_QEMU > + > +# include "internal.h" > +# include "qemu/qemu_conf.h" > +# include "qemu/qemu_domain.h" > +# include "checkpoint_conf.h" > +# include "testutilsqemu.h" > +# include "virstring.h" > + > +# define VIR_FROM_THIS VIR_FROM_NONE > + > +static virQEMUDriver driver; > + > +enum { > + TEST_REDEFINE = 1 << 0, /* Test use of REDEFINE parse flag */ > + TEST_PARENT = 1 << 1, /* hard-code parent after parse */ > + TEST_VDA_BITMAP = 1 << 2, /* hard-code disk vda after parse */ > + TEST_SIZE = 1 << 3, /* Test use of SIZE format flag */ > + TEST_INVALID = 1 << 4, /* Test that input fails parse */ > +}; > + > +static int > +testCompareXMLToXMLFiles(const char *inxml, > + const char *outxml, > + unsigned int flags) > +{ > + unsigned int parseflags = 0; > + unsigned int formatflags = VIR_DOMAIN_CHECKPOINT_FORMAT_SECURE; > + VIR_AUTOFREE(char *) inXmlData = NULL; > + VIR_AUTOFREE(char *) outXmlData = NULL; > + VIR_AUTOFREE(char *) actual = NULL; > + VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL; > + > + if (flags & TEST_REDEFINE) > + parseflags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE; > + > + if (virTestLoadFile(inxml, &inXmlData) < 0) > + return -1; > + > + if (!(flags & TEST_INVALID) && > + virTestLoadFile(outxml, &outXmlData) < 0) > + return -1; > + > + if (!(def = virDomainCheckpointDefParseString(inXmlData, driver.caps, > + driver.xmlopt, > + parseflags))) { > + if (flags & TEST_INVALID) > + return 0; > + return -1; > + } > + if (flags & TEST_PARENT) { > + if (def->parent.parent_name) > + return -1; > + if (VIR_STRDUP(def->parent.parent_name, "1525111885") < 0) > + return -1; > + } > + if (flags & TEST_VDA_BITMAP) { > + virDomainCheckpointDiskDefPtr disk; > + > + if (VIR_EXPAND_N(def->disks, def->ndisks, 1) < 0) > + return -1; > + disk = &def->disks[0]; > + if (disk->bitmap) > + return -1; > + if (!disk->name) { > + disk->type = VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP; > + if (VIR_STRDUP(disk->name, "vda") < 0) > + return -1; > + } else if (STRNEQ(disk->name, "vda")) { > + return -1; > + } > + if (VIR_STRDUP(disk->bitmap, def->parent.name) < 0) > + return -1; > + } > + if (flags & TEST_SIZE) { > + def->disks[0].size = 1048576; > + formatflags |= VIR_DOMAIN_CHECKPOINT_FORMAT_SIZE; > + } > + > + /* Parsing XML does not populate the domain definition; work > + * around that by not requesting domain on output */ > + if (!def->parent.dom) > + formatflags |= VIR_DOMAIN_CHECKPOINT_FORMAT_NO_DOMAIN; > + > + if (!(actual = virDomainCheckpointDefFormat(def, driver.caps, > + driver.xmlopt, > + formatflags))) > + return -1; > + > + if (STRNEQ(outXmlData, actual)) { > + virTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml); > + return -1; > + } > + > + return 0; > +} > + > +struct testInfo { > + const char *inxml; > + const char *outxml; > + long long creationTime; > + unsigned int flags; > +}; > +static long long mocktime; > + > +static int > +testCheckpointPostParse(virDomainMomentDefPtr def) > +{ > + if (!mocktime) > + return 0; > + if (def->creationTime) > + return -1; > + def->creationTime = mocktime; > + if (!def->name && > + virAsprintf(&def->name, "%lld", def->creationTime) < 0) > + return -1; > + return 0; > +} > + > +static int > +testCompareXMLToXMLHelper(const void *data) > +{ > + const struct testInfo *info = data; > + > + mocktime = info->creationTime; > + return testCompareXMLToXMLFiles(info->inxml, info->outxml, info->flags); > +} > + > + > +static int > +mymain(void) > +{ > + int ret = 0; > + > + if (qemuTestDriverInit(&driver) < 0) > + return EXIT_FAILURE; > + > + virDomainXMLOptionSetMomentPostParse(driver.xmlopt, > + testCheckpointPostParse); > + > +# define DO_TEST(prefix, name, inpath, outpath, time, flags) \ > + do { \ > + const struct testInfo info = {abs_srcdir "/" inpath "/" name ".xml", \ > + abs_srcdir "/" outpath "/" name ".xml", \ > + time, flags}; \ > + if (virTestRun("CHECKPOINT XML-2-XML " prefix " " name, \ > + testCompareXMLToXMLHelper, &info) < 0) \ > + ret = -1; \ > + } while (0) > + > +# define DO_TEST_INOUT(name, time, flags) \ > + DO_TEST("in->out", name, \ > + "qemudomaincheckpointxml2xmlin", \ > + "qemudomaincheckpointxml2xmlout", \ > + time, flags) > +# define DO_TEST_OUT(name, flags) \ > + DO_TEST("out->out", name, \ > + "qemudomaincheckpointxml2xmlout", \ > + "qemudomaincheckpointxml2xmlout", \ > + 0, flags | TEST_REDEFINE) > +# define DO_TEST_INVALID(name) \ > + DO_TEST("in->out", name, \ > + "qemudomaincheckpointxml2xmlin", \ > + "qemudomaincheckpointxml2xmlout", \ > + 0, TEST_INVALID) > + > + /* Unset or set all envvars here that are copied in qemudBuildCommandLine > + * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected > + * values for these envvars */ > + setenv("PATH", "/bin", 1); > + > + /* Test a normal user redefine */ > + DO_TEST_OUT("redefine", 0); > + > + /* Tests of valid user input, and resulting output */ > + DO_TEST_INOUT("empty", 1525889631, TEST_VDA_BITMAP); > + DO_TEST_INOUT("disk-default", 1525889631, TEST_PARENT | TEST_VDA_BITMAP); > + DO_TEST_INOUT("sample", 1525889631, TEST_PARENT | TEST_VDA_BITMAP); > + DO_TEST_INOUT("size", 1553648510, > + TEST_PARENT | TEST_VDA_BITMAP | TEST_SIZE); > + > + /* Tests of invalid user input */ > + DO_TEST_INVALID("disk-invalid"); > + DO_TEST_INVALID("name-invalid"); > + > + qemuTestDriverFree(&driver); > + > + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; > +} > + > +VIR_TEST_MAIN(mymain) > + > +#else > + > +int > +main(void) > +{ > + return EXIT_AM_SKIP; > +} > + > +#endif /* WITH_QEMU */ > -- > 2.20.1 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list