Right now, copying the state of a transient domain with snapshots from one host to another requires multiple API calls on both machines - on the host: get the domain XML, get a list of the snapshots, and then for each snapshot get the snapshot's XML; then on the destination: create the domain, then multiple domain snapshot create calls (must be in topological order) with the REDEFINE flag. This patch aims to make the process use fewer APIs by making it possible to grab the XML for all snapshots at once on the source, then redefine from that same list at the destination. In fact, although it is easy to match recent changes to allow output in topological order, it is also easy to do bulk redefine in arbitrary order. Consideration was given to adding a flag to existing APIs; but to do that, there would be an asymmetry of dumping <snapshots> as a sub-element of <domain> during virDomainGetXMLDesc(), vs. redefining <snapshots> as a top-level element (and different than the normal top-level element) of virDomainSnapshotCreateXML(). Thus, it is cleaner to have two new APIs: virDomainGetSnapshotsXMLDesc virDomainImportSnapshotsXML Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- include/libvirt/libvirt-domain-snapshot.h | 16 ++- src/driver-hypervisor.h | 13 ++- src/libvirt-domain-snapshot.c | 122 +++++++++++++++++++++- src/libvirt_public.syms | 2 + 4 files changed, 148 insertions(+), 5 deletions(-) diff --git a/include/libvirt/libvirt-domain-snapshot.h b/include/libvirt/libvirt-domain-snapshot.h index 602e5def59..7c2e001b87 100644 --- a/include/libvirt/libvirt-domain-snapshot.h +++ b/include/libvirt/libvirt-domain-snapshot.h @@ -3,7 +3,7 @@ * Summary: APIs for management of domain snapshots * Description: Provides APIs for the management of domain snapshots * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -78,6 +78,11 @@ virDomainSnapshotPtr virDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, unsigned int flags); +/* Bulk import a list of snapshots */ +int virDomainImportSnapshotsXML(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags); + typedef enum { VIR_DOMAIN_SNAPSHOT_XML_SECURE = VIR_DOMAIN_XML_SECURE, /* dump security sensitive information too */ } virDomainSnapshotXMLFlags; @@ -86,6 +91,15 @@ typedef enum { char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags); +typedef enum { + VIR_DOMAIN_GET_SNAPSHOTS_XML_SECURE = (1 << 0), /* dump security sensitive information too */ + VIR_DOMAIN_GET_SNAPSHOTS_XML_TOPOLOGICAL = (1 << 1), /* ensure parents occur before children */ +} virDomainGetSnapshotsXMLFlags; + +/* Dump the XML of all snapshots */ +char *virDomainGetSnapshotsXMLDesc(virDomainPtr domain, + unsigned int flags); + /** * virDomainSnapshotListFlags: * diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 5315e33dde..c30e16b690 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1,7 +1,7 @@ /* * driver-hypervisor.h: entry points for hypervisor drivers * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -797,10 +797,19 @@ typedef virDomainSnapshotPtr const char *xmlDesc, unsigned int flags); +typedef int +(*virDrvDomainImportSnapshotsXML)(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags); + typedef char * (*virDrvDomainSnapshotGetXMLDesc)(virDomainSnapshotPtr snapshot, unsigned int flags); +typedef char * +(*virDrvDomainGetSnapshotsXMLDesc)(virDomainPtr domain, + unsigned int flags); + typedef int (*virDrvDomainSnapshotNum)(virDomainPtr domain, unsigned int flags); @@ -1494,7 +1503,9 @@ struct _virHypervisorDriver { virDrvDomainManagedSaveGetXMLDesc domainManagedSaveGetXMLDesc; virDrvDomainManagedSaveDefineXML domainManagedSaveDefineXML; virDrvDomainSnapshotCreateXML domainSnapshotCreateXML; + virDrvDomainImportSnapshotsXML domainImportSnapshotsXML; virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc; + virDrvDomainGetSnapshotsXMLDesc domainGetSnapshotsXMLDesc; virDrvDomainSnapshotNum domainSnapshotNum; virDrvDomainSnapshotListNames domainSnapshotListNames; virDrvDomainListAllSnapshots domainListAllSnapshots; diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c index d133c84933..1ad8dfbfb8 100644 --- a/src/libvirt-domain-snapshot.c +++ b/src/libvirt-domain-snapshot.c @@ -133,7 +133,8 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * not exist, the hypervisor may validate that reverting to the * snapshot appears to be possible (for example, disk images have * snapshot contents by the requested name). Not all hypervisors - * support these flags. + * support these flags; and some hypervisors support + * virDomainImportSnapshotsXML() for redefining all metadata in one call. * * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, then the * domain's disk images are modified according to @xmlDesc, but then @@ -242,6 +243,65 @@ virDomainSnapshotCreateXML(virDomainPtr domain, } +/** + * virDomainImportSnapshotsXML: + * @domain: a domain object + * @xmlDesc: string containing an XML description of a list of domain snapshots + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Imports the metadata for a list of domain snapshots using + * @xmlDesc with a top-level element of <snapshots>. + * + * This call requires that the domain currently has no snapshot + * metadata, and the snapshots can be listed in any order, whereas + * using virDomainSnapshotCreateXML() with its + * VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE flag requires multiple calls + * and topological sorting. Bulk redefinition is mainly useful for + * reinstating metadata in situations such as transient domains or + * migration. + * + * Generally, the list of snapshots is obtained from + * virDomainGetSnapshotsXMLDesc() prior to a scenario that requires + * removing snapshot metadata (such as virDomainUndefine() on a + * transient domain); although it could also be constructed by + * concatenating virDomainSnapshotGetXMLDesc() for each snapshot and + * wrapping with a <snapshots> element and optional attribute + * current='name' pointing to the current snapshot. + * + * Returns a count of snapshots imported on success, or -1 on failure. + */ +int +virDomainImportSnapshotsXML(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "xmlDesc=%s, flags=0x%x", xmlDesc, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckNonNullArgGoto(xmlDesc, error); + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainImportSnapshotsXML) { + int ret = conn->driver->domainImportSnapshotsXML(domain, xmlDesc, + flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} + + /** * virDomainSnapshotGetXMLDesc: * @snapshot: a domain snapshot object @@ -254,8 +314,8 @@ virDomainSnapshotCreateXML(virDomainPtr domain, * VIR_DOMAIN_SNAPSHOT_XML_SECURE; this flag is rejected on read-only * connections. * - * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. - * the caller must free() the returned value. + * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case + * of error. The caller must free() the returned value. */ char * virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, @@ -291,6 +351,62 @@ virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, } +/** + * virDomainGetSnapshotsXMLDesc: + * @domain: a domain object + * @flags: bitwise-OR of virDomainGetSnapshotsXMLFlags + * + * Provide an XML description of all domain snapshots, with a top-level + * element of <snapshots>. + * + * No security-sensitive data will be included unless @flags contains + * VIR_DOMAIN_GET_SNAPSHOTS_XML_SECURE; this flag is rejected on read-only + * connections. + * + * If @flags contains VIR_DOMAIN_GET_SNAPSHOTS_XML_TOPOLOGICAL, then + * it is guaranteed that no snapshot appears in the resulting XML + * prior to its parent; otherwise, the order of snapshots in the + * resulting list is unspecified. Use of this flag is not required for + * virDomainImportSnapshotsXML() to transfer snapshot metadata to + * another instance of libvirt. + * + * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case + * of error. The caller must free() the returned value. + */ +char * +virDomainGetSnapshotsXMLDesc(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + VIR_DOMAIN_DEBUG(domain, "flags=0x%x", flags); + + virResetLastError(); + + virCheckDomainReturn(domain, NULL); + conn = domain->conn; + + if ((conn->flags & VIR_CONNECT_RO) && + (flags & VIR_DOMAIN_GET_SNAPSHOTS_XML_SECURE)) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("virDomainGetSnapshotsXMLDesc with secure flag")); + goto error; + } + + if (conn->driver->domainGetSnapshotsXMLDesc) { + char *ret; + ret = conn->driver->domainGetSnapshotsXMLDesc(domain, flags); + if (!ret) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return NULL; +} + + /** * virDomainSnapshotNum: * @domain: a domain object diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index dbce3336d5..d40da8b893 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -817,6 +817,8 @@ LIBVIRT_4.10.0 { LIBVIRT_5.2.0 { global: virConnectGetStoragePoolCapabilities; + virDomainGetSnapshotsXMLDesc; + virDomainImportSnapshotsXML; } LIBVIRT_4.10.0; # .... define new API here using predicted next version number .... -- 2.20.1

On 3/12/19 7:57 AM, Ján Tomko wrote: In particular, they do NOT have the virDomainSnapshot prefix, which means I don't have to special-case them in the python generator :) I have an emacs hook that prompts me to auto-update on files I touch. I know we haven't been insistent on keeping things up-to-date, but I also don't think it hurts. Yeah, especially for public-facing files, having a reliable year in there might matter (although these days, I hope that any court litigating a copyright claim against open source would be smart enough to rely more on version control history than any thing in the file under question). Partial success is not possible; it's an all-or-none return. But I can try to tweak that wording to make it clearer. Noticed while copy-and-pasting. If needed, I can split it out as a trivial patch. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

On Mon, Mar 11, 2019 at 09:38:33PM -0500, Eric Blake wrote: Rather than introducing a bool parameter (which shows up as non-descriptive 'false' in the caller), I'd put it into the name of the function: virDomainSnapshotForEachTopological which would be a thin wrapper for virDomainSnapshotForEachDescendant. Alternatively, the one caller can call virDomainSnapshotForEachDescendant with &snapshots->metaroot directly. Jano

Change the return value of virDomainSnapshotObjLisParse() to return the number of snapshots imported, and allow a return of 0 (the original proposal of adding a flag to virDomainSnapshotCreateXML required returning an arbitrary non-NULL snapshot, but with a new API that returns a count, we are no longer constrained to a non-empty list). Change virDomainSnapshotObjListFormat()'s flags argument to be the new public virDomainGetSnapshotsXMLFlags, since it is easy to support both flag values. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/snapshot_conf.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 8235d7c526..3f24a80f76 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -507,8 +507,10 @@ 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. +/* Parse a <snapshots> XML entry into snapshots, which must start + * empty. Any <domain> sub-elements of a <domainsnapshot> must match + * domain_uuid. @flags is virDomainSnapshotParseFlags. Return the + * number of snapshots parsed, or -1 on error. */ int virDomainSnapshotObjListParse(const char *xmlStr, @@ -562,11 +564,6 @@ virDomainSnapshotObjListParse(const char *xmlStr, 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; @@ -601,7 +598,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, (*current_snap)->def->current = true; } - ret = 0; + ret = n; cleanup: if (ret < 0) { /* There were no snapshots before this call; so on error, just @@ -1025,8 +1022,9 @@ virDomainSnapshotFormatOne(void *payload, } -/* Format the XML for all snapshots in the list into buf. On error, - * clear the buffer and return -1. */ +/* Format the XML for all snapshots in the list into buf. @flags is + * virDomainGetSnapshotsXMLFlags. On error, clear the buffer and + * return -1. */ int virDomainSnapshotObjListFormat(virBufferPtr buf, const char *uuidstr, @@ -1041,17 +1039,23 @@ virDomainSnapshotObjListFormat(virBufferPtr buf, .uuidstr = uuidstr, .caps = caps, .xmlopt = xmlopt, - .flags = flags, + .flags = 0, }; + bool topological = flags & VIR_DOMAIN_GET_SNAPSHOTS_XML_TOPOLOGICAL; + virCheckFlags(VIR_DOMAIN_GET_SNAPSHOTS_XML_SECURE | + VIR_DOMAIN_GET_SNAPSHOTS_XML_TOPOLOGICAL, -1); + + if (flags & VIR_DOMAIN_GET_SNAPSHOTS_XML_SECURE) + data.flags |= VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE; virBufferAddLit(buf, "<snapshots"); if (current_snapshot) virBufferEscapeString(buf, " current='%s'", current_snapshot->def->name); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); - if (virDomainSnapshotForEach(snapshots, false, virDomainSnapshotFormatOne, - &data) < 0) { + if (virDomainSnapshotForEach(snapshots, topological, + virDomainSnapshotFormatOne, &data) < 0) { virBufferFreeAndReset(buf); return -1; } -- 2.20.1