[libvirt] [PATCH] conf: do not count per-device boot elements when parsing <os><boot>

Ján Tomko posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/7320dbe22d7345a59de92e23c8758d6284811012.1503388101.git.jtomko@redhat.com
src/conf/domain_conf.c        | 46 ++++++++++++++++++++-----------------------
src/conf/domain_conf.h        |  3 ++-
src/lxc/lxc_native.c          |  2 +-
src/qemu/qemu_driver.c        |  6 +++---
src/qemu/qemu_parse_command.c |  2 +-
src/qemu/qemu_process.c       |  2 +-
src/vmx/vmx.c                 |  2 +-
src/xenconfig/xen_sxpr.c      |  2 +-
src/xenconfig/xen_xl.c        |  2 +-
src/xenconfig/xen_xm.c        |  2 +-
10 files changed, 33 insertions(+), 36 deletions(-)
[libvirt] [PATCH] conf: do not count per-device boot elements when parsing <os><boot>
Posted by Ján Tomko 6 years, 7 months ago
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        | 46 ++++++++++++++++++++-----------------------
 src/conf/domain_conf.h        |  3 ++-
 src/lxc/lxc_native.c          |  2 +-
 src/qemu/qemu_driver.c        |  6 +++---
 src/qemu/qemu_parse_command.c |  2 +-
 src/qemu/qemu_process.c       |  2 +-
 src/vmx/vmx.c                 |  2 +-
 src/xenconfig/xen_sxpr.c      |  2 +-
 src/xenconfig/xen_xl.c        |  2 +-
 src/xenconfig/xen_xm.c        |  2 +-
 10 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6f0f038b7..355da1478 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4784,7 +4784,8 @@ virDomainDefPostParseCPU(virDomainDefPtr def)
 
 static int
 virDomainDefPostParseInternal(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 @@ virDomainDefPostParseInternal(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 @@ virDomainDefPostParse(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 @@ virDomainDefPostParse(virDomainDefPtr def,
     if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0)
         goto cleanup;
 
-    if ((ret = virDomainDefPostParseInternal(def, &data)) < 0)
+    if ((ret = virDomainDefPostParseInternal(def, &data, bootHash)) < 0)
         goto cleanup;
 
     if (xmlopt->config.assignAddressesCallback) {
@@ -16135,28 +16151,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");
@@ -16175,10 +16174,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");
@@ -18947,7 +18942,8 @@ virDomainDefParseXML(xmlDocPtr xml,
         goto error;
 
     /* callback to fill driver specific domain aspects */
-    if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque) < 0)
+    if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque,
+                              bootHash) < 0)
         goto error;
 
     /* valdiate configuration */
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1f1dc1de0..2fe7e545d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2643,7 +2643,8 @@ int virDomainDefPostParse(virDomainDefPtr def,
                           virCapsPtr caps,
                           unsigned int parseFlags,
                           virDomainXMLOptionPtr xmlopt,
-                          void *parseOpaque);
+                          void *parseOpaque,
+                          virHashTablePtr bootHash);
 
 int virDomainDefValidate(virDomainDefPtr def,
                          virCapsPtr caps,
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 5fc6e7cda..1542eda11 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -1095,7 +1095,7 @@ lxcParseConfigString(const char *config,
     lxcSetCapDrop(vmdef, properties);
 
     if (virDomainDefPostParse(vmdef, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
-                              xmlopt, NULL) < 0)
+                              xmlopt, NULL, NULL) < 0)
         goto cleanup;
 
     goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9f07c6e7..f2cc1ee34 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7916,7 +7916,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
          return -1;
     }
 
-    if (virDomainDefPostParse(vmdef, caps, parse_flags, xmlopt, NULL) < 0)
+    if (virDomainDefPostParse(vmdef, caps, parse_flags, xmlopt, NULL, NULL) < 0)
         return -1;
 
     return 0;
@@ -8082,7 +8082,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
         return -1;
     }
 
-    if (virDomainDefPostParse(vmdef, caps, parse_flags, xmlopt, NULL) < 0)
+    if (virDomainDefPostParse(vmdef, caps, parse_flags, xmlopt, NULL, NULL) < 0)
         return -1;
 
     return 0;
