[libvirt] [PATCH 0/5] Allow disjunct ranges in VIR_TEST_RANGE

Now you can do: VIR_TEST_RANGE=7-14,^10,1024 ./virschematest Ján Tomko (5): Do not return number of set bits in virBitmapParse Introduce virBitmapParseSeparator Remove separator argument from virBitmapParse Introduce virBitmapParseUnlimited Allow disjunct ranges in VIR_TEST_RANGE src/conf/domain_conf.c | 15 +++-- src/conf/network_conf.c | 2 +- src/conf/numa_conf.c | 7 +-- src/libvirt_private.syms | 2 + src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_native.c | 4 +- src/nodeinfo.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virbitmap.c | 149 ++++++++++++++++++++++++++++++++++++++++++--- src/util/virbitmap.h | 12 +++- src/util/virhostcpu.c | 2 +- src/vz/vz_sdk.c | 2 +- src/xen/xend_internal.c | 5 +- src/xenconfig/xen_common.c | 2 +- src/xenconfig/xen_sxpr.c | 3 +- tests/qemuxml2argvtest.c | 2 +- tests/testutils.c | 30 ++------- tests/virbitmaptest.c | 31 ++++++---- tests/vircapstest.c | 2 +- tools/virsh-domain.c | 2 +- 22 files changed, 205 insertions(+), 77 deletions(-) -- 2.7.3

This is only used by one caller. --- src/util/virbitmap.c | 4 ++-- src/xen/xend_internal.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 4ca59f9..7e9f3fd 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -419,7 +419,7 @@ char *virBitmapFormat(virBitmapPtr bitmap) * Pass 0 as @terminator if it is not needed. Whitespace characters may not * be used as terminators. * - * Returns the number of bits set in @bitmap, or -1 in case of error. + * Returns 0 on success, or -1 in case of error. */ int virBitmapParse(const char *str, @@ -509,7 +509,7 @@ virBitmapParse(const char *str, } } - return virBitmapCountBits(*bitmap); + return 0; error: virReportError(VIR_ERR_INVALID_ARG, diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 3e3be58..21ccff9 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1049,9 +1049,10 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) if (!(cpuset = virBitmapNew(numCpus))) goto error; } else { - nb_cpus = virBitmapParse(cur, 'n', &cpuset, numCpus); - if (nb_cpus < 0) + if (virBitmapParse(cur, 'n', &cpuset, numCpus) < 0) goto error; + + nb_cpus = virBitmapCountBits(cpuset); } if (VIR_ALLOC_N(cpuInfo, numCpus) < 0) { -- 2.7.3

On Fri, Jun 17, 2016 at 03:33:39PM +0200, Ján Tomko wrote:
This is only used by one caller. --- src/util/virbitmap.c | 4 ++-- src/xen/xend_internal.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-)
ACK

