On 08/26/14 13:21, Eric Blake wrote: > The new blockcopy API wants to reuse only a subset of the disk > hotplug parser - namely, we only care about the embedded > virStorageSourcePtr inside a <disk> XML. Strange as it may > seem, it was easier to just parse an entire disk definition, > then throw away everything but the embedded source, than it > was to disentangle the source parsing code from the rest of > the overall disk parsing function. All that I needed was a > couple of tweaks and a new internal flag that determines > whether the normally-mandatory target element can be > gracefully skipped. > > While adding the new flag, I noticed we had a verify() that > was incomplete after several recent internal flag additions; > move that up higher in the code to make it harder to forget > to modify on the next flag addition. > > * src/conf/domain_conf.h (virDomainDiskSourceParse): New > prototype. > * src/conf/domain_conf.c (VIR_DOMAIN_XML_INTERNAL_DISK_SOURCE): > New flag. > (virDomainDiskDefParseXML): Honor flag to make target optional. > (virDomainDiskSourceParse): New function. > > Signed-off-by: Eric Blake <[email protected]> > --- > src/conf/domain_conf.c | 147 > +++++++++++++++++++++++++++++++---------------- > src/conf/domain_conf.h | 4 ++ > src/libvirt_private.syms | 1 + > 3 files changed, 103 insertions(+), 49 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index dd512ca..2ee2af0 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -89,19 +89,36 @@ struct _virDomainXMLOption { > > > /* Private flags used internally by virDomainSaveStatus and > - * virDomainLoadStatus. */ > + * virDomainLoadStatus, in addition to the public virDomainXMLFlags. */ > typedef enum { > /* dump internal domain status information */ > - VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), > + VIR_DOMAIN_XML_INTERNAL_STATUS = 1 << 16, > /* dump/parse <actual> element */ > - VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET = (1<<17), > + 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), > + 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); > +
Again, code tweaks and unrelated fixes ...
> VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
> "custom-argv",
> "custom-monitor",
> @@ -10359,7 +10379,7 @@ virDomainDeviceDefParse(const char *xmlStr,
> }
>
> /* callback to fill driver specific device aspects */
> - if (virDomainDeviceDefPostParse(dev, def, caps, xmlopt) < 0)
> + if (virDomainDeviceDefPostParse(dev, def, caps, xmlopt) < 0)
> goto error;
Unrelated.
>
> cleanup:
> @@ -10373,6 +10393,47 @@ virDomainDeviceDefParse(const char *xmlStr,
> }
>
>
> +virStorageSourcePtr
> +virDomainDiskDefSourceParse(const char *xmlStr,
> + const virDomainDef *def,
> + virDomainXMLOptionPtr xmlopt,
> + unsigned int flags)
> +{
> + xmlDocPtr xml;
> + xmlNodePtr node;
> + xmlXPathContextPtr ctxt = NULL;
> + virDomainDiskDefPtr disk = NULL;
> + virStorageSourcePtr ret = NULL;
> +
> + if (!(xml = virXMLParseStringCtxt(xmlStr, _("(disk_definition)"),
> &ctxt)))
> + goto cleanup;
> + node = ctxt->node;
Is the extra variable used to convert types?
> +
> + if (!xmlStrEqual(node->name, BAD_CAST "disk")) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("expecting root element of 'disk', not '%s'"),
> + node->name);
> + goto cleanup;
> + }
> +
> + flags |= VIR_DOMAIN_XML_INTERNAL_DISK_SOURCE;
> + if (!(disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
> + NULL, def->seclabels,
> + def->nseclabels,
> + flags)))
> + goto cleanup;
> +
> + ret = disk->src;
> + disk->src = NULL;
> +
> + cleanup:
> + virDomainDiskDefFree(disk);
> + xmlFreeDoc(xml);
> + xmlXPathFreeContext(ctxt);
> + return ret;
> +}
> +
> +
> static const char *
> virDomainChrTargetTypeToString(int deviceType,
> int targetType)
> @@ -17726,18 +17787,6 @@ virDomainHugepagesFormat(virBufferPtr buf,
> }
>
>
> -#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_CLOCK_ADJUST)
> - & DUMPXML_FLAGS) == 0);
> -
> static bool
> virDomainDefHasCapabilitiesFeatures(virDomainDefPtr def)
> {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index aead903..512d097 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2294,6 +2294,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const
> char *xmlStr,
> virCapsPtr caps,
> virDomainXMLOptionPtr xmlopt,
> unsigned int flags);
> +virStorageSourcePtr virDomainDiskDefSourceParse(const char *xmlStr,
> + const virDomainDef *def,
> + virDomainXMLOptionPtr xmlopt,
> + unsigned int flags);
> virDomainDefPtr virDomainDefParseString(const char *xmlStr,
> virCapsPtr caps,
> virDomainXMLOptionPtr xmlopt,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 6b9ee21..1195208 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -222,6 +222,7 @@ virDomainDiskDefAssignAddress;
> virDomainDiskDefForeachPath;
> virDomainDiskDefFree;
> virDomainDiskDefNew;
> +virDomainDiskDefSourceParse;
> virDomainDiskDeviceTypeToString;
> virDomainDiskDiscardTypeToString;
> virDomainDiskErrorPolicyTypeFromString;
>
I'd really prefer the unrelated tweaks posted separately.
Again, what you have works so ACK to this patch even if you decide
against splitting it up.
Peter
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
