Re: [libvirt] [PATCH v2 6/8] blockcopy: add a way to parse disk source

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(a)redhat.com&gt; --- 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); +

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;

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;

+ + 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;