@@ -8169,7 +8169,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
         return -1;
     }
 
-    if (virDomainDefPostParse(vmdef, caps, parse_flags, xmlopt, NULL) < 0)
+    if (virDomainDefPostParse(vmdef, caps, parse_flags, xmlopt, NULL, NULL) < 0)
         return -1;
 
     return 0;
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index ee7112775..345d31e6a 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -2639,7 +2639,7 @@ qemuParseCommandLine(virCapsPtr caps,
 
     VIR_FREE(nics);
 
-    if (virDomainDefPostParse(def, caps, 0, xmlopt, NULL) < 0)
+    if (virDomainDefPostParse(def, caps, 0, xmlopt, NULL, NULL) < 0)
         goto error;
 
     if (cmd->num_args || cmd->num_env) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 589d0ed2c..d0e1ad6f7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4696,7 +4696,7 @@ qemuProcessInit(virQEMUDriverPtr driver,
     if (vm->def->postParseFailed) {
         VIR_DEBUG("re-running the post parse callback");
 
-        if (virDomainDefPostParse(vm->def, caps, 0, driver->xmlopt, NULL) < 0)
+        if (virDomainDefPostParse(vm->def, caps, 0, driver->xmlopt, NULL, NULL) < 0)
             goto cleanup;
     }
 
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 3e2f4c3e1..822ba1230 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1819,7 +1819,7 @@ virVMXParseConfig(virVMXContext *ctx,
     }
 
     if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
-                              xmlopt, NULL) < 0)
+                              xmlopt, NULL, NULL) < 0)
         goto cleanup;
 
     success = true;
diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index fefa61ac2..ea19ca0bf 100644
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -1458,7 +1458,7 @@ xenParseSxpr(const struct sexpr *root,
     }
 
     if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
-                              xmlopt, NULL) < 0)
+                              xmlopt, NULL, NULL) < 0)
         goto error;
 
     return def;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index d168d3fa4..11eb1a2e6 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -882,7 +882,7 @@ xenParseXL(virConfPtr conf,
         goto cleanup;
 
     if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
-                              xmlopt, NULL) < 0)
+                              xmlopt, NULL, NULL) < 0)
         goto cleanup;
 
     return def;
diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index 8ef68bbc0..5f4572b84 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -461,7 +461,7 @@ xenParseXM(virConfPtr conf,
          goto cleanup;
 
     if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
-                              xmlopt, NULL) < 0)
+                              xmlopt, NULL, NULL) < 0)
         goto cleanup;
 
     return def;
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: do not count per-device boot elements when parsing <os><boot>
Posted by Peter Krempa 6 years, 7 months ago
On Tue, Aug 22, 2017 at 09:51:04 +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        | 46 ++++++++++++++++++++-----------------------
>  src/conf/domain_conf.h        |  3 ++-
>  src/lxc/lxc_native.c          |  2 +-
>  src/qemu/qemu_driver.c        |  6 +++---
>  src/qemu/qemu_parse_command.c |  2 +-
>  src/qemu/qemu_process.c       |  2 +-
>  src/vmx/vmx.c                 |  2 +-
>  src/xenconfig/xen_sxpr.c      |  2 +-
>  src/xenconfig/xen_xl.c        |  2 +-
>  src/xenconfig/xen_xm.c        |  2 +-
>  10 files changed, 33 insertions(+), 36 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6f0f038b7..355da1478 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 1f1dc1de0..2fe7e545d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2643,7 +2643,8 @@ int virDomainDefPostParse(virDomainDefPtr def,
>                            virCapsPtr caps,
>                            unsigned int parseFlags,
>                            virDomainXMLOptionPtr xmlopt,
> -                          void *parseOpaque);
> +                          void *parseOpaque,
> +                          virHashTablePtr bootHash);
>  
>  int virDomainDefValidate(virDomainDefPtr def,
>                           virCapsPtr caps,

Hmm, I think I'd prefer that you rename the function and create a thin
wrapper with the original name that will put the NULL argument in the
correct place. The boot hash won't really be ever passed from the
drivers. If it will be necessary it can be agregated from 'def' in the
wrapper.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list