[PATCH 07/25] conf: domain: Split out parsing of source data from <disk> XML parser

Extract all code related to parsing data which ends up in the 'src' member of a virDomainDiskDef. This allows to use the new function directly in virDomainDiskDefParseSource and removes the use of the VIR_DOMAIN_DEF_PARSE_DISK_SOURCE parser flag. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 185 ++++++++++++++++++++++------------------- 1 file changed, 100 insertions(+), 85 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0eec4c9356..1793032301 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9341,6 +9341,86 @@ virDomainDiskDefParsePrivateData(xmlXPathContextPtr ctxt, } +static virStorageSource * +virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + g_autoptr(virStorageSource) src = virStorageSourceNew(); + VIR_XPATH_NODE_AUTORESTORE(ctxt) + g_autofree char *type = NULL; + xmlNodePtr tmp; + + ctxt->node = node; + + src->type = VIR_STORAGE_TYPE_FILE; + + if ((type = virXMLPropString(node, "type")) && + (src->type = virStorageTypeFromString(type)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown disk type '%s'"), type); + return NULL; + } + + if ((tmp = virXPathNode("./source[1]", ctxt))) { + if (virDomainStorageSourceParse(tmp, ctxt, src, flags, xmlopt) < 0) + return NULL; + + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { + g_autofree char *sourceindex = NULL; + + if ((sourceindex = virXMLPropString(tmp, "index")) && + virStrToLong_uip(sourceindex, NULL, 10, &src->id) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid disk index '%s'"), sourceindex); + return NULL; + } + } + } else { + /* Reset src->type in case when 'source' was not present */ + src->type = VIR_STORAGE_TYPE_FILE; + } + + if (virXPathNode("./readonly[1]", ctxt)) + src->readonly = true; + + if (virXPathNode("./shareable[1]", ctxt)) + src->shared = true; + + if ((tmp = virXPathNode("./auth", ctxt))) { + /* If we've already parsed <source> and found an <auth> child, + * then generate an error to avoid ambiguity */ + if (src->auth) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an <auth> definition already found for disk source")); + return NULL; + } + + if (!(src->auth = virStorageAuthDefParse(tmp, ctxt))) + return NULL; + } + + if ((tmp = virXPathNode("./encryption", ctxt))) { + /* If we've already parsed <source> and found an <encryption> child, + * then generate an error to avoid ambiguity */ + if (src->encryption) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an <encryption> definition already found for disk source")); + return NULL; + } + + if (!(src->encryption = virStorageEncryptionParseNode(tmp, ctxt))) + return NULL; + } + + if (virDomainDiskBackingStoreParse(ctxt, src, flags, xmlopt) < 0) + return NULL; + + return g_steal_pointer(&src); +} + + static virDomainDiskDef * virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr node, @@ -9351,8 +9431,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr cur; VIR_XPATH_NODE_AUTORESTORE(ctxt) bool source = false; - g_autoptr(virStorageEncryption) encryption = NULL; - g_autoptr(virStorageAuthDef) authdef = NULL; g_autofree char *tmp = NULL; g_autofree char *snapshot = NULL; g_autofree char *rawio = NULL; @@ -9368,24 +9446,19 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *product = NULL; g_autofree char *domain_name = NULL; g_autofree char *rotation_rate = NULL; + g_autoptr(virStorageSource) src = NULL; + + if (!(src = virDomainDiskDefParseSourceXML(xmlopt, node, ctxt, flags))) + return NULL; - if (!(def = virDomainDiskDefNew(xmlopt))) + if (!(def = virDomainDiskDefNewSource(xmlopt, &src))) return NULL; ctxt->node = node; /* defaults */ - def->src->type = VIR_STORAGE_TYPE_FILE; def->device = VIR_DOMAIN_DISK_DEVICE_DISK; - if ((tmp = virXMLPropString(node, "type")) && - (def->src->type = virStorageTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk type '%s'"), tmp); - return NULL; - } - VIR_FREE(tmp); - if ((tmp = virXMLPropString(node, "device")) && (def->device = virDomainDiskDeviceTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -9412,9 +9485,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, continue; if (!source && virXMLNodeNameEqual(cur, "source")) { - if (virDomainStorageSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0) - return NULL; - source = true; if (virXMLPropEnum(cur, "startupPolicy", @@ -9423,13 +9493,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, &def->startupPolicy) < 0) return NULL; - if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && - (tmp = virXMLPropString(cur, "index")) && - virStrToLong_uip(tmp, NULL, 10, &def->src->id) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("invalid disk index '%s'"), tmp); - return NULL; - } - VIR_FREE(tmp); } else if (!target && virXMLNodeNameEqual(cur, "target")) { target = virXMLPropString(cur, "dev"); @@ -9484,27 +9547,15 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { if (virDomainDiskDefMirrorParse(def, cur, ctxt, flags, xmlopt) < 0) return NULL; - } else if (!authdef && - virXMLNodeNameEqual(cur, "auth")) { - if (!(authdef = virStorageAuthDefParse(cur, ctxt))) - return NULL; + } else if (virXMLNodeNameEqual(cur, "auth")) { def->diskElementAuth = true; } else if (virXMLNodeNameEqual(cur, "iotune")) { if (virDomainDiskDefIotuneParse(def, ctxt) < 0) return NULL; - } else if (virXMLNodeNameEqual(cur, "readonly")) { - def->src->readonly = true; - } else if (virXMLNodeNameEqual(cur, "shareable")) { - def->src->shared = true; } else if (virXMLNodeNameEqual(cur, "transient")) { def->transient = true; - } else if (!encryption && - virXMLNodeNameEqual(cur, "encryption")) { - if (!(encryption = virStorageEncryptionParseNode(cur, ctxt))) - return NULL; - + } else if (virXMLNodeNameEqual(cur, "encryption")) { def->diskElementEnc = true; - } else if (!serial && virXMLNodeNameEqual(cur, "serial")) { if (!(serial = virXMLNodeContentString(cur))) @@ -9548,10 +9599,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - /* Reset def->src->type in case when 'source' was not present */ - if (!source) - def->src->type = VIR_STORAGE_TYPE_FILE; - /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device presently */ @@ -9676,40 +9723,12 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } def->dst = g_steal_pointer(&target); - if (authdef) { - /* If we've already parsed <source> and found an <auth> child, - * then generate an error to avoid ambiguity */ - if (def->src->auth) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("an <auth> definition already found for " - "disk source")); - return NULL; - } - - def->src->auth = g_steal_pointer(&authdef); - } - - if (encryption) { - /* If we've already parsed <source> and found an <encryption> child, - * then generate an error to avoid ambiguity */ - if (def->src->encryption) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("an <encryption> definition already found for " - "disk source")); - return NULL; - } - - def->src->encryption = g_steal_pointer(&encryption); - } def->domain_name = g_steal_pointer(&domain_name); def->serial = g_steal_pointer(&serial); def->wwn = g_steal_pointer(&wwn); def->vendor = g_steal_pointer(&vendor); def->product = g_steal_pointer(&product); - if (virDomainDiskBackingStoreParse(ctxt, def->src, flags, xmlopt) < 0) - return NULL; - if (flags & VIR_DOMAIN_DEF_PARSE_STATUS && virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0) return NULL; @@ -9721,22 +9740,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } -static virStorageSource * -virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt, - xmlNodePtr node, - xmlXPathContextPtr ctxt, - unsigned int flags) -{ - g_autoptr(virDomainDiskDef) diskdef = NULL; - - if (!(diskdef = virDomainDiskDefParseXML(xmlopt, node, ctxt, - flags | VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) - return NULL; - - return g_steal_pointer(&diskdef->src); -} - - /** * virDomainParseMemory: * @xpath: XPath to memory amount @@ -16484,11 +16487,23 @@ virDomainDiskDefParseSource(const char *xmlStr, { g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; + g_autoptr(virStorageSource) src = NULL; if (!(xml = virXMLParseStringCtxtRoot(xmlStr, _("(disk_definition)"), "disk", &ctxt))) return NULL; - return virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags); + if (!(src = virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags))) + return NULL; + + if (virStorageSourceIsEmpty(src)) { + virReportError(VIR_ERR_NO_SOURCE, NULL); + return NULL; + } + + if (virDomainDiskDefParseValidateSource(src) < 0) + return NULL; + + return g_steal_pointer(&src); } -- 2.30.2