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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
This and the rest would be 5.2.0 related...
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
