virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls

This patch adds checks for empty bitmaps right after the calls of
virBitmapParse. These only include spots where set API's are called and
where domain's XML is parsed.
Also, it partially reverts commit 983f5a which added a check for
invalid nodeset "0,^0" into virBitmapParse function. This change broke
the logic, as an empty bitmap should not cause an error.

https://bugzilla.redhat.com/show_bug.cgi?id=1210545

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 823e003df88d80b866fee07001c8adb256e12869..f7f68ba250374c9b6f3102364b39008124572e07 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11577,6 +11577,12 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node,
         if (virBitmapParse(nodemask, 0, &def->sourceNodes,
                            VIR_DOMAIN_CPUMASK_LEN) < 0)
             goto cleanup;
+
+        if (virBitmapIsAllClear(def->sourceNodes)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Invalid value of 'nodemask': %s"), nodemask);
+            goto cleanup;
+        }
     }
 
     ret = 0;
@@ -13265,6 +13271,13 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node,
     if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
         goto error;
 
+    if (virBitmapIsAllClear(def->cpumask)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Invalid value of 'cpuset': %s"),
+                       tmp);
+        goto error;
+    }
+
  cleanup:
     VIR_FREE(tmp);
     ctxt->node = oldnode;
@@ -13366,6 +13379,12 @@ virDomainHugepagesParseXML(xmlNodePtr node,
         if (virBitmapParse(nodeset, 0, &hugepage->nodemask,
                            VIR_DOMAIN_CPUMASK_LEN) < 0)
             goto cleanup;
+
+        if (virBitmapIsAllClear(hugepage->nodemask)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Invalid value of 'nodeset': %s"), nodeset);
+            goto cleanup;
+        }
     }
 
     ret = 0;
@@ -13487,13 +13506,14 @@ virDomainThreadSchedParse(xmlNodePtr node,
         goto error;
     }
 
-    if (!virBitmapParse(tmp, 0, &sp->ids,
-                        VIR_DOMAIN_CPUMASK_LEN) ||
-        virBitmapIsAllClear(sp->ids) ||
+    if (virBitmapParse(tmp, 0, &sp->ids, VIR_DOMAIN_CPUMASK_LEN) < 0)
+        goto error;
+
+    if (virBitmapIsAllClear(sp->ids) ||
         virBitmapNextSetBit(sp->ids, -1) < minid ||
         virBitmapLastSetBit(sp->ids) > maxid) {
 
-        virReportError(VIR_ERR_XML_ERROR,
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("Invalid value of '%s': %s"),
                        name, tmp);
         goto error;
@@ -13861,6 +13881,13 @@ virDomainDefParseXML(xmlDocPtr xml,
             if (virBitmapParse(tmp, 0, &def->cpumask,
                                VIR_DOMAIN_CPUMASK_LEN) < 0)
                 goto error;
+
+            if (virBitmapIsAllClear(def->cpumask)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("Invalid value of 'cpuset': %s"), tmp);
+                goto error;
+            }
+
             VIR_FREE(tmp);
         }
     }

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 8a0f6867e7a5b7bf2dc17bdff6156dcd80205591..7ad3f661da0672e61d182dba9526f2b9e77bd36c 100644 (file)
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -178,6 +178,12 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr numa,
         if (virBitmapParse(tmp, 0, &mem_node->nodeset,
                            VIR_DOMAIN_CPUMASK_LEN) < 0)
             goto cleanup;
+
+        if (virBitmapIsAllClear(mem_node->nodeset)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Invalid value of 'nodeset': %s"), tmp);
+            goto cleanup;
+        }
         VIR_FREE(tmp);
     }
 
@@ -233,10 +239,19 @@ virDomainNumatuneParseXML(virDomainNumaPtr numa,
         }
         VIR_FREE(tmp);
 
-        if ((tmp = virXMLPropString(node, "nodeset")) &&
-            virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
-            goto cleanup;
-        VIR_FREE(tmp);
+        tmp = virXMLPropString(node, "nodeset");
+        if (tmp) {
+            if (virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
+                goto cleanup;
+
+            if (virBitmapIsAllClear(nodeset)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("Invalid value of 'nodeset': %s"), tmp);
+                goto cleanup;
+            }
+
+            VIR_FREE(tmp);
+        }
     }
 
     if (virDomainNumatuneSet(numa,

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f37a11e3974ee1690be79e81265605799bab8464..cbb6e1b6c14eb64e2acde4cbbf8abac5d2a84490 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10134,8 +10134,9 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
                 goto endjob;
 
             if (virBitmapIsAllClear(nodeset)) {
-                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-                               _("Invalid nodeset for numatune"));
+                virReportError(VIR_ERR_OPERATION_INVALID,
+                               _("Invalid nodeset of 'numatune': %s"),
+                               param->value.s);
                 goto endjob;
             }
         }

diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 5322bce65ee19769b9c2c43bdc578100fa8757a4..bf905aba4d9dab5aeb03a5540c5b18345227440c 100644 (file)
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -416,9 +416,6 @@ virBitmapParse(const char *str,
         }
     }
 
-    if (virBitmapIsAllClear(*bitmap))
-        goto error;
-
     return virBitmapCountBits(*bitmap);
 
  error:

diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index 5a170d3f6b0fe598942da0d66d66fa3c2ac8a536..d77abf3a6e283d577f3e77876035dc99f11b380d 100644 (file)
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -1165,6 +1165,13 @@ xenParseSxpr(const struct sexpr *root,
         if (virBitmapParse(cpus, 0, &def->cpumask,
                            VIR_DOMAIN_CPUMASK_LEN) < 0)
             goto error;
+
+        if (virBitmapIsAllClear(def->cpumask)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Invalid value of 'cpumask': %s"),
+                           cpus);
+            goto error;
+        }
     }
 
     def->maxvcpus = sexpr_int(root, "domain/vcpus");

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index f24727594aafe1e38ebc1c299181bcd4ed30ed28..9a84e4ccae5440ce8e88fd6f656ae161d37fc9f0 100644 (file)
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -524,16 +524,23 @@ static int
 test10(const void *opaque ATTRIBUTE_UNUSED)
 {
     int ret = -1;
-    virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL;
+    virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL;
 
     if (virBitmapParse("0-3,5-8,11-15", 0, &b1, 20) < 0 ||
         virBitmapParse("4,9,10,16-19", 0, &b2, 20) < 0 ||
-        virBitmapParse("15", 0, &b3, 20) < 0)
+        virBitmapParse("15", 0, &b3, 20) < 0 ||
+        virBitmapParse("0,^0", 0, &b4, 20) < 0)
+        goto cleanup;
+
+    if (!virBitmapIsAllClear(b4))
         goto cleanup;
 
     if (virBitmapOverlaps(b1, b2) ||
+        virBitmapOverlaps(b1, b4) ||
         virBitmapOverlaps(b2, b3) ||
-        !virBitmapOverlaps(b1, b3))
+        virBitmapOverlaps(b2, b4) ||
+        !virBitmapOverlaps(b1, b3) ||
+        virBitmapOverlaps(b3, b4))
         goto cleanup;
 
     ret = 0;