[libvirt] [PATCH v2 07/11] domain: Add VIR_DOMAIN_XML_SNAPSHOTS flag

[John Ferlan]

On 2/23/19 4:24 PM, Eric Blake wrote:
> 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 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 the same time as
> grabbing the domain XML.  Note that we had to do the modification
> to virDomainGetXMLDesc(), rather than virDomainSnapshotGetXMLDesc(),
> since the latter requires a single non-NULL snapshot object,
> whereas we want the list of all snapshots for the domain (even
> if the list has 0 elements).
> 
> Once wired up in drivers in later patches, the new information
> is provided as:
> 
> <domain ...>
>   ...
>   </devices>
>   <snapshots current='name'>
>     <domainsnapshot>
>       ...
>     </domainsnapshot>
>     <domainsnapshot>
>       ...
>     </domainsnapshot>
>   </snapshots>
> </domain>
> 
> For now, I did not modify the schema to permit this information
> during virDomainDefineXML; it is still necessary to use
> virDomainSnapshotCreateXML with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE
> multiple times to recreate the added state output here.
> 
> Unfortunately, libvirt versions between 1.2.12 and 5.0.0 will
> silently ignore the new flag, rather than diagnosing that they
> don't support it; but at least silent lack of snapshots from
> an older server is not a security hole.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  include/libvirt/libvirt-domain.h |  1 +
>  src/conf/domain_conf.c           | 13 ++++++++-----
>  src/libvirt-domain.c             |  5 +++++
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 

[...]

> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 072b92b717..2691698bd5 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -2570,6 +2570,11 @@ virDomainGetControlInfo(virDomainPtr domain,
>   * XML might not validate against the schema, so it is mainly for
>   * internal use.
>   *
> + * If @flags contains VIR_DOMAIN_XML_SNAPSHOTS, the XML will include

Should we even try to say that "and supported by the target libvirt
system with the appropriate version of the software installed" ;-)... I
know implied somewhat - but perhaps notable in this (and future) cases
because of the issue mentioned in the commit message that outward facing
docs consumers may never read.

> + * an additional <snapshots> child element describing all snapshots
> + * belonging to the domain, including an attribute current='name' if
> + * one of those snapshots is current.
> + *
>   * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
>   *         the caller must free() the returned value.
>   */
> 

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

John