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

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,
+    if (virBitmapParse(tmp, 0, &sp->ids, VIR_DOMAIN_CPUMASK_LEN) < 0)
-                        VIR_DOMAIN_CPUMASK_LEN) ||
+        goto error;
-        virBitmapIsAllClear(sp->ids) ||
+
+    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);
         }
     }

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")) &&
+        tmp = virXMLPropString(node, "nodeset");
-            virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
+        if (tmp) {
-            goto cleanup;
+            if (virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
-        VIR_FREE(tmp);
+                goto cleanup;
+
+            if (virBitmapIsAllClear(nodeset)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("Invalid value of 'nodeset': %s"), tmp);
+                goto cleanup;
+            }
+
+            VIR_FREE(tmp);
+        }
     }
 
     if (virDomainNumatuneSet(numa,

src/qemu/qemu_driver.c

@@ -10134,8 +10134,9 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
                 goto endjob;
 
             if (virBitmapIsAllClear(nodeset)) {
-                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                virReportError(VIR_ERR_OPERATION_INVALID,
-                               _("Invalid nodeset for numatune"));
+                               _("Invalid nodeset of 'numatune': %s"),
+                               param->value.s);
                 goto endjob;
             }
         }

src/util/virbitmap.c

@@ -416,9 +416,6 @@ virBitmapParse(const char *str,
         }
     }
 
-    if (virBitmapIsAllClear(*bitmap))
-        goto error;
-
     return virBitmapCountBits(*bitmap);
 
  error:

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");

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;