The virDomainLoaderDef struct contains fields for both <loader> and <nvram> elements, so it makes sense to parse them in the same method, just like we'll format them in the same method. Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> --- src/conf/domain_conf.c | 92 ++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 52 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1b423298ce..d79af0b6c8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17795,29 +17795,51 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDef *def) } static int -virDomainLoaderDefParseXML(xmlNodePtr node, - virDomainLoaderDef *loader, - bool fwAutoSelect) +virDomainLoaderDefParseXML(virDomainDef *def, + xmlXPathContextPtr ctxt) { - if (!fwAutoSelect) { - if (virXMLPropTristateBool(node, "readonly", VIR_XML_PROP_NONE, - &loader->readonly) < 0) - return -1; + xmlNodePtr loader_node = virXPathNode("./os/loader[1]", ctxt); + xmlNodePtr nvram_node = virXPathNode("./os/nvram[1]", ctxt); + const bool fwAutoSelect = def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE; + virDomainLoaderDef *loader; + + if (!loader_node && !nvram_node) + return 0; + + def->os.loader = loader = g_new0(virDomainLoaderDef, 1); + + if (loader_node) { + if (!fwAutoSelect) { + if (virXMLPropTristateBool(loader_node, "readonly", VIR_XML_PROP_NONE, + &loader->readonly) < 0) + return -1; + + if (virXMLPropEnum(loader_node, "type", virDomainLoaderTypeFromString, + VIR_XML_PROP_NONZERO, &loader->type) < 0) + return -1; - if (virXMLPropEnum(node, "type", virDomainLoaderTypeFromString, - VIR_XML_PROP_NONZERO, &loader->type) < 0) + if (!(loader->path = virXMLNodeContentString(loader_node))) + return -1; + + if (STREQ(loader->path, "")) + VIR_FREE(loader->path); + } + + if (virXMLPropTristateBool(loader_node, "secure", VIR_XML_PROP_NONE, + &loader->secure) < 0) return -1; + } - if (!(loader->path = virXMLNodeContentString(node))) + if (nvram_node) { + if (!(loader->nvram = virXMLNodeContentString(nvram_node))) return -1; - if (STREQ(loader->path, "")) - VIR_FREE(loader->path); - } + if (STREQ(loader->nvram, "")) + VIR_FREE(loader->nvram); - if (virXMLPropTristateBool(node, "secure", VIR_XML_PROP_NONE, - &loader->secure) < 0) - return -1; + if (!fwAutoSelect) + loader->nvramTemplate = virXMLPropString(nvram_node, "template"); + } return 0; } @@ -18206,40 +18228,6 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def, } -static int -virDomainDefParseBootLoaderOptions(virDomainDef *def, - xmlXPathContextPtr ctxt) -{ - xmlNodePtr loader_node = virXPathNode("./os/loader[1]", ctxt); - xmlNodePtr nvram_node = virXPathNode("./os/nvram[1]", ctxt); - const bool fwAutoSelect = def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE; - virDomainLoaderDef *loader; - - if (!loader_node) - return 0; - - def->os.loader = loader = g_new0(virDomainLoaderDef, 1); - - if (virDomainLoaderDefParseXML(loader_node, - def->os.loader, - fwAutoSelect) < 0) - return -1; - - if (nvram_node) { - if (!(loader->nvram = virXMLNodeContentString(nvram_node))) - return -1; - - if (STREQ(loader->nvram, "")) - VIR_FREE(loader->nvram); - - if (!fwAutoSelect) - def->os.loader->nvramTemplate = virXMLPropString(nvram_node, "template"); - } - - return 0; -} - - static int virDomainDefParseBootAcpiOptions(virDomainDef *def, xmlXPathContextPtr ctxt) @@ -18304,7 +18292,7 @@ virDomainDefParseBootOptions(virDomainDef *def, if (virDomainDefParseBootFirmwareOptions(def, ctxt) < 0) return -1; - if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0) + if (virDomainLoaderDefParseXML(def, ctxt) < 0) return -1; if (virDomainDefParseBootAcpiOptions(def, ctxt) < 0) @@ -18320,7 +18308,7 @@ virDomainDefParseBootOptions(virDomainDef *def, case VIR_DOMAIN_OSTYPE_UML: virDomainDefParseBootKernelOptions(def, ctxt); - if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0) + if (virDomainLoaderDefParseXML(def, ctxt) < 0) return -1; break; -- 2.34.1