virBitmapParse: Fix behavior in case of error and fix up callers

Re-arrange the code so that the returned bitmap is always initialized to
NULL even on early failures and return an error message as some callers
are already expecting it. Fix up the rest not to shadow the error.

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 36d027a5c6154a942ae4afb68d026741fcf1f6ac..9a830691ea38a6d3d15428e141113a7db3cb5be5 100644 (file)
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -147,6 +147,7 @@ src/util/viralloc.c
 src/util/viraudit.c
 src/util/virauth.c
 src/util/virauthconfig.c
+src/util/virbitmap.c
 src/util/vircgroup.c
 src/util/virclosecallbacks.c
 src/util/vircommand.c

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ea49d2cda38af3ddde7fae8eaaa187cd77fd5d52..8a187a63d3262d2bfd7e020dee75bcfc31e64c25 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10981,11 +10981,8 @@ virDomainDefParseXML(xmlDocPtr xml,
         tmp = virXPathString("string(./vcpu[1]/@cpuset)", ctxt);
         if (tmp) {
             if (virBitmapParse(tmp, 0, &def->cpumask,
-                               VIR_DOMAIN_CPUMASK_LEN) < 0) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               "%s", _("topology cpuset syntax error"));
+                               VIR_DOMAIN_CPUMASK_LEN) < 0)
                 goto error;
-            }
             VIR_FREE(tmp);
         }
     }

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index bbc980b69364605c0b1fa5bae30f50231b368c57..8aef6098309e3c6078474775e938e0961e483636 100644 (file)
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2897,9 +2897,6 @@ virNetworkLoadState(virNetworkObjListPtr nets,
         if ((class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt))) {
             if (virBitmapParse(class_id, 0, &class_id_map,
                                CLASS_ID_BITMAP_SIZE) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Malformed 'class_id' attribute: %s"),
-                               class_id);
                 VIR_FREE(class_id);
                 goto error;
             }

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 4df48515bf2be663d70c57a7954923361937f244..33a79b7ae93221efe3e5ff7c03702ac8af8543d0 100644 (file)
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1547,11 +1547,8 @@ virNodeGetSiblingsList(const char *dir, int cpu_id)
     if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGTH_MAX, &buf) < 0)
         goto cleanup;
 
-    if (virBitmapParse(buf, 0, &ret, NUMA_MAX_N_CPUS) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Failed to parse thread siblings"));
+    if (virBitmapParse(buf, 0, &ret, NUMA_MAX_N_CPUS) < 0)
         goto cleanup;
-    }
 
 cleanup:
     VIR_FREE(buf);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2ad236e0e187af04b8c0c08ae1c27ae1cbdcdba8..5124f271ad557763be6a9b4baa85c368a5b01945 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8357,8 +8357,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
             if (virBitmapParse(params[i].value.s,
                                0, &nodeset,
                                VIR_DOMAIN_CPUMASK_LEN) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("Failed to parse nodeset"));
                 ret = -1;
                 continue;
             }

diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 47c678ee0a2bf5a553be951044c4184f713990d8..870c8fe8142407ed5d2cc2b9fa2d70cfda9385e2 100644 (file)
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -298,23 +298,21 @@ virBitmapParse(const char *str,
                size_t bitmapSize)
 {
     bool neg = false;
-    const char *cur;
+    const char *cur = str;
     char *tmp;
     size_t i;
     int start, last;
 
-    if (!str)
+    if (!(*bitmap = virBitmapNew(bitmapSize)))
         return -1;
 
-    cur = str;
-    virSkipSpaces(&cur);
+    if (!str)
+        goto error;
 
-    if (*cur == 0)
-        return -1;
+    virSkipSpaces(&cur);
 
-    *bitmap = virBitmapNew(bitmapSize);
-    if (!*bitmap)
-        return -1;
+    if (*cur == '\0')
+        goto error;
 
     while (*cur != 0 && *cur != terminator) {
         /*
@@ -384,6 +382,8 @@ virBitmapParse(const char *str,
     return virBitmapCountBits(*bitmap);
 
 error:
+    virReportError(VIR_ERR_INVALID_ARG,
+                   _("Failed to parse bitmap '%s'"), str);
     virBitmapFree(*bitmap);
     *bitmap = NULL;
     return -1;

diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index fbbbaa9389d458effe175b32796360b7c75ad7a8..6209c683a659dbd34f478171d9158eaa217ed1f1 100644 (file)
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1160,11 +1160,8 @@ xenParseSxpr(const struct sexpr *root,
 
     if (cpus != NULL) {
         if (virBitmapParse(cpus, 0, &def->cpumask,
-                           VIR_DOMAIN_CPUMASK_LEN) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("invalid CPU mask %s"), cpus);
+                           VIR_DOMAIN_CPUMASK_LEN) < 0)
             goto error;
-        }
     }
 
     def->maxvcpus = sexpr_int(root, "domain/vcpus");

diff --git a/tests/cpuset b/tests/cpuset
index b617d6fad9898efee147601fe9cf3488e0c08ebf..35803be02c1db6e73294d094ff521608fb84be8c 100755 (executable)
--- a/tests/cpuset
+++ b/tests/cpuset
@@ -42,7 +42,7 @@ sed "s/vcpu placement='static'>/vcpu cpuset='aaa'>/" xml > xml-invalid || fail=1
 $abs_top_builddir/tools/virsh --connect test:///default define xml-invalid > out 2>&1 && fail=1
 cat <<\EOF > exp || fail=1
 error: Failed to define domain from xml-invalid
-error: XML error: topology cpuset syntax error
+error: invalid argument: Failed to parse bitmap 'aaa'
 
 EOF
 compare exp out || fail=1