[libvirt] [PATCH v3 11/18] snapshot: Add virDomainSnapshotObjListParse

[John Ferlan]

On 3/4/19 10:34 PM, Eric Blake wrote:
> Add a new function to make it possible to parse a list of snapshots
> at once.  This is a counterpart to an earlier patch making it
> possible to produce all snapshots in a single XML string, and
> intentionally parses the same top-level element <snapshots> with
> an optional attribute current='name'.
> 
> Note that since we know we started with no relations at all, and
> since checking parent relationships per-snapshot is not viable as
> we don't control which order the snapshots appear in, that we are
> fine with doing a final pass to update all parent/child
> relationships among the definitions.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/conf/snapshot_conf.h |   7 +++
>  src/conf/snapshot_conf.c | 111 +++++++++++++++++++++++++++++++++++++++
>  src/libvirt_private.syms |   1 +
>  3 files changed, 119 insertions(+)
> 
> diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
> index 69a7750b0b..f8af991907 100644
> --- a/src/conf/snapshot_conf.h
> +++ b/src/conf/snapshot_conf.h
> @@ -132,6 +132,13 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseNode(xmlDocPtr xml,
>                                                        virCapsPtr caps,
>                                                        virDomainXMLOptionPtr xmlopt,
>                                                        unsigned int flags);
> +int virDomainSnapshotObjListParse(const char *xmlStr,
> +                                  const unsigned char *domain_uuid,
> +                                  virDomainSnapshotObjListPtr snapshots,
> +                                  virDomainSnapshotObjPtr *current_snap,
> +                                  virCapsPtr caps,
> +                                  virDomainXMLOptionPtr xmlopt,
> +                                  unsigned int flags);
>  void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def);
>  char *virDomainSnapshotDefFormat(const char *uuidstr,
>                                   virDomainSnapshotDefPtr def,
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index a5b05eadf4..52742d82d6 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -507,6 +507,117 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def,
>  }
> 
> 
> +/* Parse a <snapshots> XML entry into snapshots, which must start empty.
> + * Any <domain> sub-elements of a <domainsnapshot> must match domain_uuid.
> + */
> +int
> +virDomainSnapshotObjListParse(const char *xmlStr,
> +                              const unsigned char *domain_uuid,
> +                              virDomainSnapshotObjListPtr snapshots,
> +                              virDomainSnapshotObjPtr *current_snap,
> +                              virCapsPtr caps,
> +                              virDomainXMLOptionPtr xmlopt,
> +                              unsigned int flags)
> +{
> +    int ret = -1;
> +    xmlDocPtr xml;
> +    xmlNodePtr root;
> +    xmlXPathContextPtr ctxt = NULL;
> +    int n;
> +    size_t i;
> +    int keepBlanksDefault = xmlKeepBlanksDefault(0);
> +    VIR_AUTOFREE(xmlNodePtr *) nodes = NULL;
> +    VIR_AUTOFREE(char *) current = NULL;
> +
> +    if (!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) ||
> +        (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("incorrect flags for bulk parse"));
> +        return -1;
> +    }
> +    if (snapshots->metaroot.nchildren || *current_snap) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("bulk define of snapshots only possible with "
> +                         "no existing snapshot"));
> +        return -1;
> +    }
> +
> +    if (!(xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"))))
> +        goto cleanup;

Could just be return -1

> +
> +    root = xmlDocGetRootElement(xml);
> +    if (!virXMLNodeNameEqual(root, "snapshots")) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("unexpected root element <%s>, "
> +                         "expecting <snapshots>"), root->name);
> +        goto cleanup;
> +    }
> +    ctxt = xmlXPathNewContext(xml);
> +    if (ctxt == NULL) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +    ctxt->node = root;
> +    current = virXMLPropString(root, "current");
> +
> +    if ((n = virXPathNodeSet("./domainsnapshot", ctxt, &nodes)) < 0)
> +        goto cleanup;
> +    if (!n) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("expected at least one <domainsnapshot> child"));
> +        goto cleanup;
> +    }
> +
> +    for (i = 0; i < n; i++) {
> +        virDomainSnapshotDefPtr def;
> +        virDomainSnapshotObjPtr snap;
> +
> +        def = virDomainSnapshotDefParseNode(xml, nodes[i], caps, xmlopt, flags);
> +        if (!def)
> +            goto cleanup;
> +        if (!(snap = virDomainSnapshotAssignDef(snapshots, def))) {
> +            virDomainSnapshotDefFree(def);
> +            goto cleanup;
> +        }
> +        if (virDomainSnapshotRedefineValidate(def, domain_uuid, NULL, NULL,
> +                                              flags) < 0)
> +            goto cleanup;
> +    }
> +
> +    if (virDomainSnapshotUpdateRelations(snapshots) < 0) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("<snapshots> contains inconsistent parent-child "
> +                         "relationships"));
> +        goto cleanup;
> +    }
> +
> +    if (current) {
> +        if (!(*current_snap = virDomainSnapshotFindByName(snapshots,
> +                                                          current))) {
> +            virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
> +                           _("no snapshot matching current='%s'"), current);
> +            goto cleanup;
> +        }
> +        (*current_snap)->def->current = true;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    if (ret < 0) {
> +        /* There were no snapshots before this call; so on error, just
> +         * blindly delete anything created before the failure. */
> +        virHashRemoveAll(snapshots->objs);
> +        snapshots->metaroot.nchildren = 0;
> +        snapshots->metaroot.first_child = NULL;
> +    }
> +    VIR_FREE(current);

Unnecessary / VIR_AUTOFREE

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

> +    xmlXPathFreeContext(ctxt);
> +    xmlFreeDoc(xml);
> +    xmlKeepBlanksDefault(keepBlanksDefault);
> +    return ret;
> +}
> +
> +
>  /**
>   * virDomainSnapshotDefAssignExternalNames:
>   * @def: snapshot def object
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 35e0c6d9dc..395e1f8764 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -899,6 +899,7 @@ virDomainSnapshotObjListFree;
>  virDomainSnapshotObjListGetNames;
>  virDomainSnapshotObjListNew;
>  virDomainSnapshotObjListNum;
> +virDomainSnapshotObjListParse;
>  virDomainSnapshotObjListRemove;
>  virDomainSnapshotRedefinePrep;
>  virDomainSnapshotStateTypeFromString;
>