This will be used for the caller that needs to specify a separator. Currently identical to virBitmapParse. --- src/libvirt_private.syms | 1 + src/util/virbitmap.c | 41 ++++++++++++++++++++++++++++++++++++----- src/util/virbitmap.h | 5 +++++ src/xen/xend_internal.c | 2 +- tests/virbitmaptest.c | 2 +- 5 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index af30476..a88a3d1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1206,6 +1206,7 @@ virBitmapNextClearBit; virBitmapNextSetBit; virBitmapOverlaps; virBitmapParse; +virBitmapParseSeparator; virBitmapSetAll; virBitmapSetBit; virBitmapSetBitExpand; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 7e9f3fd..139e3ba 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -400,7 +400,7 @@ char *virBitmapFormat(virBitmapPtr bitmap) } /** - * virBitmapParse: + * virBitmapParseSeparator: * @str: points to a string representing a human-readable bitmap * @terminator: character separating the bitmap to parse * @bitmap: a bitmap created from @str @@ -422,10 +422,10 @@ char *virBitmapFormat(virBitmapPtr bitmap) * Returns 0 on success, or -1 in case of error. */ int -virBitmapParse(const char *str, - char terminator, - virBitmapPtr *bitmap, - size_t bitmapSize) +virBitmapParseSeparator(const char *str, + char terminator, + virBitmapPtr *bitmap, + size_t bitmapSize) { bool neg = false; const char *cur = str; @@ -520,6 +520,37 @@ virBitmapParse(const char *str, } /** + * virBitmapParse: + * @str: points to a string representing a human-readable bitmap + * @terminator: character separating the bitmap to parse + * @bitmap: a bitmap created from @str + * @bitmapSize: the upper limit of num of bits in created bitmap + * + * This function is the counterpart of virBitmapFormat. This function creates + * a bitmap, in which bits are set according to the content of @str. + * + * @str is a comma separated string of fields N, which means a number of bit + * to set, and ^N, which means to unset the bit, and N-M for ranges of bits + * to set. + * + * To allow parsing of bitmaps within larger strings it is possible to set + * a termination character in the argument @terminator. When the character + * in @terminator is encountered in @str, the parsing of the bitmap stops. + * Pass 0 as @terminator if it is not needed. Whitespace characters may not + * be used as terminators. + * + * Returns 0 on success, or -1 in case of error. + */ +int +virBitmapParse(const char *str, + char terminator, + virBitmapPtr *bitmap, + size_t bitmapSize) +{ + return virBitmapParseSeparator(str, terminator, bitmap, bitmapSize); +} + +/** * virBitmapNewCopy: * @src: the source bitmap. * diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 79f53dd..79922ea 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -90,6 +90,11 @@ int virBitmapParse(const char *str, virBitmapPtr *bitmap, size_t bitmapSize) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); +int +virBitmapParseSeparator(const char *str, + char terminator, + virBitmapPtr *bitmap, + size_t bitmapSize); virBitmapPtr virBitmapNewCopy(virBitmapPtr src) ATTRIBUTE_NONNULL(1); diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 21ccff9..605c3cd 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1049,7 +1049,7 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) if (!(cpuset = virBitmapNew(numCpus))) goto error; } else { - if (virBitmapParse(cur, 'n', &cpuset, numCpus) < 0) + if (virBitmapParseSeparator(cur, 'n', &cpuset, numCpus) < 0) goto error; nb_cpus = virBitmapCountBits(cpuset); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 00369af..5fa13c2 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -526,7 +526,7 @@ test10(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL; - if (virBitmapParse("0-3,5-8,11-15", 0, &b1, 20) < 0 || + if (virBitmapParseSeparator("0-3,5-8,11-15f16", 'f', &b1, 20) < 0 || virBitmapParse("4,9,10,16-19", 0, &b2, 20) < 0 || virBitmapParse("15", 0, &b3, 20) < 0 || virBitmapParse("0,^0", 0, &b4, 20) < 0) -- 2.7.3

On Fri, Jun 17, 2016 at 03:33:40PM +0200, Ján Tomko wrote:
This will be used for the caller that needs to specify a separator. Currently identical to virBitmapParse. --- src/libvirt_private.syms | 1 + src/util/virbitmap.c | 41 ++++++++++++++++++++++++++++++++++++----- src/util/virbitmap.h | 5 +++++ src/xen/xend_internal.c | 2 +- tests/virbitmaptest.c | 2 +- 5 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index af30476..a88a3d1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1206,6 +1206,7 @@ virBitmapNextClearBit; virBitmapNextSetBit; virBitmapOverlaps; virBitmapParse; +virBitmapParseSeparator; virBitmapSetAll; virBitmapSetBit; virBitmapSetBitExpand; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 7e9f3fd..139e3ba 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -400,7 +400,7 @@ char *virBitmapFormat(virBitmapPtr bitmap) }
/** - * virBitmapParse: + * virBitmapParseSeparator: * @str: points to a string representing a human-readable bitmap * @terminator: character separating the bitmap to parse * @bitmap: a bitmap created from @str @@ -422,10 +422,10 @@ char *virBitmapFormat(virBitmapPtr bitmap) * Returns 0 on success, or -1 in case of error. */ int -virBitmapParse(const char *str, - char terminator, - virBitmapPtr *bitmap, - size_t bitmapSize) +virBitmapParseSeparator(const char *str, + char terminator, + virBitmapPtr *bitmap, + size_t bitmapSize) { bool neg = false; const char *cur = str; @@ -520,6 +520,37 @@ virBitmapParse(const char *str, }
/** + * virBitmapParse: + * @str: points to a string representing a human-readable bitmap + * @terminator: character separating the bitmap to parse + * @bitmap: a bitmap created from @str + * @bitmapSize: the upper limit of num of bits in created bitmap + * + * This function is the counterpart of virBitmapFormat. This function creates + * a bitmap, in which bits are set according to the content of @str. + * + * @str is a comma separated string of fields N, which means a number of bit + * to set, and ^N, which means to unset the bit, and N-M for ranges of bits + * to set. + * + * To allow parsing of bitmaps within larger strings it is possible to set + * a termination character in the argument @terminator. When the character + * in @terminator is encountered in @str, the parsing of the bitmap stops. + * Pass 0 as @terminator if it is not needed. Whitespace characters may not + * be used as terminators. + * + * Returns 0 on success, or -1 in case of error. + */ +int +virBitmapParse(const char *str, + char terminator, + virBitmapPtr *bitmap, + size_t bitmapSize) +{ + return virBitmapParseSeparator(str, terminator, bitmap, bitmapSize); +} + +/** * virBitmapNewCopy: * @src: the source bitmap. * diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 79f53dd..79922ea 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -90,6 +90,11 @@ int virBitmapParse(const char *str, virBitmapPtr *bitmap, size_t bitmapSize) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); +int +virBitmapParseSeparator(const char *str, + char terminator, + virBitmapPtr *bitmap, + size_t bitmapSize);
virBitmapPtr virBitmapNewCopy(virBitmapPtr src) ATTRIBUTE_NONNULL(1);
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 21ccff9..605c3cd 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1049,7 +1049,7 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) if (!(cpuset = virBitmapNew(numCpus))) goto error; } else { - if (virBitmapParse(cur, 'n', &cpuset, numCpus) < 0) + if (virBitmapParseSeparator(cur, 'n', &cpuset, numCpus) < 0) goto error;
nb_cpus = virBitmapCountBits(cpuset); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 00369af..5fa13c2 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -526,7 +526,7 @@ test10(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL;
- if (virBitmapParse("0-3,5-8,11-15", 0, &b1, 20) < 0 || + if (virBitmapParseSeparator("0-3,5-8,11-15f16", 'f', &b1, 20) < 0 ||
You are changing the zero here to 'f' on purpose? If yes, please either explain that in the commit message or create a separate patch for that in case someone is looking for this in the future. If this was a mistake or you will add another patch for it, then ACK with this hunk removed.
virBitmapParse("4,9,10,16-19", 0, &b2, 20) < 0 || virBitmapParse("15", 0, &b3, 20) < 0 || virBitmapParse("0,^0", 0, &b4, 20) < 0) -- 2.7.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Most the callers pass 0 in one form or another, including vircapstest which used VIR_ARCH_NONE. --- src/conf/domain_conf.c | 15 +++++++-------- src/conf/network_conf.c | 2 +- src/conf/numa_conf.c | 7 +++---- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_native.c | 4 ++-- src/nodeinfo.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virbitmap.c | 10 +--------- src/util/virbitmap.h | 3 +-- src/util/virhostcpu.c | 2 +- src/vz/vz_sdk.c | 2 +- src/xenconfig/xen_common.c | 2 +- src/xenconfig/xen_sxpr.c | 3 +-- tests/qemuxml2argvtest.c | 2 +- tests/virbitmaptest.c | 22 +++++++++++----------- tests/vircapstest.c | 2 +- tools/virsh-domain.c | 2 +- 19 files changed, 38 insertions(+), 50 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 215538a..94fa7e9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13145,7 +13145,7 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, goto cleanup; if ((nodemask = virXPathString("string(./nodemask)", ctxt))) { - if (virBitmapParse(nodemask, 0, &def->sourceNodes, + if (virBitmapParse(nodemask, &def->sourceNodes, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; @@ -14871,7 +14871,7 @@ virDomainVcpuPinDefParseXML(virDomainDefPtr def, goto cleanup; } - if (virBitmapParse(tmp, 0, &vcpu->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(tmp, &vcpu->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; if (virBitmapIsAllClear(vcpu->cpumask)) { @@ -14938,7 +14938,7 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node, goto cleanup; } - if (virBitmapParse(tmp, 0, &cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(tmp, &cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; if (virBitmapIsAllClear(cpumask)) { @@ -14983,7 +14983,7 @@ virDomainEmulatorPinDefParseXML(xmlNodePtr node) return NULL; } - if (virBitmapParse(tmp, 0, &def, VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(tmp, &def, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; if (virBitmapIsAllClear(def)) { @@ -15146,7 +15146,7 @@ virDomainHugepagesParseXML(xmlNodePtr node, } if ((nodeset = virXMLPropString(node, "nodeset"))) { - if (virBitmapParse(nodeset, 0, &hugepage->nodemask, + if (virBitmapParse(nodeset, &hugepage->nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; @@ -15276,7 +15276,7 @@ virDomainSchedulerParse(xmlNodePtr node, goto error; } - if (virBitmapParse(tmp, 0, &ret, VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(tmp, &ret, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; if (virBitmapIsAllClear(ret)) { @@ -15453,8 +15453,7 @@ virDomainVcpuParse(virDomainDefPtr def, if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { tmp = virXPathString("string(./vcpu[1]/@cpuset)", ctxt); if (tmp) { - if (virBitmapParse(tmp, 0, &def->cpumask, - VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(tmp, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; if (virBitmapIsAllClear(def->cpumask)) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 48f39c7..02b8cd7 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3097,7 +3097,7 @@ virNetworkLoadState(virNetworkObjListPtr nets, ctxt->node = node; if ((class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt))) { - if (virBitmapParse(class_id, 0, &class_id_map, + if (virBitmapParse(class_id, &class_id_map, CLASS_ID_BITMAP_SIZE) < 0) { VIR_FREE(class_id); goto error; diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index e0d5688..9818d95 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -175,8 +175,7 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr numa, "in memnode element")); goto cleanup; } - if (virBitmapParse(tmp, 0, &mem_node->nodeset, - VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(tmp, &mem_node->nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; if (virBitmapIsAllClear(mem_node->nodeset)) { @@ -241,7 +240,7 @@ virDomainNumatuneParseXML(virDomainNumaPtr numa, tmp = virXMLPropString(node, "nodeset"); if (tmp) { - if (virBitmapParse(tmp, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(tmp, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; if (virBitmapIsAllClear(nodeset)) { @@ -748,7 +747,7 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def, goto cleanup; } - if (virBitmapParse(tmp, 0, &def->mem_nodes[cur_cell].cpumask, + if (virBitmapParse(tmp, &def->mem_nodes[cur_cell].cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 32ea906..2aee41c 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -784,7 +784,7 @@ static int virLXCControllerGetNumadAdvice(virLXCControllerPtr ctrl, VIR_DEBUG("Nodeset returned from numad: %s", nodeset); - if (virBitmapParse(nodeset, 0, &nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(nodeset, &nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; } diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 5f9e50e..7f8e904 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -833,7 +833,7 @@ lxcSetCpusetTune(virDomainDefPtr def, virConfPtr properties) if ((value = virConfGetValue(properties, "lxc.cgroup.cpuset.cpus")) && value->str) { - if (virBitmapParse(value->str, 0, &def->cpumask, + if (virBitmapParse(value->str, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) return -1; @@ -842,7 +842,7 @@ lxcSetCpusetTune(virDomainDefPtr def, virConfPtr properties) if ((value = virConfGetValue(properties, "lxc.cgroup.cpuset.mems")) && value->str) { - if (virBitmapParse(value->str, 0, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(value->str, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) return -1; if (virDomainNumatuneSet(def->numa, def->placement_mode == diff --git a/src/nodeinfo.c b/src/nodeinfo.c index b02c294..f2ded02 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -121,7 +121,7 @@ virNodeGetSiblingsListLinux(const char *dir, int cpu_id) if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGTH_MAX, &buf) < 0) goto cleanup; - if (virBitmapParse(buf, 0, &ret, virNumaGetMaxCPUs()) < 0) + if (virBitmapParse(buf, &ret, virNumaGetMaxCPUs()) < 0) goto cleanup; cleanup: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c844566..7fcc33a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1594,7 +1594,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, goto error; if ((tmp = virXPathString("string(./numad/@nodeset)", ctxt))) { - if (virBitmapParse(tmp, 0, &priv->autoNodeset, + if (virBitmapParse(tmp, &priv->autoNodeset, caps->host.nnumaCell_max) < 0) goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e918de5..8195ec1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9540,7 +9540,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { - if (virBitmapParse(param->value.s, 0, &nodeset, + if (virBitmapParse(param->value.s, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 01d607d..acb76b9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4860,7 +4860,7 @@ qemuProcessPrepareDomain(virConnectPtr conn, VIR_DEBUG("Nodeset returned from numad: %s", nodeset); - if (virBitmapParse(nodeset, 0, &priv->autoNodeset, + if (virBitmapParse(nodeset, &priv->autoNodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 139e3ba..3381a3d 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -522,7 +522,6 @@ virBitmapParseSeparator(const char *str, /** * virBitmapParse: * @str: points to a string representing a human-readable bitmap - * @terminator: character separating the bitmap to parse * @bitmap: a bitmap created from @str * @bitmapSize: the upper limit of num of bits in created bitmap * @@ -533,21 +532,14 @@ virBitmapParseSeparator(const char *str, * to set, and ^N, which means to unset the bit, and N-M for ranges of bits * to set. * - * To allow parsing of bitmaps within larger strings it is possible to set - * a termination character in the argument @terminator. When the character - * in @terminator is encountered in @str, the parsing of the bitmap stops. - * Pass 0 as @terminator if it is not needed. Whitespace characters may not - * be used as terminators. - * * Returns 0 on success, or -1 in case of error. */ int virBitmapParse(const char *str, - char terminator, virBitmapPtr *bitmap, size_t bitmapSize) { - return virBitmapParseSeparator(str, terminator, bitmap, bitmapSize); + return virBitmapParseSeparator(str, '\0', bitmap, bitmapSize); } /** diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 79922ea..3356e1e 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -86,10 +86,9 @@ char *virBitmapString(virBitmapPtr bitmap) char *virBitmapFormat(virBitmapPtr bitmap); int virBitmapParse(const char *str, - char sep, virBitmapPtr *bitmap, size_t bitmapSize) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virBitmapParseSeparator(const char *str, char terminator, diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index a481be0..4ed7b21 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -947,7 +947,7 @@ virHostCPUParseMapLinux(int max_cpuid, const char *path) if (virFileReadAll(path, 5 * VIR_HOST_CPU_MASK_LEN, &str) < 0) goto error; - if (virBitmapParse(str, 0, &map, max_cpuid) < 0) + if (virBitmapParse(str, &map, max_cpuid) < 0) goto error; VIR_FREE(str); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 5f89079..221f6df 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1173,7 +1173,7 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom, goto cleanup; virBitmapSetAll(def->cpumask); } else { - if (virBitmapParse(buf, 0, &def->cpumask, hostcpus) < 0) + if (virBitmapParse(buf, &def->cpumask, hostcpus) < 0) goto cleanup; } diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index a568666..a6eda2b 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -509,7 +509,7 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) if (xenConfigGetString(conf, "cpus", &str, NULL) < 0) return -1; - if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0)) + if (str && (virBitmapParse(str, &def->cpumask, 4096) < 0)) return -1; if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 6c7b2c7..d08c831 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1222,8 +1222,7 @@ xenParseSxpr(const struct sexpr *root, def->mem.cur_balloon = virDomainDefGetMemoryActual(def); if (cpus != NULL) { - if (virBitmapParse(cpus, 0, &def->cpumask, - VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(cpus, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) goto error; if (virBitmapIsAllClear(def->cpumask)) { diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c406b64..41e0ac4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -282,7 +282,7 @@ static int testCompareXMLToArgvFiles(const char *xml, } priv = vm->privateData; - if (virBitmapParse("0-3", '\0', &priv->autoNodeset, 4) < 0) + if (virBitmapParse("0-3", &priv->autoNodeset, 4) < 0) goto out; if (!virDomainDefCheckABIStability(vm->def, vm->def)) { diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 5fa13c2..89c41d0 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -90,7 +90,7 @@ test2(const void *data ATTRIBUTE_UNUSED) int ret = -1; int size = 1025; - if (virBitmapParse(bitsString1, 0, &bitmap, size) < 0) + if (virBitmapParse(bitsString1, &bitmap, size) < 0) goto error; if (testBit(bitmap, 1, 32, true) < 0) @@ -218,7 +218,7 @@ test4(const void *data ATTRIBUTE_UNUSED) /* 2. partial set */ - if (virBitmapParse(bitsString, 0, &bitmap, size) < 0) + if (virBitmapParse(bitsString, &bitmap, size) < 0) goto error; if (!bitmap) goto error; @@ -495,19 +495,19 @@ test9(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; virBitmapPtr bitmap = NULL; - if (virBitmapParse("100000000", 0, &bitmap, 20) != -1) + if (virBitmapParse("100000000", &bitmap, 20) != -1) goto cleanup; if (bitmap) goto cleanup; - if (virBitmapParse("1-1000000000", 0, &bitmap, 20) != -1) + if (virBitmapParse("1-1000000000", &bitmap, 20) != -1) goto cleanup; if (bitmap) goto cleanup; - if (virBitmapParse("1-10^10000000000", 0, &bitmap, 20) != -1) + if (virBitmapParse("1-10^10000000000", &bitmap, 20) != -1) goto cleanup; if (bitmap) @@ -527,9 +527,9 @@ test10(const void *opaque ATTRIBUTE_UNUSED) virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL; if (virBitmapParseSeparator("0-3,5-8,11-15f16", 'f', &b1, 20) < 0 || - virBitmapParse("4,9,10,16-19", 0, &b2, 20) < 0 || - virBitmapParse("15", 0, &b3, 20) < 0 || - virBitmapParse("0,^0", 0, &b4, 20) < 0) + virBitmapParse("4,9,10,16-19", &b2, 20) < 0 || + virBitmapParse("15", &b3, 20) < 0 || + virBitmapParse("0,^0", &b4, 20) < 0) goto cleanup; if (!virBitmapIsAllClear(b4)) @@ -567,9 +567,9 @@ test11(const void *opaque) virBitmapPtr resmap = NULL; int ret = -1; - if (virBitmapParse(data->a, 0, &amap, 256) < 0 || - virBitmapParse(data->b, 0, &bmap, 256) < 0 || - virBitmapParse(data->res, 0, &resmap, 256) < 0) + if (virBitmapParse(data->a, &amap, 256) < 0 || + virBitmapParse(data->b, &bmap, 256) < 0 || + virBitmapParse(data->res, &resmap, 256) < 0) goto cleanup; virBitmapSubtract(amap, bmap); diff --git a/tests/vircapstest.c b/tests/vircapstest.c index 561c13d..27d17bd 100644 --- a/tests/vircapstest.c +++ b/tests/vircapstest.c @@ -104,7 +104,7 @@ test_virCapabilitiesGetCpusForNodemask(const void *data ATTRIBUTE_UNUSED) if (!(caps = buildNUMATopology(3))) goto error; - if (virBitmapParse(nodestr, VIR_ARCH_NONE, &nodemask, mask_size) < 0) + if (virBitmapParse(nodestr, &nodemask, mask_size) < 0) goto error; if (!(cpumap = virCapabilitiesGetCpusForNodemask(caps, nodemask))) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 02be58f..bd6db47 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6476,7 +6476,7 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen, return NULL; virBitmapSetAll(map); } else { - if ((virBitmapParse(cpulist, '\0', &map, 1024) < 0) || + if (virBitmapParse(cpulist, &map, 1024) < 0 || virBitmapIsAllClear(map)) { vshError(ctl, _("Invalid cpulist '%s'"), cpulist); goto cleanup; -- 2.7.3

On Fri, Jun 17, 2016 at 03:33:41PM +0200, Ján Tomko wrote:
Most the callers pass 0 in one form or another, including vircapstest which used VIR_ARCH_NONE. --- src/conf/domain_conf.c | 15 +++++++-------- src/conf/network_conf.c | 2 +- src/conf/numa_conf.c | 7 +++---- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_native.c | 4 ++-- src/nodeinfo.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virbitmap.c | 10 +--------- src/util/virbitmap.h | 3 +-- src/util/virhostcpu.c | 2 +- src/vz/vz_sdk.c | 2 +- src/xenconfig/xen_common.c | 2 +- src/xenconfig/xen_sxpr.c | 3 +-- tests/qemuxml2argvtest.c | 2 +- tests/virbitmaptest.c | 22 +++++++++++----------- tests/vircapstest.c | 2 +- tools/virsh-domain.c | 2 +- 19 files changed, 38 insertions(+), 50 deletions(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 5fa13c2..89c41d0 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -527,9 +527,9 @@ test10(const void *opaque ATTRIBUTE_UNUSED) virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL;
if (virBitmapParseSeparator("0-3,5-8,11-15f16", 'f', &b1, 20) < 0 ||
Of course that in case the condition from the previous commit is true, then this needs to be changed, but it's kinda obvious.
@@ -567,9 +567,9 @@ test11(const void *opaque) virBitmapPtr resmap = NULL; int ret = -1;
- if (virBitmapParse(data->a, 0, &amap, 256) < 0 || - virBitmapParse(data->b, 0, &bmap, 256) < 0 || - virBitmapParse(data->res, 0, &resmap, 256) < 0) + if (virBitmapParse(data->a, &amap, 256) < 0 || + virBitmapParse(data->b, &bmap, 256) < 0 || + virBitmapParse(data->res, &resmap, 256) < 0) goto cleanup;
virBitmapSubtract(amap, bmap); diff --git a/tests/vircapstest.c b/tests/vircapstest.c index 561c13d..27d17bd 100644 --- a/tests/vircapstest.c +++ b/tests/vircapstest.c @@ -104,7 +104,7 @@ test_virCapabilitiesGetCpusForNodemask(const void *data ATTRIBUTE_UNUSED) if (!(caps = buildNUMATopology(3))) goto error;
- if (virBitmapParse(nodestr, VIR_ARCH_NONE, &nodemask, mask_size) < 0) + if (virBitmapParse(nodestr, &nodemask, mask_size) < 0)
LOL! ACK

For parsing a bitmap of an unknown size. --- src/libvirt_private.syms | 1 + src/util/virbitmap.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virbitmap.h | 4 ++ tests/virbitmaptest.c | 7 +++ 4 files changed, 124 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a88a3d1..a2f699b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1207,6 +1207,7 @@ virBitmapNextSetBit; virBitmapOverlaps; virBitmapParse; virBitmapParseSeparator; +virBitmapParseUnlimited; virBitmapSetAll; virBitmapSetBit; virBitmapSetBitExpand; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 3381a3d..3276763 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -543,6 +543,118 @@ virBitmapParse(const char *str, } /** + * virBitmapParseUnlimited: + * @str: points to a string representing a human-readable bitmap + * @bitmap: a bitmap created from @str + * + * This function is the counterpart of virBitmapFormat. This function creates + * a bitmap, in which bits are set according to the content of @str. + * + * The bitmap is expanded to accomodate all the bits. + * + * @str is a comma separated string of fields N, which means a number of bit + * to set, and ^N, which means to unset the bit, and N-M for ranges of bits + * to set. + * + * Returns 0 on success, or -1 in case of error. + */ +int +virBitmapParseUnlimited(const char *str, + virBitmapPtr *bitmap) +{ + bool neg = false; + const char *cur = str; + char *tmp; + size_t i; + int start, last; + + if (!(*bitmap = virBitmapNewEmpty())) + return -1; + + if (!str) + goto error; + + virSkipSpaces(&cur); + + if (*cur == '\0') + goto error; + + while (*cur != 0) { + /* + * 3 constructs are allowed: + * - N : a single CPU number + * - N-M : a range of CPU numbers with N < M + * - ^N : remove a single CPU number from the current set + */ + if (*cur == '^') { + cur++; + neg = true; + } + + if (!c_isdigit(*cur)) + goto error; + + if (virStrToLong_i(cur, &tmp, 10, &start) < 0) + goto error; + if (start < 0) + goto error; + + cur = tmp; + + virSkipSpaces(&cur); + + if (*cur == ',' || *cur == 0) { + if (neg) { + if (virBitmapClearBitExpand(*bitmap, start) < 0) + goto error; + } else { + if (virBitmapSetBitExpand(*bitmap, start) < 0) + goto error; + } + } else if (*cur == '-') { + if (neg) + goto error; + + cur++; + virSkipSpaces(&cur); + + if (virStrToLong_i(cur, &tmp, 10, &last) < 0) + goto error; + if (last < start) + goto error; + + cur = tmp; + + for (i = start; i <= last; i++) { + if (virBitmapSetBitExpand(*bitmap, i) < 0) + goto error; + } + + virSkipSpaces(&cur); + } + + if (*cur == ',') { + cur++; + virSkipSpaces(&cur); + neg = false; + } else if (*cur == 0) { + break; + } else { + goto error; + } + } + + return 0; + + error: + virReportError(VIR_ERR_INVALID_ARG, + _("Failed to parse bitmap '%s'"), str); + virBitmapFree(*bitmap); + *bitmap = NULL; + return -1; +} + +/** * virBitmapNewCopy: * @src: the source bitmap. * diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 3356e1e..5984b80 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -94,6 +94,10 @@ virBitmapParseSeparator(const char *str, char terminator, virBitmapPtr *bitmap, size_t bitmapSize); +int +virBitmapParseUnlimited(const char *str, + virBitmapPtr *bitmap) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); virBitmapPtr virBitmapNewCopy(virBitmapPtr src) ATTRIBUTE_NONNULL(1); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 89c41d0..009fa0d 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -632,12 +632,19 @@ test12(const void *opaque ATTRIBUTE_UNUSED) TEST_MAP(151, "100"); + virBitmapFree(map); + if (virBitmapParseUnlimited("34,1023", &map) < 0) + goto cleanup; + + TEST_MAP(1024, "34,1023"); + ret = 0; cleanup: virBitmapFree(map); return ret; } + #undef TEST_MAP -- 2.7.3

On Fri, Jun 17, 2016 at 03:33:42PM +0200, Ján Tomko wrote:
For parsing a bitmap of an unknown size.
This should be called virBitmapParse() in an utopian world. Pity it would be usable by read-only connections then :-/ It's also pity that now we have two functions which share all the code except two lines. But I would be against this patch if it was the first (or hundred) case only, so ACK.

Use virBitmapParseUnlimited to parse the env variable. --- tests/testutils.c | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index 54adab2..21b3bc6 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -59,6 +59,7 @@ VIR_LOG_INIT("tests.testutils"); +#include "virbitmap.h" #include "virfile.h" static unsigned int testDebug = -1; @@ -79,8 +80,7 @@ int ntestAllocStack; static bool testOOMActive; static size_t testCounter; -static size_t testStart; -static size_t testEnd; +static virBitmapPtr testBitmap; char *progname; @@ -169,9 +169,7 @@ virTestRun(const char *title, /* Skip tests if out of range */ - if ((testStart != 0) && - (testCounter < testStart || - testCounter > testEnd)) + if (testBitmap && !virBitmapIsBitSet(testBitmap, testCounter)) return 0; if (virTestGetVerbose()) @@ -914,30 +912,10 @@ int virTestMain(int argc, } if ((testRange = getenv("VIR_TEST_RANGE")) != NULL) { - char *end = NULL; - unsigned int iv; - if (virStrToLong_ui(testRange, &end, 10, &iv) < 0) { + if (virBitmapParseUnlimited(testRange, &testBitmap) < 0) { fprintf(stderr, "Cannot parse range %s\n", testRange); return EXIT_FAILURE; } - testStart = testEnd = iv; - if (end && *end) { - if (*end != '-') { - fprintf(stderr, "Cannot parse range %s\n", testRange); - return EXIT_FAILURE; - } - end++; - if (virStrToLong_ui(end, NULL, 10, &iv) < 0) { - fprintf(stderr, "Cannot parse range %s\n", testRange); - return EXIT_FAILURE; - } - testEnd = iv; - - if (testEnd < testStart) { - fprintf(stderr, "Test range end %zu must be >= %zu\n", testEnd, testStart); - return EXIT_FAILURE; - } - } } #ifdef TEST_OOM -- 2.7.3

On Fri, Jun 17, 2016 at 03:33:43PM +0200, Ján Tomko wrote:
Use virBitmapParseUnlimited to parse the env variable. --- tests/testutils.c | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-)
ACK, nice diffstat for a feature ;)
participants (2)
-
Ján Tomko
-
Martin Kletzander