Re: [libvirt] [PATCH v2 09/11] Give virDomainDef parser & formatter their own flags

The virDomainDefParse* and virDomainDefFormat* methods both accept the VIR_DOMAIN_XML_* flags defined in the public API, along with a set of other VIR_DOMAIN_XML_INTERNAL_* flags defined in domain_conf.c. This is seriously confusing & error prone for a number of reasons: - VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_XML_MIGRATABLE and VIR_DOMAIN_XML_UPDATE_CPU are only relevant for the formatting operation - Some of the VIR_DOMAIN_XML_INTERNAL_* flags only apply to parse or to format, but not both. This patch cleanly separates out the flags. There are two distint VIR_DOMAIN_DEF_PARSE_* and VIR_DOMAIN_DEF_FORMAT_* flags that are used by the corresponding methods. The VIR_DOMAIN_XML_* flags received via public API calls must be converted to the VIR_DOMAIN_DEF_FORMAT_* flags where needed. The various calls to virDomainDefParse which hardcoded the use of the VIR_DOMAIN_XML_INACTIVE flag change to use the VIR_DOMAIN_DEF_PARSE_INACTIVE flag.

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e3bede..4361834 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -86,39 +86,12 @@ struct _virDomainXMLOption { /* XML namespace callbacks */ virDomainXMLNamespace ns; - }; - - -/* Private flags used internally by virDomainSaveStatus and - * virDomainLoadStatus, in addition to the public virDomainXMLFlags. */ -typedef enum { - /* dump internal domain status information */ - VIR_DOMAIN_XML_INTERNAL_STATUS = 1 << 16, - /* dump/parse <actual> element */ - VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET = 1 << 17, - /* dump/parse original states of host PCI device */ - VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = 1 << 18, - VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = 1 << 19, - VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = 1 << 20, - VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST = 1 << 21, - /* parse only source half of <disk> */ - VIR_DOMAIN_XML_INTERNAL_DISK_SOURCE = 1 << 22, -} virDomainXMLInternalFlags; - -#define DUMPXML_FLAGS \ - (VIR_DOMAIN_XML_SECURE | \ - VIR_DOMAIN_XML_INACTIVE | \ - VIR_DOMAIN_XML_UPDATE_CPU | \ - VIR_DOMAIN_XML_MIGRATABLE) - -verify(((VIR_DOMAIN_XML_INTERNAL_STATUS | - VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET | - VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES | - VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM | - VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT | - VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST | - VIR_DOMAIN_XML_INTERNAL_DISK_SOURCE) - & DUMPXML_FLAGS) == 0); +}; + +#define VIR_DOMAIN_DEF_FORMAT_COMMON_FLAGS \ + (VIR_DOMAIN_DEF_FORMAT_SECURE | \ + VIR_DOMAIN_DEF_FORMAT_INACTIVE | \ + VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)

VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST, "custom-argv",

@@ -19303,8 +19274,7 @@ virDomainDefHasCapabilitiesFeatures(virDomainDefPtr def) return false; } -/* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*, - * whereas the public version cannot. Also, it appends to an existing +/* This internal version appends to an existing

* buffer (possibly with auto-indent), rather than flattening to string. * Return -1 on failure. */ int @@ -19320,11 +19290,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, bool blkio = false; bool cputune = false; - virCheckFlags(DUMPXML_FLAGS | - VIR_DOMAIN_XML_INTERNAL_STATUS | - VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET | - VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES | - VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST, + virCheckFlags(VIR_DOMAIN_DEF_FORMAT_COMMON_FLAGS | + VIR_DOMAIN_DEF_FORMAT_STATUS | + VIR_DOMAIN_DEF_FORMAT_ACTUAL_NET | + VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES | + VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST, -1);

if (!(type = virDomainVirtTypeToString(def->virtType))) {

@@ -19878,7 +19848,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, } if (virCPUDefFormatBufFull(buf, def->cpu, - !!(flags & VIR_DOMAIN_XML_UPDATE_CPU)) < 0) + !!(flags & VIR_DOMAIN_DEF_FORMAT_UPDATE_CPU)) < 0) goto error;

virBufferAsprintf(buf, "<clock offset='%s'",

@@ -20131,12 +20101,29 @@ virDomainDefFormatInternal(virDomainDefPtr def, return -1; } +int virDomainDefFormatConvertXMLFlags(unsigned int flags) +{ + int formatFlags = 0;

+ + if (flags & VIR_DOMAIN_XML_SECURE) + formatFlags |= VIR_DOMAIN_DEF_FORMAT_SECURE; + if (flags & VIR_DOMAIN_XML_INACTIVE) + formatFlags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE; + if (flags & VIR_DOMAIN_XML_UPDATE_CPU) + formatFlags |= VIR_DOMAIN_DEF_FORMAT_UPDATE_CPU; + if (flags & VIR_DOMAIN_XML_MIGRATABLE) + formatFlags |= VIR_DOMAIN_DEF_FORMAT_MIGRATABLE; + + return formatFlags; +} + + char * virDomainDefFormat(virDomainDefPtr def, unsigned int flags) { virBuffer buf = VIR_BUFFER_INITIALIZER; - virCheckFlags(DUMPXML_FLAGS, NULL); + virCheckFlags(VIR_DOMAIN_DEF_FORMAT_COMMON_FLAGS, NULL);

if (virDomainDefFormatInternal(def, flags, &buf) < 0) return NULL;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ac1f4f8..00ebdf8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h

@@ -2467,6 +2498,8 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, int virDomainDefAddImplicitControllers(virDomainDefPtr def); +int virDomainDefFormatConvertXMLFlags(unsigned int flags);

+ char *virDomainDefFormat(virDomainDefPtr def, unsigned int flags); int virDomainDefFormatInternal(virDomainDefPtr def,

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3d4023c..c4a9508 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c

@@ -2160,7 +2162,8 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, virUUIDFormat(vm->def->uuid, uuidstr); newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, - QEMU_DOMAIN_FORMAT_LIVE_FLAGS, 1); + VIR_DOMAIN_DEF_FORMAT_SECURE | + VIR_DOMAIN_DEF_FORMAT_UPDATE_CPU, 1);

if (newxml == NULL) return -1;