[libvirt] [PATCHv2 0/3] conf: do not count per-device boot elements when parsing <os><boot>
Ján Tomko (3): conf: rename virDomain*PostParseInternal to virDomain*PostParseCommon conf: create a thin wrapper above virDomainDefPostParse conf: do not count per-device boot elements when parsing <os><boot> src/conf/domain_conf.c | 82 +++++++++++++++++++++++++++----------------------- 1 file changed, 45 insertions(+), 37 deletions(-) -- 2.13.0
These functions contain the post-parse steps common for all drivers. Rename it to use the 'Common' prefix, instead of the vagueness of 'Internal', leaving 'Internal' available for other vague uses. --- src/conf/domain_conf.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6f0f038b7..0315ec6ff 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4367,11 +4367,11 @@ virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio) static int -virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, - const virDomainDef *def, - virCapsPtr caps ATTRIBUTE_UNUSED, - unsigned int parseFlags ATTRIBUTE_UNUSED, - virDomainXMLOptionPtr xmlopt) +virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, + const virDomainDef *def, + virCapsPtr caps ATTRIBUTE_UNUSED, + unsigned int parseFlags ATTRIBUTE_UNUSED, + virDomainXMLOptionPtr xmlopt) { if (dev->type == VIR_DOMAIN_DEVICE_CHR) { virDomainChrDefPtr chr = dev->data.chr; @@ -4667,7 +4667,7 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, return ret; } - if ((ret = virDomainDeviceDefPostParseInternal(dev, def, caps, flags, xmlopt)) < 0) + if ((ret = virDomainDeviceDefPostParseCommon(dev, def, caps, flags, xmlopt)) < 0) return ret; if (virDomainDeviceDefPostParseCheckFeatures(dev, xmlopt) < 0) @@ -4783,8 +4783,8 @@ virDomainDefPostParseCPU(virDomainDefPtr def) static int -virDomainDefPostParseInternal(virDomainDefPtr def, - struct virDomainDefPostParseDeviceIteratorData *data) +virDomainDefPostParseCommon(virDomainDefPtr def, + struct virDomainDefPostParseDeviceIteratorData *data) { /* verify init path for container based domains */ if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { @@ -4917,7 +4917,7 @@ virDomainDefPostParse(virDomainDefPtr def, if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0) goto cleanup; - if ((ret = virDomainDefPostParseInternal(def, &data)) < 0) + if ((ret = virDomainDefPostParseCommon(def, &data)) < 0) goto cleanup; if (xmlopt->config.assignAddressesCallback) { -- 2.13.0
On Tue, Aug 22, 2017 at 13:50:37 +0200, Ján Tomko wrote:
These functions contain the post-parse steps common for all drivers. Rename it to use the 'Common' prefix, instead of the vagueness of 'Internal', leaving 'Internal' available for other vague uses. --- src/conf/domain_conf.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
ACK
Rename the original function to virDomainDefPostParseInternal to allow adding arguments that will be only used by the internal version. --- src/conf/domain_conf.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0315ec6ff..396ace329 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4856,12 +4856,12 @@ virDomainDefPostParseCheckFailure(virDomainDefPtr def, } -int -virDomainDefPostParse(virDomainDefPtr def, - virCapsPtr caps, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque) +static int +virDomainDefPostParseInternal(virDomainDefPtr def, + virCapsPtr caps, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque) { int ret = -1; bool localParseOpaque = false; @@ -4944,6 +4944,18 @@ virDomainDefPostParse(virDomainDefPtr def, } +int +virDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque) +{ + return virDomainDefPostParseInternal(def, caps, parseFlags, xmlopt, + parseOpaque); +} + + /** * virDomainDiskAddressDiskBusCompatibility: * @bus: disk bus type -- 2.13.0
On Tue, Aug 22, 2017 at 13:50:38 +0200, Ján Tomko wrote:
Rename the original function to virDomainDefPostParseInternal to allow adding arguments that will be only used by the internal version. --- src/conf/domain_conf.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
ACK
When parsing bootable devices, we maintain a bitmap of used <boot order=""> elements. Use it in the post-parse function to figure out whether the user tried to mix per-device and per-domain boot elements. This removes the need to count them twice. --- src/conf/domain_conf.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 396ace329..10003bb2e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4784,7 +4784,8 @@ virDomainDefPostParseCPU(virDomainDefPtr def) static int virDomainDefPostParseCommon(virDomainDefPtr def, - struct virDomainDefPostParseDeviceIteratorData *data) + struct virDomainDefPostParseDeviceIteratorData *data, + virHashTablePtr bootHash) { /* verify init path for container based domains */ if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { @@ -4793,6 +4794,20 @@ virDomainDefPostParseCommon(virDomainDefPtr def, return -1; } + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && bootHash) { + if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("per-device boot elements cannot be used" + " together with os/boot elements")); + return -1; + } + + if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) { + def->os.nBootDevs = 1; + def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; + } + } + if (virDomainVcpuDefPostParse(def) < 0) return -1; @@ -4861,7 +4876,8 @@ virDomainDefPostParseInternal(virDomainDefPtr def, virCapsPtr caps, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt, - void *parseOpaque) + void *parseOpaque, + virHashTablePtr bootHash) { int ret = -1; bool localParseOpaque = false; @@ -4917,7 +4933,7 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0) goto cleanup; - if ((ret = virDomainDefPostParseCommon(def, &data)) < 0) + if ((ret = virDomainDefPostParseCommon(def, &data, bootHash)) < 0) goto cleanup; if (xmlopt->config.assignAddressesCallback) { @@ -4952,7 +4968,7 @@ virDomainDefPostParse(virDomainDefPtr def, void *parseOpaque) { return virDomainDefPostParseInternal(def, caps, parseFlags, xmlopt, - parseOpaque); + parseOpaque, NULL); } @@ -16147,28 +16163,11 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, int n; char *tmp = NULL; int ret = -1; - unsigned long deviceBoot; - - if (virXPathULong("count(./devices/disk[boot]" - "|./devices/interface[boot]" - "|./devices/hostdev[boot]" - "|./devices/redirdev[boot])", ctxt, &deviceBoot) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot count boot devices")); - goto cleanup; - } /* analysis of the boot devices */ if ((n = virXPathNodeSet("./os/boot", ctxt, &nodes)) < 0) goto cleanup; - if (n > 0 && deviceBoot) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("per-device boot elements cannot be used" - " together with os/boot elements")); - goto cleanup; - } - for (i = 0; i < n && i < VIR_DOMAIN_BOOT_LAST; i++) { int val; char *dev = virXMLPropString(nodes[i], "dev"); @@ -16187,10 +16186,6 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, VIR_FREE(dev); def->os.bootDevs[def->os.nBootDevs++] = val; } - if (def->os.nBootDevs == 0 && !deviceBoot) { - def->os.nBootDevs = 1; - def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; - } if ((node = virXPathNode("./os/bootmenu[1]", ctxt))) { tmp = virXMLPropString(node, "enable"); @@ -18959,7 +18954,8 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; /* callback to fill driver specific domain aspects */ - if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque) < 0) + if (virDomainDefPostParseInternal(def, caps, flags, xmlopt, parseOpaque, + bootHash) < 0) goto error; /* valdiate configuration */ -- 2.13.0
On Tue, Aug 22, 2017 at 13:50:39 +0200, Ján Tomko wrote:
When parsing bootable devices, we maintain a bitmap of used <boot order=""> elements. Use it in the post-parse function to figure out whether the user tried to mix per-device and per-domain boot elements.
This removes the need to count them twice. --- src/conf/domain_conf.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-)
ACK
participants (2)
-
Ján Tomko -
Peter Krempa