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 <eblake@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list