[libvirt] [PATCH v2 02/11] snapshot: Give virDomainSnapshotDefFormat its own flags

[John Ferlan]

On 2/23/19 4:24 PM, Eric Blake wrote:
> virDomainSnapshotDefFormat currently takes two sets of knobs:
> an 'unsigned int flags' argument that can currently just be
> VIR_DOMAIN_DEF_FORMAT_SECURE, and an 'int internal' argument used as
> a bool to determine whether to output an additional element.  It
> then reuses the 'flags' knob to call into virDomainDefFormatInternal(),
> which takes a different set of flags. In fact, prior to commit 0ecd6851
> (1.2.12), the 'flags' argument actually took the public
> VIR_DOMAIN_XML_SECURE, which was even more confusing.  Let's borrow
> from the style of that earlier commit, by introducing a function
> for translating from the public flags (VIR_DOMAIN_SNAPSHOT_XML_SECURE
> was just recently introduced) into a new enum specific to snapshot
> formatting, and adjust all callers to use snapshot-specific enum
> values when formatting, and where the formatter now uses a new
> variable 'domainflags' to make it obvious when we are translating
> from snapshot flags back to domain flags.  We don't even have to
> use the conversion function for drivers that don't accept the
> public VIR_DOMAIN_SNAPSHOT_XML_SECURE flag.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/conf/snapshot_conf.h          | 10 ++++++++--
>  src/conf/snapshot_conf.c          | 28 ++++++++++++++++++++++------
>  src/esx/esx_driver.c              |  1 -
>  src/libvirt_private.syms          |  1 +
>  src/qemu/qemu_domain.c            |  3 +--
>  src/qemu/qemu_driver.c            |  3 +--
>  src/test/test_driver.c            |  3 +--
>  src/vbox/vbox_common.c            |  5 ++---
>  src/vz/vz_driver.c                |  3 +--
>  tests/domainsnapshotxml2xmltest.c | 16 +++++++++-------
>  10 files changed, 46 insertions(+), 27 deletions(-)
> 

[...]

> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index 3372c4df86..652d963f84 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -652,6 +652,19 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
>      return ret;
>  }
> 
> +/* Converts public VIR_DOMAIN_SNAPSHOT_XML_* into
> + * VIR_DOMAIN_SNAPSHOT_FORMAT_* flags, and silently ignores any other
> + * flags.  */

NIT: extra spaces between . and *

> +unsigned int virDomainSnapshotFormatConvertXMLFlags(unsigned int flags)
> +{
> +    unsigned int formatFlags = 0;
> +
> +    if (flags & VIR_DOMAIN_SNAPSHOT_XML_SECURE)
> +        formatFlags |= VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE;
> +
> +    return formatFlags;
> +}
> +

[...]

> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index d95fe7c7ae..1efd357b49 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c

[...]

> @@ -6321,8 +6321,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
>      virUUIDFormat(dom->uuid, uuidstr);
>      memcpy(def->dom->uuid, dom->uuid, VIR_UUID_BUFLEN);
>      ret = virDomainSnapshotDefFormat(uuidstr, def, data->caps, data->xmlopt,
> -                                      virDomainDefFormatConvertXMLFlags(flags),
> -                                      0);
> +                                     0);

NIT:

The "0);" could fit on the previous line.

> 
>   cleanup:
>      virDomainSnapshotDefFree(def);

[...]

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

This and the rest would be 5.2.0 related...

John