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

On Tue, Jan 13, 2015 at 14:13:19 +0000, Daniel Berrange wrote: > On Tue, Jan 13, 2015 at 03:05:48PM +0100, Jiri Denemark wrote: > > On Thu, Jan 08, 2015 at 15:48:20 +0000, Daniel Berrange wrote: > > > 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. > > ... ... > > > @@ -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); > > > > What about > > virDomainDefFormatConvertXMLFlags(QEMU_DOMAIN_FORMAT_LIVE_FLAGS) to make > > sure we still use the right flags in case QEMU_DOMAIN_FORMAT_LIVE_FLAGS > > is ever changed? > > QEMU_DOMAIN_FORMAT_LIVE_FLAGS uses the VIR_DOMAIN_XML flags from the > public API, whereas this method needs to use the internal flags. Exactly, that's why I suggested to use virDomainDefFormatConvertXMLFlags to convert them instead of hardcoding the result of this conversion.