[libvirt] [PATCH v2 0/9] improve virBitmap

In many places we store bitmap info in a chunk of data (pointed to by a char *), and have redundant codes to set/unset bits. This series extends virBitmap, and convert those codes to use virBitmap. changes: v2: - fix bug in qemuSetupCgroupForEmulator - new function virBitmapNextSetBit - virBitmapcmp -> virBitmapEqual - virBitmap: store bits in little endian format - some improvements of virBitmap - fix some memory leaks Hu Tao (9): fix bug in qemuSetupCgroupForEmulator New functions for virBitmap use virBitmap to store cpupin info use virBitmap to store cpu affinity info use virBitmap to store numa nodemask info. use virBitmap to store cpumask info. use virBitmap to store cells' cpumask info. use virBitmap to store nodeinfo. remove virDomainCpuSetFormat and virDomainCpuSetParse .gitignore | 1 + src/conf/cpu_conf.c | 17 +- src/conf/cpu_conf.h | 3 +- src/conf/domain_conf.c | 392 ++++++--------------------------- src/conf/domain_conf.h | 18 +- src/libvirt_private.syms | 14 +- src/lxc/lxc_controller.c | 56 ++--- src/nodeinfo.c | 26 +-- src/nodeinfo.h | 6 +- src/parallels/parallels_driver.c | 5 +- src/qemu/qemu_cgroup.c | 18 +- src/qemu/qemu_cgroup.h | 2 +- src/qemu/qemu_command.c | 43 +--- src/qemu/qemu_driver.c | 168 +++++++------- src/qemu/qemu_process.c | 141 ++++-------- src/test/test_driver.c | 5 +- src/util/bitmap.c | 451 +++++++++++++++++++++++++++++++++++++- src/util/bitmap.h | 34 +++ src/util/processinfo.c | 36 +-- src/util/processinfo.h | 9 +- src/vmx/vmx.c | 36 +-- tests/Makefile.am | 7 +- tests/cpuset | 2 +- tests/virbitmaptest.c | 233 ++++++++++++++++++++ 24 files changed, 1034 insertions(+), 689 deletions(-) create mode 100644 tests/virbitmaptest.c -- 1.7.10.2

Should not return 0 when failed to setup cgroup. --- src/qemu/qemu_cgroup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 5b42793..c95cc77 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -691,13 +691,15 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, } if (def->cputune.emulatorpin && - qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) && - qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin) < 0) + qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin); + if (rc < 0) goto cleanup; + } if (period || quota) { if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) + if ((rc = qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota)) < 0) goto cleanup; } } -- 1.7.10.2

On 09/06/2012 04:13 AM, Hu Tao wrote:
Should not return 0 when failed to setup cgroup. --- src/qemu/qemu_cgroup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
ACK with one nit.
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 5b42793..c95cc77 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -691,13 +691,15 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, }
if (def->cputune.emulatorpin && - qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) && - qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin) < 0) + qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin); + if (rc < 0) goto cleanup;
Indentation is now off. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/06/2012 10:56 PM, Eric Blake wrote:
On 09/06/2012 04:13 AM, Hu Tao wrote:
Should not return 0 when failed to setup cgroup. --- src/qemu/qemu_cgroup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
ACK with one nit.
+ if (rc < 0) goto cleanup;
Indentation is now off.
Pushed with this fixed. Awaiting v3 for the rest of the series, but do make sure that you are aware of the interface that Dan has proposed and reconcile according to my suggestions: https://www.redhat.com/archives/libvir-list/2012-September/msg00698.html -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

In many places we store bitmap info in a chunk of data (pointed to by a char *), and have redundant codes to set/unset bits. This patch extends virBitmap, and convert those codes to use virBitmap in subsequent patches. --- .gitignore | 1 + src/libvirt_private.syms | 11 ++ src/util/bitmap.c | 451 +++++++++++++++++++++++++++++++++++++++++++++- src/util/bitmap.h | 34 ++++ tests/Makefile.am | 7 +- tests/virbitmaptest.c | 233 ++++++++++++++++++++++++ 6 files changed, 731 insertions(+), 6 deletions(-) create mode 100644 tests/virbitmaptest.c diff --git a/.gitignore b/.gitignore index 5041ddf..4851a83 100644 --- a/.gitignore +++ b/.gitignore @@ -156,6 +156,7 @@ /tests/utiltest /tests/viratomictest /tests/virauthconfigtest +/tests/virbitmaptest /tests/virbuftest /tests/virdrivermoduletest /tests/virhashtest diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 65067d6..028b65c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -7,11 +7,22 @@ # bitmap.h virBitmapAlloc; +virBitmapAllocFromData; +virBitmapClearAll; virBitmapClearBit; +virBitmapCopy; +virBitmapEqual; +virBitmapFormat; virBitmapFree; virBitmapGetBit; +virBitmapIsAllSet; +virBitmapNextSetBit; +virBitmapParse; +virBitmapSetAll; virBitmapSetBit; +virBitmapSize; virBitmapString; +virBitmapToData; # buf.h diff --git a/src/util/bitmap.c b/src/util/bitmap.c index 53a8a38..2ca417a 100644 --- a/src/util/bitmap.c +++ b/src/util/bitmap.c @@ -33,11 +33,14 @@ #include "bitmap.h" #include "memory.h" #include "buf.h" +#include "util.h" +#include "c-ctype.h" struct _virBitmap { - size_t size; - unsigned long *map; + size_t size; /* size in bits */ + size_t size2; /* size in LONGs */ + unsigned long *map; /* bits are stored in little-endian format */ }; @@ -76,6 +79,7 @@ virBitmapPtr virBitmapAlloc(size_t size) } bitmap->size = size; + bitmap->size2 = sz; return bitmap; } @@ -129,6 +133,12 @@ int virBitmapClearBit(virBitmapPtr bitmap, size_t b) return 0; } +/* Helper function. caller must ensure b < bitmap->size */ +static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) +{ + return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b)); +} + /** * virBitmapGetBit: * @bitmap: Pointer to bitmap @@ -145,7 +155,7 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) if (bitmap->size <= b) return -1; - *result = !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b)); + *result = virBitmapIsSet(bitmap, b); return 0; } @@ -164,8 +174,7 @@ char *virBitmapString(virBitmapPtr bitmap) virBufferAddLit(&buf, "0x"); - sz = (bitmap->size + VIR_BITMAP_BITS_PER_UNIT - 1) / - VIR_BITMAP_BITS_PER_UNIT; + sz = bitmap->size2; while (sz--) { virBufferAsprintf(&buf, "%0*lx", @@ -180,3 +189,435 @@ char *virBitmapString(virBitmapPtr bitmap) return virBufferContentAndReset(&buf); } + +/** + * virBitmapFormat: + * @bitmap: the bitmap + * + * This function is the counterpart of virBitmapParse. This function creates + * a human-readable string representing the bits in bitmap. + * + * See virBitmapParse for the format of @str. + * + * Returns the string on success or NULL otherwise. Caller should call + * VIR_FREE to free the string. + */ +char *virBitmapFormat(virBitmapPtr bitmap) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + int first = -1; + int start, cur; + int ret; + bool isset; + + if (!bitmap) + return NULL; + + cur = 0; + start = -1; + while (cur < bitmap->size) { + ret = virBitmapGetBit(bitmap, cur, &isset); + if (ret != 0) + goto error; + else if (isset) { + if (start == -1) + start = cur; + } else if (start != -1) { + if (!first) + virBufferAddLit(&buf, ","); + else + first = 0; + if (cur == start + 1) + virBufferAsprintf(&buf, "%d", start); + else + virBufferAsprintf(&buf, "%d-%d", start, cur - 1); + start = -1; + } + cur++; + } + + if (start != -1) { + if (!first) + virBufferAddLit(&buf, ","); + if (cur == start + 1) + virBufferAsprintf(&buf, "%d", start); + else + virBufferAsprintf(&buf, "%d-%d", start, cur - 1); + } + + if (virBufferError(&buf)) { +error: + virBufferFreeAndReset(&buf); + return NULL; + } + + return virBufferContentAndReset(&buf); +} + +/** + * virBitmapParse: + * @str: points to a string representing a human-readable bitmap + * @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. + * + * Returns the number of bits set in @bitmap, or -1 in case of error. + */ + +int virBitmapParse(const char *str, + char sep, + virBitmapPtr *bitmap, + size_t bitmapSize) +{ + int ret = 0; + bool neg = false; + const char *cur; + char *tmp; + int i, start, last; + + if (!str) + return -1; + + cur = str; + virSkipSpaces(&cur); + + if (*cur == 0) + return -1; + + *bitmap = virBitmapAlloc(bitmapSize); + if (!*bitmap) + return -1; + + while (*cur != 0 && *cur != sep) { + /* + * 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 parse_error; + + if (virStrToLong_i(cur, &tmp, 10, &start) < 0) + goto parse_error; + if (start < 0) + goto parse_error; + + cur = tmp; + + virSkipSpaces(&cur); + + if (*cur == ',' || *cur == 0 || *cur == sep) { + if (neg) { + if (virBitmapIsSet(*bitmap, start)) { + ignore_value(virBitmapClearBit(*bitmap, start)); + ret--; + } + } else { + if (!virBitmapIsSet(*bitmap, start)) { + ignore_value(virBitmapSetBit(*bitmap, start)); + ret++; + } + } + } else if (*cur == '-') { + if (neg) + goto parse_error; + + cur++; + virSkipSpaces(&cur); + + if (virStrToLong_i(cur, &tmp, 10, &last) < 0) + goto parse_error; + if (last < start) + goto parse_error; + + cur = tmp; + + for (i = start; i <= last; i++) { + if (!virBitmapIsSet(*bitmap, i)) { + ignore_value(virBitmapSetBit(*bitmap, i)); + ret++; + } + } + + virSkipSpaces(&cur); + } + + if (*cur == ',') { + cur++; + virSkipSpaces(&cur); + neg = false; + } else if(*cur == 0 || *cur == sep) { + break; + } else { + goto parse_error; + } + } + + return ret; + +parse_error: + virBitmapFree(*bitmap); + *bitmap = NULL; + return -1; +} + +/** + * virBitmapCopy: + * @src: the source bitmap. + * + * Makes a copy of bitmap @src. + * + * returns the copied bitmap on success, or NULL otherwise. Caller + * should call virBitmapFree to free the returned bitmap. + */ +virBitmapPtr virBitmapCopy(virBitmapPtr src) +{ + virBitmapPtr dst; + + if (VIR_ALLOC(dst) < 0) + return NULL; + + if (VIR_ALLOC_N(dst->map, src->size2) < 0) { + VIR_FREE(dst); + return NULL; + } + + memcpy(dst->map, src->map, src->size2); + dst->size = src->size; + dst->size = src->size2; + + return dst; +} + +#ifdef __BIG_ENDIAN__ +static unsigned long +virSwapEndian(unsigned long l) +{ + if (sizeof(long) == 8) + return ((l & 0x00000000000000ffULL) << 56) + | ((l & 0x000000000000ff00ULL) << 40) + | ((l & 0x0000000000ff0000ULL) << 24) + | ((l & 0x00000000ff000000ULL) << 8) + | ((l & 0x000000ff00000000ULL) >> 8) + | ((l & 0x0000ff0000000000ULL) >> 24) + | ((l & 0x00ff000000000000ULL) >> 40) + | ((l & 0xff00000000000000ULL) >> 56); + else + return ((l & 0x000000ffUL) << 24) + | ((l & 0x0000ff00UL) << 8) + | ((l & 0x00ff0000UL) >> 8) + | ((l & 0xff000000UL) >> 24); +} +#endif + +/** + * virBitmapAllocFromData: + * @data: the data + * @len: length of @data in bytes + * + * Allocate a bitmap from a chunk of data containing bits + * information + * + * Returns a pointer to the allocated bitmap or NULL if + * memory cannot be allocated. + */ +virBitmapPtr virBitmapAllocFromData(void *data, int len) +{ + virBitmapPtr bitmap; +#ifdef __BIG_ENDIAN__ + int i; +#endif + + bitmap = virBitmapAlloc(len * CHAR_BIT); + if (!bitmap) + return NULL; + + memcpy(bitmap->map, data, len); +#ifdef __BIG_ENDIAN__ + for (i = 0; i < bitmap->size2; i++) + bitmap->map[i] = virSwapEndian(bitmap->map[i]); +#endif + + return bitmap; +} + +/** + * virBitmapToData: + * @data: the data + * @len: len of @data in byte + * + * Convert a bitmap to a chunk of data containing bits information. + * Data consists of sequential bytes, with lower bytes containing + * lower bits. + * + * Returns 0 on success, -1 otherwise. + */ +int virBitmapToData(virBitmapPtr bitmap, char **data, int *dataLen) +{ + int len; +#ifdef __BIG_ENDIAN__ + unsigned long *l; +#endif + + len = bitmap->size2 * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT); + + if (VIR_ALLOC_N(*data, len) < 0) + return -1; + + memcpy(*data, bitmap->map, len); + *dataLen = len; + +#ifdef __BIG_ENDIAN__ + l = (unsigned long *)*data; + for (i = 0; i < bitmap->size2; i++, l++) + *l = virSwapEndian(*l); +#endif + + return 0; +} + +/** + * virBitmapEqual: + * @b1: bitmap 1 + * @b2: bitmap 2 + * + * Compares two bitmaps, whose lengths can be different from each other. + * + * Returns true if two bitmaps have exactly the same set of bits set, + * otherwise false. + */ +bool virBitmapEqual(virBitmapPtr b1, virBitmapPtr b2) +{ + virBitmapPtr tmp; + int i; + + if (b1->size > b2->size) { + tmp = b1; + b1 = b2; + b2 = tmp; + } + + /* Now b1 is the smaller one, if not equal */ + + for (i = 0; i < b1->size2; i++) { + if (b1->map[i] != b2->map[i]) + return false; + } + + for (; i < b2->size2; i++) { + if (b2->map[i]) + return false; + } + + return true; +} + +size_t virBitmapSize(virBitmapPtr bitmap) +{ + return bitmap->size; +} + +/** + * virBitmapSetAll: + * @bitmap: the bitmap + * + * set all bits in @bitmap. + */ +void virBitmapSetAll(virBitmapPtr bitmap) +{ + memset(bitmap->map, 0xff, + bitmap->size2 * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT)); +} + +/** + * virBitmapClearAll: + * @bitmap: the bitmap + * + * clear all bits in @bitmap. + */ +void virBitmapClearAll(virBitmapPtr bitmap) +{ + memset(bitmap->map, 0, + bitmap->size2 * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT)); +} + +/** + * virBitmapIsAllSet: + * @bitmap: the bitmap to check + * + * check if all bits in @bitmap are set. + */ +bool virBitmapIsAllSet(virBitmapPtr bitmap) +{ + int i; + int unusedBits; + size_t sz; + + unusedBits = bitmap->size2 * VIR_BITMAP_BITS_PER_UNIT - bitmap->size; + + sz = bitmap->size2; + if (unusedBits > 0) + sz--; + + for (i = 0; i < sz; i++) + if (bitmap->map[i] != -1) + return false; + + if (unusedBits > 0) { + if ((bitmap->map[sz] & ((1U << (VIR_BITMAP_BITS_PER_UNIT - unusedBits)) - 1)) + != ((1U << (VIR_BITMAP_BITS_PER_UNIT - unusedBits)) - 1)) + return false; + } + + return true; +} + +/** + * virBitmapNextSetBit: + * @bitmap: the bitmap + * @i: the position after which to search for a set bit + * + * search the first set bit after position $i in bitmap @bitmap. + * + * returns the position of the found bit, or -1 if no bit found. + */ +int virBitmapNextSetBit(virBitmapPtr bitmap, int pos) +{ + int nl; + int nb; + unsigned long bits; + + if (pos < 0) + pos = -1; + + pos++; + + if (pos >= bitmap->size) + return -1; + + nl = pos / VIR_BITMAP_BITS_PER_UNIT; + nb = pos % VIR_BITMAP_BITS_PER_UNIT; + + bits = bitmap->map[nl] & ~((1UL << nb) - 1); + + while (bits == 0 && ++nl < bitmap->size2) { + bits = bitmap->map[nl]; + } + + if (bits == 0) + return -1; + + return __builtin_ctzl(bits) + nl * VIR_BITMAP_BITS_PER_UNIT; +} diff --git a/src/util/bitmap.h b/src/util/bitmap.h index c3e6222..a39fbe8 100644 --- a/src/util/bitmap.h +++ b/src/util/bitmap.h @@ -62,4 +62,38 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) char *virBitmapString(virBitmapPtr bitmap) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +char *virBitmapFormat(virBitmapPtr bitmap) + ATTRIBUTE_NONNULL(1); + +int virBitmapParse(const char *str, + char sep, + virBitmapPtr *bitmap, + size_t bitmapSize) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + +virBitmapPtr virBitmapCopy(virBitmapPtr src) ATTRIBUTE_NONNULL(1); + +virBitmapPtr virBitmapAllocFromData(void *data, int len) ATTRIBUTE_NONNULL(1); + +int virBitmapToData(virBitmapPtr bitmap, char **data, int *dataLen) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +bool virBitmapEqual(virBitmapPtr b1, virBitmapPtr b2) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +size_t virBitmapSize(virBitmapPtr bitmap) + ATTRIBUTE_NONNULL(1); + +void virBitmapSetAll(virBitmapPtr bitmap) + ATTRIBUTE_NONNULL(1); + +void virBitmapClearAll(virBitmapPtr bitmap) + ATTRIBUTE_NONNULL(1); + +bool virBitmapIsAllSet(virBitmapPtr bitmap) + ATTRIBUTE_NONNULL(1); + +int virBitmapNextSetBit(virBitmapPtr bitmap, int pos) + ATTRIBUTE_NONNULL(1); + #endif diff --git a/tests/Makefile.am b/tests/Makefile.am index 8cf8015..ed52cc8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -92,7 +92,8 @@ test_programs = virshtest sockettest \ viratomictest \ utiltest virnettlscontexttest shunloadtest \ virtimetest viruritest virkeyfiletest \ - virauthconfigtest + virauthconfigtest \ + virbitmaptest if WITH_SECDRIVER_SELINUX test_programs += securityselinuxtest @@ -562,6 +563,10 @@ viratomictest_SOURCES = \ viratomictest.c testutils.h testutils.c viratomictest_LDADD = $(LDADDS) +virbitmaptest_SOURCES = \ + virbitmaptest.c testutils.h testutils.c +virbitmaptest_LDADD = $(LDADDS) + jsontest_SOURCES = \ jsontest.c testutils.h testutils.c jsontest_LDADD = $(LDADDS) diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c new file mode 100644 index 0000000..f5c79c8 --- /dev/null +++ b/tests/virbitmaptest.c @@ -0,0 +1,233 @@ +/* + * Copyright (C) 2012 Fujitsu. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include <time.h> +#include <sched.h> + +#include "testutils.h" + +#include "bitmap.h" + +static int test1(const void *data ATTRIBUTE_UNUSED) +{ + virBitmapPtr bitmap; + int size; + int bit; + bool result; + + size = 1024; + bit = 100; + bitmap = virBitmapAlloc(size); + if (virBitmapSetBit(bitmap, bit) < 0) + return -1; + + if (virBitmapGetBit(bitmap, bit, &result) < 0) + return -1; + + if (!result) + return -1; + + if (virBitmapGetBit(bitmap, bit + 1, &result) < 0) + return -1; + + if (result) + return -1; + + return 0; +} + +int testBit(virBitmapPtr bitmap, + unsigned int start, + unsigned int end, + bool expected) +{ + int i; + bool result; + + for (i = start; i <= end; i++) { + if (virBitmapGetBit(bitmap, i, &result) < 0) + return -1; + if (result == expected) + return 0; + } + + return -1; +} + +static int test2(const void *data ATTRIBUTE_UNUSED) +{ + const char *bitsString1 = "1-32,50,88-99,1021-1023"; + char *bitsString2 = NULL; + virBitmapPtr bitmap = NULL; + int ret = -1; + int size = 1025; + + if (virBitmapParse(bitsString1, 0, &bitmap, size) < 0) + goto error; + + if (testBit(bitmap, 1, 32, true) < 0) + goto error; + if (testBit(bitmap, 50, 50, true) < 0) + goto error; + if (testBit(bitmap, 88, 99, true) < 0) + goto error; + if (testBit(bitmap, 1021, 1023, true) < 0) + goto error; + + if (testBit(bitmap, 0, 0, false) < 0) + goto error; + if (testBit(bitmap, 33, 49, false) < 0) + goto error; + if (testBit(bitmap, 51, 87, false) < 0) + goto error; + if (testBit(bitmap, 100, 1020, false) < 0) + goto error; + + bitsString2 = virBitmapFormat(bitmap); + if (strcmp(bitsString1, bitsString2)) + goto error; + + virBitmapSetAll(bitmap); + if (testBit(bitmap, 0, size - 1, true) < 0) + goto error; + + if (!virBitmapIsAllSet(bitmap)) + goto error; + + virBitmapClearAll(bitmap); + if (testBit(bitmap, 0, size - 1, false) < 0) + goto error; + + ret = 0; + +error: + virBitmapFree(bitmap); + VIR_FREE(bitsString2); + return ret; +} + +static int test3(const void *data ATTRIBUTE_UNUSED) +{ + virBitmapPtr bitmap = NULL; + int ret = -1; + int size = 5; + int i; + + if ((bitmap = virBitmapAlloc(size)) == NULL) + goto error; + + for (i = 0; i < size; i++) + ignore_value(virBitmapSetBit(bitmap, i)); + + if (!virBitmapIsAllSet(bitmap)) + goto error; + + ret = 0; + +error: + virBitmapFree(bitmap); + return ret; +} + +static int test4(const void *data ATTRIBUTE_UNUSED) +{ + const char *bitsString = "0, 2-4, 6-10, 12, 14-18, 20, 22, 25"; + int size = 40; + int bitsPos[] = { + 0, 2, 3, 4, 6, 7, 8, 9, 10, 12, + 14, 15, 16, 17, 18, 20, 22, 25 + }; + int npos = 18; + virBitmapPtr bitmap = NULL; + int i, j; + + /* 1. zero set */ + + bitmap = virBitmapAlloc(size); + if (!bitmap) + goto error; + + if (virBitmapNextSetBit(bitmap, -1) >= 0) + goto error; + + virBitmapFree(bitmap); + bitmap = NULL; + + /* 2. partial set */ + + if (virBitmapParse(bitsString, 0, &bitmap, size) < 0) + goto error; + if (!bitmap) + goto error; + + j = 0; + i = -1; + + while (j < npos) { + i = virBitmapNextSetBit(bitmap, i); + if (i != bitsPos[j++]) + goto error; + } + + if (virBitmapNextSetBit(bitmap, i) > 0) + goto error; + + /* 3. full set */ + + i = -1; + virBitmapSetAll(bitmap); + + for (j = 0; j < size; j++) { + i = virBitmapNextSetBit(bitmap, i); + if (i != j) + goto error; + } + + if (virBitmapNextSetBit(bitmap, i) > 0) + goto error; + + virBitmapFree(bitmap); + return 0; + +error: + virBitmapFree(bitmap); + return -1; +} + +static int +mymain(void) +{ + int ret = 0; + + if (virtTestRun("test1", 1, test1, NULL) < 0) + ret = -1; + if (virtTestRun("test2", 1, test2, NULL) < 0) + ret = -1; + if (virtTestRun("test3", 1, test3, NULL) < 0) + ret = -1; + if (virtTestRun("test4", 1, test4, NULL) < 0) + ret = -1; + + + return ret; +} + +VIRT_TEST_MAIN(mymain) -- 1.7.10.2

On 09/06/2012 04:13 AM, Hu Tao wrote:
In many places we store bitmap info in a chunk of data (pointed to by a char *), and have redundant codes to set/unset bits. This patch extends virBitmap, and convert those codes to use virBitmap in subsequent patches. ---
struct _virBitmap { - size_t size; - unsigned long *map; + size_t size; /* size in bits */ + size_t size2; /* size in LONGs */
The name 'size2' isn't very descriptive. Maybe we should rename to s/size/max_bit/ and s/size2/map_len/ for easier reading?
+ unsigned long *map; /* bits are stored in little-endian format */
This comment...
+/* Helper function. caller must ensure b < bitmap->size */ +static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) +{ + return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b));
...and this code disagree. This code is reading in machine-native format, not little-endian. And I much prefer operating in machine-native format. Which means when converting from char* to long, you'll have to properly pass things through endian conversion, rather than requiring the longs to be little-endian.
+char *virBitmapFormat(virBitmapPtr bitmap) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + int first = -1; + int start, cur; + int ret; + bool isset; + + if (!bitmap) + return NULL; + + cur = 0; + start = -1; + while (cur < bitmap->size) { + ret = virBitmapGetBit(bitmap, cur, &isset);
I'm wondering if we should optimize this by using things like ffsl() and iterating a long at a time for longs that are 0 or -1, rather than blindly processing one bit at a time. Or even make this use the new virBitmapNextSetBit, and have that function be optimized a bit more.
+ if (ret != 0) + goto error; + else if (isset) {
Style. Since the else branch used {}, the if branch must also use {}.
+ if (start == -1) + start = cur; + } else if (start != -1) { + if (!first) + virBufferAddLit(&buf, ","); + else + first = 0; + if (cur == start + 1) + virBufferAsprintf(&buf, "%d", start); + else + virBufferAsprintf(&buf, "%d-%d", start, cur - 1); + start = -1; + } + cur++; + } + + if (start != -1) { + if (!first) + virBufferAddLit(&buf, ","); + if (cur == start + 1) + virBufferAsprintf(&buf, "%d", start); + else + virBufferAsprintf(&buf, "%d-%d", start, cur - 1); + } + + if (virBufferError(&buf)) { +error: + virBufferFreeAndReset(&buf); + return NULL; + } + + return virBufferContentAndReset(&buf);
Ouch. If the bitset is completely unset, then this returns NULL for both errors and success. You need to special-case a map that is completely unset to return strdup("") instead.
+ +#ifdef __BIG_ENDIAN__ +static unsigned long +virSwapEndian(unsigned long l)
Yuck. __BIG_ENDIAN__ vs. __LITTLE_ENDIAN__ is not guaranteed to exist. And even if you can rely on it, there's bound to be better ways to implement this instead of open-coding it ourselves (not to mention that by avoiding the #ifdefs, we avoid introducing bugs in the big-endian code that cannot be detected on little-endian machines). (Hmm, too bad gnulib doesn't guarantee be32toh and be64toh).
+/** + * virBitmapAllocFromData: + * @data: the data + * @len: length of @data in bytes + * + * Allocate a bitmap from a chunk of data containing bits + * information + * + * Returns a pointer to the allocated bitmap or NULL if + * memory cannot be allocated. + */ +virBitmapPtr virBitmapAllocFromData(void *data, int len) +{ + virBitmapPtr bitmap; +#ifdef __BIG_ENDIAN__ + int i; +#endif + + bitmap = virBitmapAlloc(len * CHAR_BIT); + if (!bitmap) + return NULL; + + memcpy(bitmap->map, data, len);
Instead of trying to memcpy() and then conditionally virSwapEndian(), I would just do it the manual way of reading one byte at a time for both endian types. Fewer #ifdefs, less ugly code.
+int virBitmapToData(virBitmapPtr bitmap, char **data, int *dataLen) +{ + int len; +#ifdef __BIG_ENDIAN__ + unsigned long *l; +#endif + + len = bitmap->size2 * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT); + + if (VIR_ALLOC_N(*data, len) < 0) + return -1; + + memcpy(*data, bitmap->map, len); + *dataLen = len; + +#ifdef __BIG_ENDIAN__ + l = (unsigned long *)*data; + for (i = 0; i < bitmap->size2; i++, l++) + *l = virSwapEndian(*l); +#endif
Again, I'm not a fan of these #ifdefs.
+int virBitmapNextSetBit(virBitmapPtr bitmap, int pos) +{ + int nl; + int nb; + unsigned long bits; + + if (pos < 0) + pos = -1; + + pos++; + + if (pos >= bitmap->size) + return -1; + + nl = pos / VIR_BITMAP_BITS_PER_UNIT; + nb = pos % VIR_BITMAP_BITS_PER_UNIT; + + bits = bitmap->map[nl] & ~((1UL << nb) - 1); + + while (bits == 0 && ++nl < bitmap->size2) { + bits = bitmap->map[nl]; + }
Use ffsl() instead of iterating one bit at a time.
+ + if (bits == 0) + return -1; + + return __builtin_ctzl(bits) + nl * VIR_BITMAP_BITS_PER_UNIT;
__builtin_ctzl() is not guaranteed to exist. ffsl() should already give you what you need.
+++ b/tests/virbitmaptest.c @@ -0,0 +1,233 @@
+#include <config.h> + +#include <time.h> +#include <sched.h>
What are you using <time.h> and <sched.h> for? It's late for me, so I didn't closely read the entire patch, so much as identified things that jumped out on first glance. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/06/2012 11:17 PM, Eric Blake wrote:
On 09/06/2012 04:13 AM, Hu Tao wrote:
In many places we store bitmap info in a chunk of data (pointed to by a char *), and have redundant codes to set/unset bits. This patch extends virBitmap, and convert those codes to use virBitmap in subsequent patches. ---
struct _virBitmap { - size_t size; - unsigned long *map; + size_t size; /* size in bits */ + size_t size2; /* size in LONGs */
The name 'size2' isn't very descriptive. Maybe we should rename to s/size/max_bit/ and s/size2/map_len/ for easier reading?
If you go with the rename idea, then I'd split this into two patches - one that introduces the new field and names and retrofits all existing functions, but adds no new code other than the use of the new field; and the second that only adds the new functions. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, Sep 06, 2012 at 11:23:49PM -0600, Eric Blake wrote:
On 09/06/2012 11:17 PM, Eric Blake wrote:
On 09/06/2012 04:13 AM, Hu Tao wrote:
In many places we store bitmap info in a chunk of data (pointed to by a char *), and have redundant codes to set/unset bits. This patch extends virBitmap, and convert those codes to use virBitmap in subsequent patches. ---
struct _virBitmap { - size_t size; - unsigned long *map; + size_t size; /* size in bits */ + size_t size2; /* size in LONGs */
The name 'size2' isn't very descriptive. Maybe we should rename to s/size/max_bit/ and s/size2/map_len/ for easier reading?
If you go with the rename idea, then I'd split this into two patches - one that introduces the new field and names and retrofits all existing functions, but adds no new code other than the use of the new field; and the second that only adds the new functions.
OK. -- Thanks, Hu Tao

On Thu, Sep 06, 2012 at 11:17:58PM -0600, Eric Blake wrote:
On 09/06/2012 04:13 AM, Hu Tao wrote:
In many places we store bitmap info in a chunk of data (pointed to by a char *), and have redundant codes to set/unset bits. This patch extends virBitmap, and convert those codes to use virBitmap in subsequent patches. ---
struct _virBitmap { - size_t size; - unsigned long *map; + size_t size; /* size in bits */ + size_t size2; /* size in LONGs */
The name 'size2' isn't very descriptive. Maybe we should rename to s/size/max_bit/ and s/size2/map_len/ for easier reading?
Thanks for suggestion.
+ unsigned long *map; /* bits are stored in little-endian format */
This comment...
+/* Helper function. caller must ensure b < bitmap->size */ +static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) +{ + return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b));
...and this code disagree. This code is reading in machine-native format, not little-endian. And I much prefer operating in machine-native format. Which means when converting from char* to long, you'll have to properly pass things through endian conversion, rather than requiring the longs to be little-endian.
OK.
+char *virBitmapFormat(virBitmapPtr bitmap) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + int first = -1; + int start, cur; + int ret; + bool isset; + + if (!bitmap) + return NULL; + + cur = 0; + start = -1; + while (cur < bitmap->size) { + ret = virBitmapGetBit(bitmap, cur, &isset);
I'm wondering if we should optimize this by using things like ffsl() and iterating a long at a time for longs that are 0 or -1, rather than blindly processing one bit at a time. Or even make this use the new virBitmapNextSetBit, and have that function be optimized a bit more.
Will improve it.
+ if (ret != 0) + goto error; + else if (isset) {
Style. Since the else branch used {}, the if branch must also use {}.
OK.
+ if (start == -1) + start = cur; + } else if (start != -1) { + if (!first) + virBufferAddLit(&buf, ","); + else + first = 0; + if (cur == start + 1) + virBufferAsprintf(&buf, "%d", start); + else + virBufferAsprintf(&buf, "%d-%d", start, cur - 1); + start = -1; + } + cur++; + } + + if (start != -1) { + if (!first) + virBufferAddLit(&buf, ","); + if (cur == start + 1) + virBufferAsprintf(&buf, "%d", start); + else + virBufferAsprintf(&buf, "%d-%d", start, cur - 1); + } + + if (virBufferError(&buf)) { +error: + virBufferFreeAndReset(&buf); + return NULL; + } + + return virBufferContentAndReset(&buf);
Ouch. If the bitset is completely unset, then this returns NULL for both errors and success. You need to special-case a map that is completely unset to return strdup("") instead.
OK.
+ +#ifdef __BIG_ENDIAN__ +static unsigned long +virSwapEndian(unsigned long l)
Yuck. __BIG_ENDIAN__ vs. __LITTLE_ENDIAN__ is not guaranteed to exist. And even if you can rely on it, there's bound to be better ways to implement this instead of open-coding it ourselves (not to mention that by avoiding the #ifdefs, we avoid introducing bugs in the big-endian code that cannot be detected on little-endian machines). (Hmm, too bad gnulib doesn't guarantee be32toh and be64toh).
+/** + * virBitmapAllocFromData: + * @data: the data + * @len: length of @data in bytes + * + * Allocate a bitmap from a chunk of data containing bits + * information + * + * Returns a pointer to the allocated bitmap or NULL if + * memory cannot be allocated. + */ +virBitmapPtr virBitmapAllocFromData(void *data, int len) +{ + virBitmapPtr bitmap; +#ifdef __BIG_ENDIAN__ + int i; +#endif + + bitmap = virBitmapAlloc(len * CHAR_BIT); + if (!bitmap) + return NULL; + + memcpy(bitmap->map, data, len);
Instead of trying to memcpy() and then conditionally virSwapEndian(), I would just do it the manual way of reading one byte at a time for both endian types. Fewer #ifdefs, less ugly code.
OK.
+int virBitmapToData(virBitmapPtr bitmap, char **data, int *dataLen) +{ + int len; +#ifdef __BIG_ENDIAN__ + unsigned long *l; +#endif + + len = bitmap->size2 * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT); + + if (VIR_ALLOC_N(*data, len) < 0) + return -1; + + memcpy(*data, bitmap->map, len); + *dataLen = len; + +#ifdef __BIG_ENDIAN__ + l = (unsigned long *)*data; + for (i = 0; i < bitmap->size2; i++, l++) + *l = virSwapEndian(*l); +#endif
Again, I'm not a fan of these #ifdefs.
+int virBitmapNextSetBit(virBitmapPtr bitmap, int pos) +{ + int nl; + int nb; + unsigned long bits; + + if (pos < 0) + pos = -1; + + pos++; + + if (pos >= bitmap->size) + return -1; + + nl = pos / VIR_BITMAP_BITS_PER_UNIT; + nb = pos % VIR_BITMAP_BITS_PER_UNIT; + + bits = bitmap->map[nl] & ~((1UL << nb) - 1); + + while (bits == 0 && ++nl < bitmap->size2) { + bits = bitmap->map[nl]; + }
Use ffsl() instead of iterating one bit at a time.
+ + if (bits == 0) + return -1; + + return __builtin_ctzl(bits) + nl * VIR_BITMAP_BITS_PER_UNIT;
__builtin_ctzl() is not guaranteed to exist. ffsl() should already give you what you need.
+++ b/tests/virbitmaptest.c @@ -0,0 +1,233 @@
+#include <config.h> + +#include <time.h> +#include <sched.h>
What are you using <time.h> and <sched.h> for?
Oops. This is a copy&paste.
It's late for me, so I didn't closely read the entire patch, so much as identified things that jumped out on first glance.
Have a good night:) -- Thanks, Hu Tao

--- src/conf/domain_conf.c | 148 +++++++++++++++------------------------------- src/conf/domain_conf.h | 6 +- src/libvirt_private.syms | 1 + src/qemu/qemu_cgroup.c | 3 +- src/qemu/qemu_driver.c | 29 +++++---- src/qemu/qemu_process.c | 21 ++++--- 6 files changed, 81 insertions(+), 127 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8952b69..b07b6b7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -52,6 +52,7 @@ #include "netdev_bandwidth_conf.h" #include "netdev_vlan_conf.h" #include "device_conf.h" +#include "bitmap.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -1510,10 +1511,9 @@ virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) for (i = 0; i < nvcpupin; i++) { if (VIR_ALLOC(ret[i]) < 0) goto no_memory; - if (VIR_ALLOC_N(ret[i]->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto no_memory; ret[i]->vcpuid = src[i]->vcpuid; - memcpy(ret[i]->cpumask, src[i]->cpumask, VIR_DOMAIN_CPUMASK_LEN); + if ((ret[i]->cpumask = virBitmapCopy(src[i]->cpumask)) == NULL) + goto no_memory; } return ret; @@ -1522,7 +1522,7 @@ no_memory: if (ret) { for ( ; i >= 0; --i) { if (ret[i]) { - VIR_FREE(ret[i]->cpumask); + virBitmapFree(ret[i]->cpumask); VIR_FREE(ret[i]); } } @@ -1534,8 +1534,17 @@ no_memory: } void -virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr *def, - int nvcpupin) +virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr def) +{ + if (def) { + virBitmapFree(def->cpumask); + VIR_FREE(def); + } +} + +void +virDomainVcpuPinDefArrayFree(virDomainVcpuPinDefPtr *def, + int nvcpupin) { int i; @@ -1543,8 +1552,7 @@ virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr *def, return; for(i = 0; i < nvcpupin; i++) { - VIR_FREE(def[i]->cpumask); - VIR_FREE(def[i]); + virDomainVcpuPinDefFree(def[i]); } VIR_FREE(def); @@ -1668,7 +1676,9 @@ void virDomainDefFree(virDomainDefPtr def) virCPUDefFree(def->cpu); - virDomainVcpuPinDefFree(def->cputune.vcpupin, def->cputune.nvcpupin); + virDomainVcpuPinDefArrayFree(def->cputune.vcpupin, def->cputune.nvcpupin); + + virDomainVcpuPinDefFree(def->cputune.emulatorpin); VIR_FREE(def->numatune.memory.nodemask); @@ -8025,12 +8035,8 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node, char *set = tmp; int cpumasklen = VIR_DOMAIN_CPUMASK_LEN; - if (VIR_ALLOC_N(def->cpumask, cpumasklen) < 0) { - virReportOOMError(); - goto error; - } - if (virDomainCpuSetParse(set, 0, def->cpumask, - cpumasklen) < 0) + if (virBitmapParse(set, 0, &def->cpumask, + cpumasklen) < 0) goto error; VIR_FREE(tmp); } else { @@ -8465,18 +8471,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } - if (VIR_ALLOC(def->cputune.emulatorpin) < 0) { - goto no_memory; - } + def->cputune.emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], ctxt, + def->maxvcpus, 1); - virDomainVcpuPinDefPtr emulatorpin = NULL; - emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], ctxt, - def->maxvcpus, 1); - - if (!emulatorpin) + if (!def->cputune.emulatorpin) goto error; - - def->cputune.emulatorpin = emulatorpin; } VIR_FREE(nodes); @@ -11093,34 +11092,6 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, return NULL; } -static char *bitmapFromBytemap(unsigned char *bytemap, int maplen) -{ - char *bitmap = NULL; - int i; - - if (VIR_ALLOC_N(bitmap, VIR_DOMAIN_CPUMASK_LEN) < 0) { - virReportOOMError(); - goto cleanup; - } - - /* Reset bitmap to all 0s. */ - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) - bitmap[i] = 0; - - /* Convert bitmap (bytemap) to bitmap, which is byte map? */ - for (i = 0; i < maplen; i++) { - int cur; - - for (cur = 0; cur < 8; cur++) { - if (bytemap[i] & (1 << cur)) - bitmap[i * 8 + cur] = 1; - } - } - -cleanup: - return bitmap; -} - int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, int *nvcpupin, unsigned char *cpumap, @@ -11128,20 +11099,21 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, int vcpu) { virDomainVcpuPinDefPtr vcpupin = NULL; - char *cpumask = NULL; if (!vcpupin_list) return -1; - if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL) - return -1; - vcpupin = virDomainVcpuPinFindByVcpu(*vcpupin_list, *nvcpupin, vcpu); if (vcpupin) { vcpupin->vcpuid = vcpu; - vcpupin->cpumask = cpumask; + virBitmapFree(vcpupin->cpumask); + vcpupin->cpumask = virBitmapAllocFromData(cpumap, maplen); + if (!vcpupin->cpumask) { + virReportOOMError(); + return -1; + } return 0; } @@ -11150,16 +11122,17 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, if (VIR_ALLOC(vcpupin) < 0) { virReportOOMError(); - VIR_FREE(cpumask); return -1; } vcpupin->vcpuid = vcpu; - vcpupin->cpumask = cpumask; - + vcpupin->cpumask = virBitmapAllocFromData(cpumap, maplen); + if (!vcpupin->cpumask) { + virReportOOMError(); + return -1; + } if (VIR_REALLOC_N(*vcpupin_list, *nvcpupin + 1) < 0) { virReportOOMError(); - VIR_FREE(cpumask); VIR_FREE(vcpupin); return -1; } @@ -11214,68 +11187,43 @@ virDomainEmulatorPinAdd(virDomainDefPtr def, int maplen) { virDomainVcpuPinDefPtr emulatorpin = NULL; - char *cpumask = NULL; - int i; - - if (VIR_ALLOC_N(cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) { - virReportOOMError(); - goto cleanup; - } - - /* Convert bitmap (cpumap) to cpumask, which is byte map. */ - for (i = 0; i < maplen; i++) { - int cur; - - for (cur = 0; cur < 8; cur++) { - if (cpumap[i] & (1 << cur)) - cpumask[i * 8 + cur] = 1; - } - } if (!def->cputune.emulatorpin) { /* No emulatorpin exists yet. */ if (VIR_ALLOC(emulatorpin) < 0) { virReportOOMError(); - goto cleanup; + return -1; } emulatorpin->vcpuid = -1; - emulatorpin->cpumask = cpumask; + emulatorpin->cpumask = virBitmapAllocFromData(cpumap, maplen); + if (!emulatorpin->cpumask) + return -1; + def->cputune.emulatorpin = emulatorpin; } else { /* Since there is only 1 emulatorpin for each vm, * juest replace the old one. */ - VIR_FREE(def->cputune.emulatorpin->cpumask); - def->cputune.emulatorpin->cpumask = cpumask; + virBitmapFree(def->cputune.emulatorpin->cpumask); + def->cputune.emulatorpin->cpumask = virBitmapAllocFromData(cpumap, maplen); + if (!def->cputune.emulatorpin->cpumask) + return -1; } return 0; - -cleanup: - VIR_FREE(cpumask); - return -1; } int virDomainEmulatorPinDel(virDomainDefPtr def) { - virDomainVcpuPinDefPtr emulatorpin = NULL; - - /* No emulatorpin exists yet */ if (!def->cputune.emulatorpin) { return 0; } - emulatorpin = def->cputune.emulatorpin; - - VIR_FREE(emulatorpin->cpumask); - VIR_FREE(emulatorpin); + virDomainVcpuPinDefFree(def->cputune.emulatorpin); def->cputune.emulatorpin = NULL; - if (def->cputune.emulatorpin) - return -1; - return 0; } @@ -13248,8 +13196,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->cputune.vcpupin[i]->vcpuid); char *cpumask = NULL; - cpumask = virDomainCpuSetFormat(def->cputune.vcpupin[i]->cpumask, - VIR_DOMAIN_CPUMASK_LEN); + cpumask = virBitmapFormat(def->cputune.vcpupin[i]->cpumask); if (cpumask == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -13266,8 +13213,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " <emulatorpin "); char *cpumask = NULL; - cpumask = virDomainCpuSetFormat(def->cputune.emulatorpin->cpumask, - VIR_DOMAIN_CPUMASK_LEN); + cpumask = virBitmapFormat(def->cputune.emulatorpin->cpumask); if (cpumask == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format cpuset for emulator")); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3995c2d..bb3721c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -46,6 +46,7 @@ # include "virnetdevvlan.h" # include "virobject.h" # include "device_conf.h" +# include "bitmap.h" /* forward declarations of all device types, required by * virDomainDeviceDef @@ -1538,10 +1539,11 @@ typedef struct _virDomainVcpuPinDef virDomainVcpuPinDef; typedef virDomainVcpuPinDef *virDomainVcpuPinDefPtr; struct _virDomainVcpuPinDef { int vcpuid; - char *cpumask; + virBitmapPtr cpumask; }; -void virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr *def, int nvcpupin); +void virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr def); +void virDomainVcpuPinDefArrayFree(virDomainVcpuPinDefPtr *def, int nvcpupin); virDomainVcpuPinDefPtr *virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 028b65c..d7cd356 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -524,6 +524,7 @@ virDomainTimerTickpolicyTypeToString; virDomainTimerTrackTypeFromString; virDomainTimerTrackTypeToString; virDomainVcpuPinAdd; +virDomainVcpuPinDefArrayFree; virDomainVcpuPinDefCopy; virDomainVcpuPinDefFree; virDomainVcpuPinDel; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index c95cc77..be95761 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -513,8 +513,7 @@ int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, int rc = 0; char *new_cpus = NULL; - new_cpus = virDomainCpuSetFormat(vcpupin->cpumask, - VIR_DOMAIN_CPUMASK_LEN); + new_cpus = virBitmapFormat(vcpupin->cpumask); if (!new_cpus) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to convert cpu mask")); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b12d9bc..bdff4bb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3785,7 +3785,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, if (virDomainVcpuPinAdd(&newVcpuPin, &newVcpuPinNum, cpumap, maplen, vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update vcpupin")); - virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); goto cleanup; } @@ -3818,7 +3818,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, } } else { if (vm->def->cputune.vcpupin) - virDomainVcpuPinDefFree(vm->def->cputune.vcpupin, vm->def->cputune.nvcpupin); + virDomainVcpuPinDefArrayFree(vm->def->cputune.vcpupin, vm->def->cputune.nvcpupin); vm->def->cputune.vcpupin = newVcpuPin; vm->def->cputune.nvcpupin = newVcpuPinNum; @@ -3826,7 +3826,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, } if (newVcpuPin) - virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto cleanup; @@ -3901,8 +3901,9 @@ qemudDomainGetVcpuPinInfo(virDomainPtr dom, int maxcpu, hostcpus, vcpu, pcpu; int n; virDomainVcpuPinDefPtr *vcpupin_list; - char *cpumask = NULL; + virBitmapPtr cpumask = NULL; unsigned char *cpumap; + bool pinned; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -3961,7 +3962,9 @@ qemudDomainGetVcpuPinInfo(virDomainPtr dom, cpumask = vcpupin_list[n]->cpumask; cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu); for (pcpu = 0; pcpu < maxcpu; pcpu++) { - if (cpumask[pcpu] == 0) + if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0) + goto cleanup; + if (!pinned) VIR_UNUSE_CPU(cpumap, pcpu); } } @@ -4045,7 +4048,7 @@ qemudDomainPinEmulator(virDomainPtr dom, if (virDomainVcpuPinAdd(&newVcpuPin, &newVcpuPinNum, cpumap, maplen, -1) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update vcpupin")); - virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); goto cleanup; } @@ -4083,16 +4086,13 @@ qemudDomainPinEmulator(virDomainPtr dom, goto cleanup; } } else { - if (vm->def->cputune.emulatorpin) { - VIR_FREE(vm->def->cputune.emulatorpin->cpumask); - VIR_FREE(vm->def->cputune.emulatorpin); - } + virDomainVcpuPinDefFree(vm->def->cputune.emulatorpin); vm->def->cputune.emulatorpin = newVcpuPin[0]; VIR_FREE(newVcpuPin); } if (newVcpuPin) - virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + virDomainVcpuPinDefArrayFree(newVcpuPin, newVcpuPinNum); } else { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cpu affinity is not supported")); @@ -4151,7 +4151,8 @@ qemudDomainGetEmulatorPinInfo(virDomainPtr dom, int ret = -1; int maxcpu, hostcpus, pcpu; virDomainVcpuPinDefPtr emulatorpin = NULL; - char *cpumask = NULL; + virBitmapPtr cpumask = NULL; + bool pinned; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -4200,7 +4201,9 @@ qemudDomainGetEmulatorPinInfo(virDomainPtr dom, cpumask = emulatorpin->cpumask; for (pcpu = 0; pcpu < maxcpu; pcpu++) { - if (cpumask[pcpu] == 0) + if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0) + goto cleanup; + if (!pinned) VIR_UNUSE_CPU(cpumaps, pcpu); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 93653c6..4e9aaef 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1951,10 +1951,11 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn, virDomainDefPtr def = vm->def; virNodeInfo nodeinfo; pid_t vcpupid; - unsigned char *cpumask; + virBitmapPtr cpumask; int vcpu, cpumaplen, hostcpus, maxcpu, n; unsigned char *cpumap = NULL; int ret = -1; + bool result; if (virNodeGetInfo(conn, &nodeinfo) != 0) { return -1; @@ -1986,13 +1987,12 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn, vcpu = def->cputune.vcpupin[n]->vcpuid; memset(cpumap, 0, cpumaplen); - cpumask = (unsigned char *)def->cputune.vcpupin[n]->cpumask; + cpumask = def->cputune.vcpupin[n]->cpumask; vcpupid = priv->vcpupids[vcpu]; - for (i = 0 ; i < VIR_DOMAIN_CPUMASK_LEN ; i++) { - if (cpumask[i]) - VIR_USE_CPU(cpumap, i); - } + i = -1; + while ((i = virBitmapNextSetBit(cpumask, i)) > 0) + VIR_USE_CPU(cpumap, i); if (virProcessInfoSetAffinity(vcpupid, cpumap, @@ -2015,11 +2015,12 @@ qemuProcessSetEmulatorAffinites(virConnectPtr conn, { virDomainDefPtr def = vm->def; pid_t pid = vm->pid; - unsigned char *cpumask = NULL; + virBitmapPtr cpumask = NULL; unsigned char *cpumap = NULL; virNodeInfo nodeinfo; int cpumaplen, hostcpus, maxcpu, i; int ret = -1; + bool result; if (virNodeGetInfo(conn, &nodeinfo) != 0) return -1; @@ -2039,9 +2040,11 @@ qemuProcessSetEmulatorAffinites(virConnectPtr conn, return -1; } - cpumask = (unsigned char *)def->cputune.emulatorpin->cpumask; + cpumask = def->cputune.emulatorpin->cpumask; for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { - if (cpumask[i]) + if (virBitmapGetBit(cpumask, i, &result) < 0) + goto cleanup; + if (result) VIR_USE_CPU(cpumap, i); } -- 1.7.10.2

--- src/lxc/lxc_controller.c | 23 +++++-------- src/qemu/qemu_driver.c | 62 ++++++++++++++++++++------------- src/qemu/qemu_process.c | 86 ++++++++-------------------------------------- src/util/processinfo.c | 36 ++++++++++--------- src/util/processinfo.h | 9 ++--- 5 files changed, 84 insertions(+), 132 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index e5aea11..ac8ce94 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -493,8 +493,7 @@ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) { int i, hostcpus, maxcpu = CPU_SETSIZE; virNodeInfo nodeinfo; - unsigned char *cpumap; - int cpumaplen; + virBitmapPtr cpumap; VIR_DEBUG("Setting CPU affinity"); @@ -507,37 +506,33 @@ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) if (maxcpu > hostcpus) maxcpu = hostcpus; - cpumaplen = VIR_CPU_MAPLEN(maxcpu); - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) { - virReportOOMError(); + cpumap = virBitmapAlloc(maxcpu); + if (!cpumap) return -1; - } if (ctrl->def->cpumask) { /* XXX why don't we keep 'cpumask' in the libvirt cpumap * format to start with ?!?! */ for (i = 0 ; i < maxcpu && i < ctrl->def->cpumasklen ; i++) if (ctrl->def->cpumask[i]) - VIR_USE_CPU(cpumap, i); + ignore_value(virBitmapSetBit(cpumap, i)); } else { /* You may think this is redundant, but we can't assume libvirtd * itself is running on all pCPUs, so we need to explicitly set * the spawned LXC instance to all pCPUs if no map is given in * its config file */ - for (i = 0 ; i < maxcpu ; i++) - VIR_USE_CPU(cpumap, i); + virBitmapSetAll(cpumap); } - /* We are pressuming we are running between fork/exec of LXC + /* We are presuming we are running between fork/exec of LXC * so use '0' to indicate our own process ID. No threads are * running at this point */ - if (virProcessInfoSetAffinity(0, /* Self */ - cpumap, cpumaplen, maxcpu) < 0) { - VIR_FREE(cpumap); + if (virProcessInfoSetAffinity(0 /* Self */, cpumap) < 0) { + virBitmapFree(cpumap); return -1; } - VIR_FREE(cpumap); + virBitmapFree(cpumap); return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bdff4bb..e96c3b8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -92,6 +92,7 @@ #include "virnodesuspend.h" #include "virtime.h" #include "virtypedparam.h" +#include "bitmap.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -3710,10 +3711,10 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, virNodeInfo nodeinfo; int ret = -1; qemuDomainObjPrivatePtr priv; - bool canResetting = true; + bool doReset = false; int newVcpuPinNum = 0; virDomainVcpuPinDefPtr *newVcpuPin = NULL; - int pcpu; + virBitmapPtr pcpumap = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -3749,15 +3750,16 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, maxcpu = maplen * 8; if (maxcpu > hostcpus) maxcpu = hostcpus; + + pcpumap = virBitmapAllocFromData(cpumap, maplen); + if (!pcpumap) + goto cleanup; + /* pinning to all physical cpus means resetting, * so check if we can reset setting. */ - for (pcpu = 0; pcpu < hostcpus; pcpu++) { - if ((cpumap[pcpu/8] & (1 << (pcpu % 8))) == 0) { - canResetting = false; - break; - } - } + if (virBitmapIsAllSet(pcpumap)) + doReset = true; if (flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -3800,8 +3802,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } } else { - if (virProcessInfoSetAffinity(priv->vcpupids[vcpu], - cpumap, maplen, maxcpu) < 0) { + if (virProcessInfoSetAffinity(priv->vcpupids[vcpu], pcpumap) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to set cpu affinity for vcpu %d"), vcpu); @@ -3809,7 +3810,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, } } - if (canResetting) { + if (doReset) { if (virDomainVcpuPinDel(vm->def, vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to delete vcpupin xml of " @@ -3834,7 +3835,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (canResetting) { + if (doReset) { if (virDomainVcpuPinDel(persistentDef, vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to delete vcpupin xml of " @@ -3874,6 +3875,7 @@ cleanup: virCgroupFree(&cgroup_dom); if (vm) virDomainObjUnlock(vm); + virBitmapFree(pcpumap); return ret; } @@ -3992,10 +3994,10 @@ qemudDomainPinEmulator(virDomainPtr dom, virNodeInfo nodeinfo; int ret = -1; qemuDomainObjPrivatePtr priv; - bool canResetting = true; - int pcpu; + bool doReset = false; int newVcpuPinNum = 0; virDomainVcpuPinDefPtr *newVcpuPin = NULL; + virBitmapPtr pcpumap = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -4024,15 +4026,16 @@ qemudDomainPinEmulator(virDomainPtr dom, maxcpu = maplen * 8; if (maxcpu > hostcpus) maxcpu = hostcpus; + + pcpumap = virBitmapAllocFromData(cpumap, maplen); + if (!pcpumap) + goto cleanup; + /* pinning to all physical cpus means resetting, * so check if we can reset setting. */ - for (pcpu = 0; pcpu < hostcpus; pcpu++) { - if ((cpumap[pcpu/8] & (1 << (pcpu % 8))) == 0) { - canResetting = false; - break; - } - } + if (virBitmapIsAllSet(pcpumap)) + doReset = true; pid = vm->pid; @@ -4070,7 +4073,7 @@ qemudDomainPinEmulator(virDomainPtr dom, } } } else { - if (virProcessInfoSetAffinity(pid, cpumap, maplen, maxcpu) < 0) { + if (virProcessInfoSetAffinity(pid, pcpumap) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, "%s", _("failed to set cpu affinity for " "emulator threads")); @@ -4078,7 +4081,7 @@ qemudDomainPinEmulator(virDomainPtr dom, } } - if (canResetting) { + if (doReset) { if (virDomainEmulatorPinDel(vm->def) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to delete emulatorpin xml of " @@ -4105,7 +4108,7 @@ qemudDomainPinEmulator(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (canResetting) { + if (doReset) { if (virDomainEmulatorPinDel(persistentDef) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to delete emulatorpin xml of " @@ -4132,6 +4135,7 @@ cleanup: virCgroupFree(&cgroup_emulator); if (cgroup_dom) virCgroupFree(&cgroup_dom); + virBitmapFree(pcpumap); if (vm) virDomainObjUnlock(vm); @@ -4286,10 +4290,20 @@ qemudDomainGetVcpus(virDomainPtr dom, if (priv->vcpupids != NULL) { for (v = 0 ; v < maxinfo ; v++) { unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); + virBitmapPtr map = NULL; + char *tmpmap = NULL; + int tmpmapLen = 0; if (virProcessInfoGetAffinity(priv->vcpupids[v], - cpumap, maplen, maxcpu) < 0) + &map, maxcpu) < 0) goto cleanup; + virBitmapToData(map, &tmpmap, &tmpmapLen); + if (tmpmapLen > maplen) + tmpmapLen = maplen; + memcpy(cpumap, tmpmap, tmpmapLen); + + VIR_FREE(tmpmap); + virBitmapFree(map); } } else { virReportError(VIR_ERR_OPERATION_INVALID, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4e9aaef..8fe9fa1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1845,8 +1845,7 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, int ret = -1; int i, hostcpus, maxcpu = QEMUD_CPUMASK_LEN; virNodeInfo nodeinfo; - unsigned char *cpumap; - int cpumaplen; + virBitmapPtr cpumap; VIR_DEBUG("Setting CPU affinity"); @@ -1859,8 +1858,8 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, if (maxcpu > hostcpus) maxcpu = hostcpus; - cpumaplen = VIR_CPU_MAPLEN(maxcpu); - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) { + cpumap = virBitmapAlloc(maxcpu); + if (!cpumap) { virReportOOMError(); return -1; } @@ -1873,7 +1872,8 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, int cur_ncpus = driver->caps->host.numaCell[i]->ncpus; if (nodemask[i]) { for (j = 0; j < cur_ncpus; j++) - VIR_USE_CPU(cpumap, driver->caps->host.numaCell[i]->cpus[j]); + ignore_value(virBitmapSetBit(cpumap, + driver->caps->host.numaCell[i]->cpus[j])); } } } else { @@ -1883,14 +1883,13 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, * format to start with ?!?! */ for (i = 0 ; i < maxcpu && i < vm->def->cpumasklen ; i++) if (vm->def->cpumask[i]) - VIR_USE_CPU(cpumap, i); + ignore_value(virBitmapSetBit(cpumap, i)); } else { /* You may think this is redundant, but we can't assume libvirtd * itself is running on all pCPUs, so we need to explicitly set * the spawned QEMU instance to all pCPUs if no map is given in * its config file */ - for (i = 0 ; i < maxcpu ; i++) - VIR_USE_CPU(cpumap, i); + virBitmapSetAll(cpumap); } } @@ -1898,14 +1897,13 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, * so use '0' to indicate our own process ID. No threads are * running at this point */ - if (virProcessInfoSetAffinity(0, /* Self */ - cpumap, cpumaplen, maxcpu) < 0) + if (virProcessInfoSetAffinity(0 /* Self */, cpumap) < 0) goto cleanup; ret = 0; cleanup: - VIR_FREE(cpumap); + virBitmapFree(cpumap); return ret; } @@ -1950,12 +1948,8 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; virNodeInfo nodeinfo; - pid_t vcpupid; - virBitmapPtr cpumask; - int vcpu, cpumaplen, hostcpus, maxcpu, n; - unsigned char *cpumap = NULL; + int vcpu, n; int ret = -1; - bool result; if (virNodeGetInfo(conn, &nodeinfo) != 0) { return -1; @@ -1970,41 +1964,17 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn, return -1; } - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); - cpumaplen = VIR_CPU_MAPLEN(hostcpus); - maxcpu = cpumaplen * 8; - - if (maxcpu > hostcpus) - maxcpu = hostcpus; - - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) { - virReportOOMError(); - return -1; - } - for (n = 0; n < def->cputune.nvcpupin; n++) { - int i; vcpu = def->cputune.vcpupin[n]->vcpuid; - memset(cpumap, 0, cpumaplen); - cpumask = def->cputune.vcpupin[n]->cpumask; - vcpupid = priv->vcpupids[vcpu]; - - i = -1; - while ((i = virBitmapNextSetBit(cpumask, i)) > 0) - VIR_USE_CPU(cpumap, i); - - if (virProcessInfoSetAffinity(vcpupid, - cpumap, - cpumaplen, - maxcpu) < 0) { + if (virProcessInfoSetAffinity(priv->vcpupids[vcpu], + def->cputune.vcpupin[n]->cpumask) < 0) { goto cleanup; } } ret = 0; cleanup: - VIR_FREE(cpumap); return ret; } @@ -2014,13 +1984,8 @@ qemuProcessSetEmulatorAffinites(virConnectPtr conn, virDomainObjPtr vm) { virDomainDefPtr def = vm->def; - pid_t pid = vm->pid; - virBitmapPtr cpumask = NULL; - unsigned char *cpumap = NULL; virNodeInfo nodeinfo; - int cpumaplen, hostcpus, maxcpu, i; int ret = -1; - bool result; if (virNodeGetInfo(conn, &nodeinfo) != 0) return -1; @@ -2028,36 +1993,13 @@ qemuProcessSetEmulatorAffinites(virConnectPtr conn, if (!def->cputune.emulatorpin) return 0; - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); - cpumaplen = VIR_CPU_MAPLEN(hostcpus); - maxcpu = cpumaplen * CHAR_BIT; - - if (maxcpu > hostcpus) - maxcpu = hostcpus; - - if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) { - virReportOOMError(); - return -1; - } - - cpumask = def->cputune.emulatorpin->cpumask; - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { - if (virBitmapGetBit(cpumask, i, &result) < 0) - goto cleanup; - if (result) - VIR_USE_CPU(cpumap, i); - } - - if (virProcessInfoSetAffinity(pid, - cpumap, - cpumaplen, - maxcpu) < 0) { + if (virProcessInfoSetAffinity(vm->pid, + def->cputune.emulatorpin->cpumask) < 0) { goto cleanup; } ret = 0; cleanup: - VIR_FREE(cpumap); return ret; } diff --git a/src/util/processinfo.c b/src/util/processinfo.c index 16ab8af..b7d2ed0 100644 --- a/src/util/processinfo.c +++ b/src/util/processinfo.c @@ -30,12 +30,10 @@ #if HAVE_SCHED_GETAFFINITY -int virProcessInfoSetAffinity(pid_t pid, - const unsigned char *map, - size_t maplen, - int maxcpu) +int virProcessInfoSetAffinity(pid_t pid, virBitmapPtr map) { int i; + bool set = false; # ifdef CPU_ALLOC /* New method dynamically allocates cpu mask, allowing unlimted cpus */ int numcpus = 1024; @@ -59,8 +57,10 @@ realloc: } CPU_ZERO_S(masklen, mask); - for (i = 0 ; i < maxcpu ; i++) { - if (VIR_CPU_USABLE(map, maplen, 0, i)) + for (i = 0 ; i < virBitmapSize(map); i++) { + if (virBitmapGetBit(map, i, &set) < 0) + return -1; + if (set) CPU_SET_S(i, masklen, mask); } @@ -81,8 +81,10 @@ realloc: cpu_set_t mask; CPU_ZERO(&mask); - for (i = 0 ; i < maxcpu ; i++) { - if (VIR_CPU_USABLE(map, maplen, 0, i)) + for (i = 0 ; i < virBitmapSize(map); i++) { + if (virBitmapGetBit(map, i, &set) < 0) + return -1; + if (set) CPU_SET(i, &mask); } @@ -97,8 +99,7 @@ realloc: } int virProcessInfoGetAffinity(pid_t pid, - unsigned char *map, - size_t maplen ATTRIBUTE_UNUSED, + virBitmapPtr *map, int maxcpu) { int i; @@ -137,9 +138,15 @@ realloc: return -1; } + *map = virBitmapAlloc(maxcpu); + if (!map) { + virReportOOMError(); + return -1; + } + for (i = 0 ; i < maxcpu ; i++) if (CPU_ISSET_S(i, masklen, mask)) - VIR_USE_CPU(map, i); + ignore_value(virBitmapSetBit(*map, i)); CPU_FREE(mask); # else /* Legacy method uses a fixed size cpu mask, only allows upto 1024 cpus */ @@ -163,9 +170,7 @@ realloc: #else /* HAVE_SCHED_GETAFFINITY */ int virProcessInfoSetAffinity(pid_t pid ATTRIBUTE_UNUSED, - const unsigned char *map ATTRIBUTE_UNUSED, - size_t maplen ATTRIBUTE_UNUSED, - int maxcpu ATTRIBUTE_UNUSED) + virBitmapPtr map ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Process CPU affinity is not supported on this platform")); @@ -173,8 +178,7 @@ int virProcessInfoSetAffinity(pid_t pid ATTRIBUTE_UNUSED, } int virProcessInfoGetAffinity(pid_t pid ATTRIBUTE_UNUSED, - unsigned char *map ATTRIBUTE_UNUSED, - size_t maplen ATTRIBUTE_UNUSED, + virBitmapPtr *map ATTRIBUTE_UNUSED, int maxcpu ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/processinfo.h b/src/util/processinfo.h index a648bb8..0424643 100644 --- a/src/util/processinfo.h +++ b/src/util/processinfo.h @@ -23,15 +23,12 @@ # define __VIR_PROCESSINFO_H__ # include "internal.h" +# include "bitmap.h" -int virProcessInfoSetAffinity(pid_t pid, - const unsigned char *map, - size_t maplen, - int maxcpu); +int virProcessInfoSetAffinity(pid_t pid, virBitmapPtr map); int virProcessInfoGetAffinity(pid_t pid, - unsigned char *map, - size_t maplen, + virBitmapPtr *map, int maxcpu); #endif /* __VIR_PROCESSINFO_H__ */ -- 1.7.10.2

--- src/conf/domain_conf.c | 24 +++++----------- src/conf/domain_conf.h | 2 +- src/lxc/lxc_controller.c | 25 ++++++++-------- src/parallels/parallels_driver.c | 2 +- src/qemu/qemu_cgroup.c | 7 ++--- src/qemu/qemu_cgroup.h | 2 +- src/qemu/qemu_driver.c | 58 ++++++++++---------------------------- src/qemu/qemu_process.c | 52 ++++++++++++++++------------------ 8 files changed, 65 insertions(+), 107 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b07b6b7..a02b805 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1680,7 +1680,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainVcpuPinDefFree(def->cputune.emulatorpin); - VIR_FREE(def->numatune.memory.nodemask); + virBitmapFree(def->numatune.memory.nodemask); virSysinfoDefFree(def->sysinfo); @@ -8520,19 +8520,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodeset = virXMLPropString(cur, "nodeset"); if (nodeset) { - char *set = nodeset; - int nodemasklen = VIR_DOMAIN_CPUMASK_LEN; - - if (VIR_ALLOC_N(def->numatune.memory.nodemask, - nodemasklen) < 0) { - virReportOOMError(); - goto error; - } - - /* "nodeset" uses the same syntax as "cpuset". */ - if (virDomainCpuSetParse(set, 0, - def->numatune.memory.nodemask, - nodemasklen) < 0) { + if (virBitmapParse(nodeset, + 0, + &def->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN) < 0) { VIR_FREE(nodeset); goto error; } @@ -8574,7 +8565,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* Ignore 'nodeset' if 'placement' is 'auto' finally */ if (placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) - VIR_FREE(def->numatune.memory.nodemask); + virBitmapFree(def->numatune.memory.nodemask); /* Copy 'placement' of <numatune> to <vcpu> if its 'placement' * is not specified and 'placement' of <numatune> is specified. @@ -13242,8 +13233,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->numatune.memory.placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) { - nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + nodemask = virBitmapFormat(def->numatune.memory.nodemask); if (nodemask == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format nodeset for " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bb3721c..975c565 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1560,7 +1560,7 @@ typedef struct _virDomainNumatuneDef virDomainNumatuneDef; typedef virDomainNumatuneDef *virDomainNumatuneDefPtr; struct _virDomainNumatuneDef { struct { - char *nodemask; + virBitmapPtr nodemask; int mode; int placement_mode; /* enum virDomainNumatuneMemPlacementMode */ } memory; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index ac8ce94..acaed60 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -418,20 +418,19 @@ static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl) /* Convert nodemask to NUMA bitmask. */ nodemask_zero(&mask); - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { - if (ctrl->def->numatune.memory.nodemask[i]) { - if (i > NUMA_NUM_NODES) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Host cannot support NUMA node %d"), i); - return -1; - } - if (i > maxnode && !warned) { - VIR_WARN("nodeset is out of range, there is only %d NUMA " - "nodes on host", maxnode); - warned = true; - } - nodemask_set(&mask, i); + i = -1; + while ((i = virBitmapNextSetBit(ctrl->def->numatune.memory.nodemask, i)) > 0) { + if (i > NUMA_NUM_NODES) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Host cannot support NUMA node %d"), i); + return -1; + } + if (i > maxnode && !warned) { + VIR_WARN("nodeset is out of range, there is only %d NUMA " + "nodes on host", maxnode); + warned = true; } + nodemask_set(&mask, i); } mode = ctrl->def->numatune.memory.mode; diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 06a75b3..0d8dcb8 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1428,7 +1428,7 @@ parallelsApplyChanges(virDomainObjPtr dom, virDomainDefPtr new) if (old->numatune.memory.mode != new->numatune.memory.mode || old->numatune.memory.placement_mode != new->numatune.memory.placement_mode || - !STREQ_NULLABLE(old->numatune.memory.nodemask, new->numatune.memory.nodemask)) { + !virBitmapEqual(old->numatune.memory.nodemask, new->numatune.memory.nodemask)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("numa parameters are not supported " diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index be95761..0947527 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -195,7 +195,7 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED, int qemuSetupCgroup(struct qemud_driver *driver, virDomainObjPtr vm, - char *nodemask) + virBitmapPtr nodemask) { virCgroupPtr cgroup = NULL; int rc; @@ -412,10 +412,9 @@ int qemuSetupCgroup(struct qemud_driver *driver, char *mask = NULL; if (vm->def->numatune.memory.placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) - mask = virDomainCpuSetFormat(nodemask, VIR_DOMAIN_CPUMASK_LEN); + mask = virBitmapFormat(nodemask); else - mask = virDomainCpuSetFormat(vm->def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + mask = virBitmapFormat(vm->def->numatune.memory.nodemask); if (!mask) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to convert memory nodemask")); diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 04f70a1..1de4856 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -49,7 +49,7 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev, void *opaque); int qemuSetupCgroup(struct qemud_driver *driver, virDomainObjPtr vm, - char *nodemask); + virBitmapPtr nodemask); int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e96c3b8..3340928 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7512,18 +7512,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { int rc; - char *nodeset = NULL; + virBitmapPtr nodeset = NULL; char *nodeset_str = NULL; - if (VIR_ALLOC_N(nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) { - virReportOOMError(); - ret = -1; - goto cleanup; - }; - - if (virDomainCpuSetParse(params[i].value.s, - 0, nodeset, - VIR_DOMAIN_CPUMASK_LEN) < 0) { + 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; @@ -7536,17 +7530,16 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("change of nodeset for running domain " "requires strict numa mode")); - VIR_FREE(nodeset); + virBitmapFree(nodeset); ret = -1; continue; } /* Ensure the cpuset string is formated before passing to cgroup */ - if (!(nodeset_str = virDomainCpuSetFormat(nodeset, - VIR_DOMAIN_CPUMASK_LEN))) { + if (!(nodeset_str = virBitmapFormat(nodeset))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to format nodeset")); - VIR_FREE(nodeset); + virBitmapFree(nodeset); ret = -1; continue; } @@ -7554,7 +7547,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, if ((rc = virCgroupSetCpusetMems(group, nodeset_str) != 0)) { virReportSystemError(-rc, "%s", _("unable to set numa tunable")); - VIR_FREE(nodeset); + virBitmapFree(nodeset); VIR_FREE(nodeset_str); ret = -1; continue; @@ -7563,40 +7556,22 @@ qemuDomainSetNumaParameters(virDomainPtr dom, /* update vm->def here so that dumpxml can read the new * values from vm->def. */ - if (!vm->def->numatune.memory.nodemask) { - if (VIR_ALLOC_N(vm->def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN) < 0) { - virReportOOMError(); - VIR_FREE(nodeset); - ret = -1; - goto cleanup; - } - } else { - VIR_FREE(vm->def->numatune.memory.nodemask); - } + virBitmapFree(vm->def->numatune.memory.nodemask); - vm->def->numatune.memory.nodemask = nodeset; vm->def->numatune.memory.placement_mode = VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC; + vm->def->numatune.memory.nodemask = virBitmapCopy(nodeset); } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!persistentDef->numatune.memory.nodemask) { - if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN) < 0) { - virReportOOMError(); - VIR_FREE(nodeset); - ret = -1; - goto cleanup; - } - } else { - VIR_FREE(persistentDef->numatune.memory.nodemask); - } + virBitmapFree(persistentDef->numatune.memory.nodemask); persistentDef->numatune.memory.nodemask = nodeset; persistentDef->numatune.memory.placement_mode = VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC; + nodeset = NULL; } + virBitmapFree(nodeset); } } @@ -7691,11 +7666,8 @@ qemuDomainGetNumaParameters(virDomainPtr dom, case 1: /* fill numa nodeset here */ if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - char *mask = persistentDef->numatune.memory.nodemask; - if (mask) - nodeset = virDomainCpuSetFormat(mask, - VIR_DOMAIN_CPUMASK_LEN); - else + nodeset = virBitmapFormat(persistentDef->numatune.memory.nodemask); + if (!nodeset) nodeset = strdup(""); } else { rc = virCgroupGetCpusetMems(group, &nodeset); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8fe9fa1..627516d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -63,6 +63,7 @@ #include "uuid.h" #include "virtime.h" #include "virnetdevtap.h" +#include "bitmap.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1696,7 +1697,7 @@ qemuProcessDetectVcpuPIDs(struct qemud_driver *driver, #if HAVE_NUMACTL static int qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm, - const char *nodemask) + virBitmapPtr nodemask) { nodemask_t mask; int mode = -1; @@ -1706,7 +1707,7 @@ qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm, int maxnode = 0; bool warned = false; virDomainNumatuneDef numatune = vm->def->numatune; - const char *tmp_nodemask = NULL; + virBitmapPtr tmp_nodemask = NULL; if (numatune.memory.placement_mode == VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) { @@ -1731,20 +1732,19 @@ qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm, maxnode = numa_max_node() + 1; /* Convert nodemask to NUMA bitmask. */ nodemask_zero(&mask); - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { - if (tmp_nodemask[i]) { - if (i > NUMA_NUM_NODES) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Host cannot support NUMA node %d"), i); - return -1; - } - if (i > maxnode && !warned) { - VIR_WARN("nodeset is out of range, there is only %d NUMA " - "nodes on host", maxnode); - warned = true; - } - nodemask_set(&mask, i); + i = -1; + while ((i = virBitmapNextSetBit(tmp_nodemask, i)) > 0) { + if (i > NUMA_NUM_NODES) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Host cannot support NUMA node %d"), i); + return -1; } + if (i > maxnode && !warned) { + VIR_WARN("nodeset is out of range, there is only %d NUMA " + "nodes on host", maxnode); + warned = true; + } + nodemask_set(&mask, i); } mode = numatune.memory.mode; @@ -1840,7 +1840,7 @@ qemuGetNumadAdvice(virDomainDefPtr def ATTRIBUTE_UNUSED) static int qemuProcessInitCpuAffinity(struct qemud_driver *driver, virDomainObjPtr vm, - const char *nodemask) + virBitmapPtr nodemask) { int ret = -1; int i, hostcpus, maxcpu = QEMUD_CPUMASK_LEN; @@ -1870,7 +1870,10 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, for (i = 0; i < driver->caps->host.nnumaCell; i++) { int j; int cur_ncpus = driver->caps->host.numaCell[i]->ncpus; - if (nodemask[i]) { + bool result; + if (virBitmapGetBit(nodemask, i, &result) < 0) + goto cleanup; + if (result) { for (j = 0; j < cur_ncpus; j++) ignore_value(virBitmapSetBit(cpumap, driver->caps->host.numaCell[i]->cpus[j])); @@ -2575,7 +2578,7 @@ struct qemuProcessHookData { virConnectPtr conn; virDomainObjPtr vm; struct qemud_driver *driver; - char *nodemask; + virBitmapPtr nodemask; }; static int qemuProcessHook(void *data) @@ -3332,7 +3335,7 @@ int qemuProcessStart(virConnectPtr conn, unsigned long cur_balloon; int i; char *nodeset = NULL; - char *nodemask = NULL; + virBitmapPtr nodemask = NULL; unsigned int stop_flags; /* Okay, these are just internal flags, @@ -3529,13 +3532,8 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Nodeset returned from numad: %s", nodeset); - if (VIR_ALLOC_N(nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { - virReportOOMError(); - goto cleanup; - } - - if (virDomainCpuSetParse(nodeset, 0, nodemask, - VIR_DOMAIN_CPUMASK_LEN) < 0) + if (virBitmapParse(nodeset, 0, &nodemask, + VIR_DOMAIN_CPUMASK_LEN) < 0) goto cleanup; } hookData.nodemask = nodemask; @@ -3864,7 +3862,7 @@ cleanup: * if we failed to initialize the now running VM. kill it off and * pretend we never started it */ VIR_FREE(nodeset); - VIR_FREE(nodemask); + virBitmapFree(nodemask); virCommandFree(cmd); VIR_FORCE_CLOSE(logfile); qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags); -- 1.7.10.2

--- src/conf/domain_conf.c | 24 +++++++++--------------- src/conf/domain_conf.h | 3 +-- src/lxc/lxc_controller.c | 14 ++++++-------- src/parallels/parallels_driver.c | 3 +-- src/qemu/qemu_process.c | 12 +++++------- src/test/test_driver.c | 5 ++++- src/vmx/vmx.c | 36 ++++++++++++++++++------------------ tests/cpuset | 2 +- 8 files changed, 45 insertions(+), 54 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a02b805..7321268 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8391,14 +8391,12 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { tmp = virXPathString("string(./vcpu[1]/@cpuset)", ctxt); if (tmp) { - char *set = tmp; - def->cpumasklen = VIR_DOMAIN_CPUMASK_LEN; - if (VIR_ALLOC_N(def->cpumask, def->cpumasklen) < 0) { - goto no_memory; - } - if (virDomainCpuSetParse(set, 0, def->cpumask, - def->cpumasklen) < 0) + if (virBitmapParse(tmp, 0, &def->cpumask, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("topology cpuset syntax error")); goto error; + } VIR_FREE(tmp); } } @@ -13007,7 +13005,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; const char *type = NULL; - int n, allones = 1; + int n; int i; bool blkio = false; @@ -13136,17 +13134,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, " </memoryBacking>\n", NULL); } - for (n = 0 ; n < def->cpumasklen ; n++) - if (def->cpumask[n] != 1) - allones = 0; - virBufferAddLit(buf, " <vcpu"); virBufferAsprintf(buf, " placement='%s'", virDomainCpuPlacementModeTypeToString(def->placement_mode)); - if (!allones) { + + if (def->cpumask && !virBitmapIsAllSet(def->cpumask)) { char *cpumask = NULL; - if ((cpumask = - virDomainCpuSetFormat(def->cpumask, def->cpumasklen)) == NULL) + if ((cpumask = virBitmapFormat(def->cpumask)) == NULL) goto cleanup; virBufferAsprintf(buf, " cpuset='%s'", cpumask); VIR_FREE(cpumask); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 975c565..042b518 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1614,8 +1614,7 @@ struct _virDomainDef { unsigned short vcpus; unsigned short maxvcpus; int placement_mode; - int cpumasklen; - char *cpumask; + virBitmapPtr cpumask; struct { unsigned long shares; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index acaed60..0c5dc39 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -490,9 +490,9 @@ static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl) */ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) { - int i, hostcpus, maxcpu = CPU_SETSIZE; + int hostcpus, maxcpu = CPU_SETSIZE; virNodeInfo nodeinfo; - virBitmapPtr cpumap; + virBitmapPtr cpumap, cpumapToSet; VIR_DEBUG("Setting CPU affinity"); @@ -509,12 +509,10 @@ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) if (!cpumap) return -1; + cpumapToSet = cpumap; + if (ctrl->def->cpumask) { - /* XXX why don't we keep 'cpumask' in the libvirt cpumap - * format to start with ?!?! */ - for (i = 0 ; i < maxcpu && i < ctrl->def->cpumasklen ; i++) - if (ctrl->def->cpumask[i]) - ignore_value(virBitmapSetBit(cpumap, i)); + cpumapToSet = ctrl->def->cpumask; } else { /* You may think this is redundant, but we can't assume libvirtd * itself is running on all pCPUs, so we need to explicitly set @@ -527,7 +525,7 @@ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) * so use '0' to indicate our own process ID. No threads are * running at this point */ - if (virProcessInfoSetAffinity(0 /* Self */, cpumap) < 0) { + if (virProcessInfoSetAffinity(0 /* Self */, cpumapToSet) < 0) { virBitmapFree(cpumap); return -1; } diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 0d8dcb8..66456d1 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1407,8 +1407,7 @@ parallelsApplyChanges(virDomainObjPtr dom, virDomainDefPtr new) return -1; } - if (old->cpumasklen != new->cpumasklen || - (memcmp(old->cpumask, new->cpumask, old->cpumasklen))) { + if (!virBitmapEqual(old->cpumask, new->cpumask)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("changing cpu mask is not supported " diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 627516d..0bad1a8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1845,7 +1845,7 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, int ret = -1; int i, hostcpus, maxcpu = QEMUD_CPUMASK_LEN; virNodeInfo nodeinfo; - virBitmapPtr cpumap; + virBitmapPtr cpumap, cpumapToSet; VIR_DEBUG("Setting CPU affinity"); @@ -1864,6 +1864,8 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, return -1; } + cpumapToSet = cpumap; + if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { VIR_DEBUG("Set CPU affinity with advisory nodeset from numad"); /* numad returns the NUMA node list, convert it to cpumap */ @@ -1882,11 +1884,7 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, } else { VIR_DEBUG("Set CPU affinity with specified cpuset"); if (vm->def->cpumask) { - /* XXX why don't we keep 'cpumask' in the libvirt cpumap - * format to start with ?!?! */ - for (i = 0 ; i < maxcpu && i < vm->def->cpumasklen ; i++) - if (vm->def->cpumask[i]) - ignore_value(virBitmapSetBit(cpumap, i)); + cpumapToSet = vm->def->cpumask; } else { /* You may think this is redundant, but we can't assume libvirtd * itself is running on all pCPUs, so we need to explicitly set @@ -1900,7 +1898,7 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, * so use '0' to indicate our own process ID. No threads are * running at this point */ - if (virProcessInfoSetAffinity(0 /* Self */, cpumap) < 0) + if (virProcessInfoSetAffinity(0 /* Self */, cpumapToSet) < 0) goto cleanup; ret = 0; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index aa4418a..2e2fb67 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -383,6 +383,7 @@ testDomainUpdateVCPU(virConnectPtr conn ATTRIBUTE_UNUSED, virVcpuInfoPtr info = &privdata->vcpu_infos[vcpu]; unsigned char *cpumap = VIR_GET_CPUMAP(privdata->cpumaps, maplen, vcpu); int j; + bool cpu; memset(info, 0, sizeof(virVcpuInfo)); memset(cpumap, 0, maplen); @@ -394,7 +395,9 @@ testDomainUpdateVCPU(virConnectPtr conn ATTRIBUTE_UNUSED, if (dom->def->cpumask) { for (j = 0; j < maxcpu && j < VIR_DOMAIN_CPUMASK_LEN; ++j) { - if (dom->def->cpumask[j]) { + if (virBitmapGetBit(dom->def->cpumask, j, &cpu) < 0) + return -1; + if (cpu) { VIR_USE_CPU(cpumap, j); info->cpu = j; } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index db22624..31bb738 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1235,6 +1235,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) int unit; bool hgfs_disabled = true; long long sharedFolder_maxNum = 0; + int cpumasklen; if (ctx->parseFileName == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1417,9 +1418,10 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) const char *current = sched_cpu_affinity; int number, count = 0; - def->cpumasklen = 0; + cpumasklen = 0; - if (VIR_ALLOC_N(def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) { + def->cpumask = virBitmapAlloc(VIR_DOMAIN_CPUMASK_LEN); + if (!def->cpumask) { virReportOOMError(); goto cleanup; } @@ -1444,11 +1446,11 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) goto cleanup; } - if (number + 1 > def->cpumasklen) { - def->cpumasklen = number + 1; + if (number + 1 > cpumasklen) { + cpumasklen = number + 1; } - def->cpumask[number] = 1; + ignore_value(virBitmapSetBit(def->cpumask, number)); ++count; virSkipSpaces(¤t); @@ -3165,15 +3167,14 @@ virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, virBufferAsprintf(&buffer, "numvcpus = \"%d\"\n", def->maxvcpus); /* def:cpumask -> vmx:sched.cpu.affinity */ - if (def->cpumasklen > 0) { + if (def->cpumask && virBitmapSize(def->cpumask) > 0) { virBufferAddLit(&buffer, "sched.cpu.affinity = \""); sched_cpu_affinity_length = 0; - for (i = 0; i < def->cpumasklen; ++i) { - if (def->cpumask[i]) { - ++sched_cpu_affinity_length; - } + i = -1; + while ((i = virBitmapNextSetBit(def->cpumask, i)) > 0) { + ++sched_cpu_affinity_length; } if (sched_cpu_affinity_length < def->maxvcpus) { @@ -3184,16 +3185,15 @@ virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, goto cleanup; } - for (i = 0; i < def->cpumasklen; ++i) { - if (def->cpumask[i]) { - virBufferAsprintf(&buffer, "%d", i); - - if (sched_cpu_affinity_length > 1) { - virBufferAddChar(&buffer, ','); - } + i = -1; + while ((i = virBitmapNextSetBit(def->cpumask, i)) > 0) { + virBufferAsprintf(&buffer, "%d", i); - --sched_cpu_affinity_length; + if (sched_cpu_affinity_length > 1) { + virBufferAddChar(&buffer, ','); } + + --sched_cpu_affinity_length; } virBufferAddLit(&buffer, "\"\n"); diff --git a/tests/cpuset b/tests/cpuset index 4618a54..b617d6f 100755 --- 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: internal error topology cpuset syntax error +error: XML error: topology cpuset syntax error EOF compare exp out || fail=1 -- 1.7.10.2

--- src/conf/cpu_conf.c | 17 ++++++----------- src/conf/cpu_conf.h | 3 ++- src/qemu/qemu_command.c | 43 +++++++------------------------------------ 3 files changed, 15 insertions(+), 48 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 87e9540..9fe93e6 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -83,7 +83,7 @@ virCPUDefFree(virCPUDefPtr def) virCPUDefFreeModel(def); for (i = 0 ; i < def->ncells ; i++) { - VIR_FREE(def->cells[i].cpumask); + virBitmapFree(def->cells[i].cpumask); VIR_FREE(def->cells[i].cpustr); } VIR_FREE(def->cells); @@ -164,11 +164,10 @@ virCPUDefCopy(const virCPUDefPtr cpu) copy->cells[i].cellid = cpu->cells[i].cellid; copy->cells[i].mem = cpu->cells[i].mem; - if (VIR_ALLOC_N(copy->cells[i].cpumask, - VIR_DOMAIN_CPUMASK_LEN) < 0) + copy->cells[i].cpumask = virBitmapCopy(cpu->cells[i].cpumask); + + if (!copy->cells[i].cpumask) goto no_memory; - memcpy(copy->cells[i].cpumask, cpu->cells[i].cpumask, - VIR_DOMAIN_CPUMASK_LEN); if (!(copy->cells[i].cpustr = strdup(cpu->cells[i].cpustr))) goto no_memory; @@ -454,7 +453,6 @@ virCPUDefParseXML(const xmlNodePtr node, for (i = 0 ; i < n ; i++) { char *cpus, *memory; - int cpumasklen = VIR_DOMAIN_CPUMASK_LEN; int ret, ncpus = 0; def->cells[i].cellid = i; @@ -466,11 +464,8 @@ virCPUDefParseXML(const xmlNodePtr node, } def->cells[i].cpustr = cpus; - if (VIR_ALLOC_N(def->cells[i].cpumask, cpumasklen) < 0) - goto no_memory; - - ncpus = virDomainCpuSetParse(cpus, 0, def->cells[i].cpumask, - cpumasklen); + ncpus = virBitmapParse(cpus, 0, &def->cells[i].cpumask, + VIR_DOMAIN_CPUMASK_LEN); if (ncpus <= 0) goto error; def->cells_cpus += ncpus; diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 601e208..4e03fd2 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -27,6 +27,7 @@ # include "util.h" # include "buf.h" # include "xml.h" +# include "bitmap.h" # define VIR_CPU_VENDOR_ID_LENGTH 12 @@ -92,7 +93,7 @@ typedef struct _virCellDef virCellDef; typedef virCellDef *virCellDefPtr; struct _virCellDef { int cellid; - char *cpumask; /* CPUs that are part of this node */ + virBitmapPtr cpumask; /* CPUs that are part of this node */ char *cpustr; /* CPUs stored in string form for dumpxml */ unsigned int mem; /* Node memory in kB */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a83d6de..5fb607f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4260,54 +4260,25 @@ qemuBuildSmpArgStr(const virDomainDefPtr def, return virBufferContentAndReset(&buf); } -static void -qemuBuildNumaCPUArgStr(char *cpumask, virBufferPtr buf) -{ - int i, first, last; - int cpuSet = 0; - - first = last = 0; - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { - if (cpumask[i]) { - if (cpuSet) { - last = i; - } else { - first = last = i; - cpuSet = 1; - } - } else { - if (!cpuSet) - continue; - if (first == last) - virBufferAsprintf(buf, "%d,", first); - else - virBufferAsprintf(buf, "%d-%d,", first, last); - cpuSet = 0; - } - } - - if (cpuSet) { - if (first == last) - virBufferAsprintf(buf, "%d,", first); - else - virBufferAsprintf(buf, "%d-%d,", first, last); - } -} - static int qemuBuildNumaArgStr(const virDomainDefPtr def, virCommandPtr cmd) { int i; virBuffer buf = VIR_BUFFER_INITIALIZER; + char *cpumask; for (i = 0; i < def->cpu->ncells; i++) { virCommandAddArg(cmd, "-numa"); virBufferAsprintf(&buf, "node,nodeid=%d", def->cpu->cells[i].cellid); virBufferAddLit(&buf, ",cpus="); - qemuBuildNumaCPUArgStr(def->cpu->cells[i].cpumask, &buf); + cpumask = virBitmapFormat(def->cpu->cells[i].cpumask); + if (cpumask) { + virBufferAsprintf(&buf, "%s", cpumask); + VIR_FREE(cpumask); + } def->cpu->cells[i].mem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024) * 1024; - virBufferAsprintf(&buf, "mem=%d", def->cpu->cells[i].mem / 1024); + virBufferAsprintf(&buf, ",mem=%d", def->cpu->cells[i].mem / 1024); if (virBufferError(&buf)) goto error; -- 1.7.10.2

--- src/nodeinfo.c | 26 +++++++++++--------------- src/nodeinfo.h | 6 +++--- src/qemu/qemu_driver.c | 19 ++++++++++++------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e3d4a24..e6b259d 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -737,10 +737,10 @@ cleanup: * and max cpu is 7. The map file shows 0-4,6-7. This function parses * it and returns cpumap. */ -static char * +static virBitmapPtr linuxParseCPUmap(int *max_cpuid, const char *path) { - char *map = NULL; + virBitmapPtr map = NULL; char *str = NULL; int max_id = 0, i; @@ -749,20 +749,16 @@ linuxParseCPUmap(int *max_cpuid, const char *path) goto error; } - if (VIR_ALLOC_N(map, VIR_DOMAIN_CPUMASK_LEN) < 0) { - virReportOOMError(); - goto error; - } - if (virDomainCpuSetParse(str, 0, map, - VIR_DOMAIN_CPUMASK_LEN) < 0) { + if (virBitmapParse(str, 0, &map, + VIR_DOMAIN_CPUMASK_LEN) < 0) { goto error; } - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { - if (map[i]) { - max_id = i; - } + i = -1; + while ((i = virBitmapNextSetBit(map, i)) > 0) { + max_id = i; } + *max_cpuid = max_id; VIR_FREE(str); @@ -770,7 +766,7 @@ linuxParseCPUmap(int *max_cpuid, const char *path) error: VIR_FREE(str); - VIR_FREE(map); + virBitmapFree(map); return NULL; } #endif @@ -909,14 +905,14 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, #endif } -char * +virBitmapPtr nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED, int *max_id ATTRIBUTE_UNUSED, const char *mapname ATTRIBUTE_UNUSED) { #ifdef __linux__ char *path; - char *cpumap; + virBitmapPtr cpumap; if (virAsprintf(&path, SYSFS_SYSTEM_PATH "/cpu/%s", mapname) < 0) { virReportOOMError(); diff --git a/src/nodeinfo.h b/src/nodeinfo.h index 12090e2..182b0b6 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -46,7 +46,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn, int maxCells); unsigned long long nodeGetFreeMemory(virConnectPtr conn); -char *nodeGetCPUmap(virConnectPtr conn, - int *max_id, - const char *mapname); +virBitmapPtr nodeGetCPUmap(virConnectPtr conn, + int *max_id, + const char *mapname); #endif /* __VIR_NODEINFO_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3340928..d271caf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13482,8 +13482,8 @@ qemuDomainGetPercpuStats(virDomainPtr domain, int start_cpu, unsigned int ncpus) { - char *map = NULL; - char *map2 = NULL; + virBitmapPtr map = NULL; + virBitmapPtr map2 = NULL; int rv = -1; int i, id, max_id; char *pos; @@ -13495,6 +13495,7 @@ qemuDomainGetPercpuStats(virDomainPtr domain, virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; + bool result; /* return the number of supported params */ if (nparams == 0 && ncpus != 0) @@ -13533,7 +13534,9 @@ qemuDomainGetPercpuStats(virDomainPtr domain, id = start_cpu + ncpus - 1; for (i = 0; i <= id; i++) { - if (!map[i]) { + if (virBitmapGetBit(map, i, &result) < 0) + goto cleanup; + if (!result) { cpu_time = 0; } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -13565,7 +13568,7 @@ qemuDomainGetPercpuStats(virDomainPtr domain, /* Check that the mapping of online cpus didn't change mid-parse. */ map2 = nodeGetCPUmap(domain->conn, &max_id, "present"); - if (!map2 || memcmp(map, map2, VIR_DOMAIN_CPUMASK_LEN) != 0) { + if (!map2 || !virBitmapEqual(map, map2)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("the set of online cpus changed while reading")); goto cleanup; @@ -13573,7 +13576,9 @@ qemuDomainGetPercpuStats(virDomainPtr domain, sum_cpu_pos = sum_cpu_time; for (i = 0; i <= id; i++) { - if (!map[i]) + if (virBitmapGetBit(map, i, &result) < 0) + goto cleanup; + if (!result) cpu_time = 0; else cpu_time = *(sum_cpu_pos++); @@ -13591,8 +13596,8 @@ qemuDomainGetPercpuStats(virDomainPtr domain, cleanup: VIR_FREE(sum_cpu_time); VIR_FREE(buf); - VIR_FREE(map); - VIR_FREE(map2); + virBitmapFree(map); + virBitmapFree(map2); return rv; } -- 1.7.10.2

virBitmap is recommanded to store cpuset info, and virBitmapFormat/virBitmapParse can do the format/parse jobs. --- src/conf/domain_conf.c | 196 ---------------------------------------------- src/conf/domain_conf.h | 7 -- src/libvirt_private.syms | 2 - 3 files changed, 205 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7321268..f28fdfa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -40,7 +40,6 @@ #include "uuid.h" #include "util.h" #include "buf.h" -#include "c-ctype.h" #include "logging.h" #include "nwfilter_conf.h" #include "storage_file.h" @@ -10848,201 +10847,6 @@ int virDomainDefAddImplicitControllers(virDomainDefPtr def) return 0; } - -/************************************************************************ - * * - * Parser and converter for the CPUset strings used in libvirt * - * * - ************************************************************************/ -/** - * virDomainCpuNumberParse - * @str: pointer to the char pointer used - * @maxcpu: maximum CPU number allowed - * - * Parse a CPU number - * - * Returns the CPU number or -1 in case of error. @str will be - * updated to skip the number. - */ -static int -virDomainCpuNumberParse(const char **str, int maxcpu) -{ - int ret = 0; - const char *cur = *str; - - if (!c_isdigit(*cur)) - return -1; - - while (c_isdigit(*cur)) { - ret = ret * 10 + (*cur - '0'); - if (ret >= maxcpu) - return -1; - cur++; - } - *str = cur; - return ret; -} - -/** - * virDomainCpuSetFormat: - * @conn: connection - * @cpuset: pointer to a char array for the CPU set - * @maxcpu: number of elements available in @cpuset - * - * Serialize the cpuset to a string - * - * Returns the new string NULL in case of error. The string needs to be - * freed by the caller. - */ -char * -virDomainCpuSetFormat(char *cpuset, int maxcpu) -{ - virBuffer buf = VIR_BUFFER_INITIALIZER; - int start, cur; - int first = 1; - - if (!cpuset || maxcpu <= 0 || maxcpu > 100000) - return NULL; - - cur = 0; - start = -1; - while (cur < maxcpu) { - if (cpuset[cur]) { - if (start == -1) - start = cur; - } else if (start != -1) { - if (!first) - virBufferAddLit(&buf, ","); - else - first = 0; - if (cur == start + 1) - virBufferAsprintf(&buf, "%d", start); - else - virBufferAsprintf(&buf, "%d-%d", start, cur - 1); - start = -1; - } - cur++; - } - if (start != -1) { - if (!first) - virBufferAddLit(&buf, ","); - if (maxcpu == start + 1) - virBufferAsprintf(&buf, "%d", start); - else - virBufferAsprintf(&buf, "%d-%d", start, maxcpu - 1); - } - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - - return virBufferContentAndReset(&buf); -} - -/** - * virDomainCpuSetParse: - * @conn: connection - * @str: a CPU set string pointer - * @sep: potential character used to mark the end of string if not 0 - * @cpuset: pointer to a char array for the CPU set - * @maxcpu: number of elements available in @cpuset - * - * Parse the cpu set, it will set the value for enabled CPUs in the @cpuset - * to 1, and 0 otherwise. The syntax allows comma separated entries; each - * can be either a CPU number, ^N to unset that CPU, or N-M for ranges. - * - * Returns the number of CPU found in that set, or -1 in case of error. - * @cpuset is modified accordingly to the value parsed. - */ -int -virDomainCpuSetParse(const char *str, char sep, - char *cpuset, int maxcpu) -{ - const char *cur; - int ret = 0; - int i, start, last; - int neg = 0; - - if (!str || !cpuset || maxcpu <= 0 || maxcpu > 100000) - return -1; - - cur = str; - virSkipSpaces(&cur); - if (*cur == 0) - goto parse_error; - - /* initialize cpumap to all 0s */ - for (i = 0; i < maxcpu; i++) - cpuset[i] = 0; - ret = 0; - - while (*cur != 0 && *cur != sep) { - /* - * 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 = 1; - } - - if (!c_isdigit(*cur)) - goto parse_error; - start = virDomainCpuNumberParse(&cur, maxcpu); - if (start < 0) - goto parse_error; - virSkipSpaces(&cur); - if (*cur == ',' || *cur == 0 || *cur == sep) { - if (neg) { - if (cpuset[start] == 1) { - cpuset[start] = 0; - ret--; - } - } else { - if (cpuset[start] == 0) { - cpuset[start] = 1; - ret++; - } - } - } else if (*cur == '-') { - if (neg) - goto parse_error; - cur++; - virSkipSpaces(&cur); - last = virDomainCpuNumberParse(&cur, maxcpu); - if (last < start) - goto parse_error; - for (i = start; i <= last; i++) { - if (cpuset[i] == 0) { - cpuset[i] = 1; - ret++; - } - } - virSkipSpaces(&cur); - } - if (*cur == ',') { - cur++; - virSkipSpaces(&cur); - neg = 0; - } else if (*cur == 0 || *cur == sep) { - break; - } else { - goto parse_error; - } - } - return ret; - - parse_error: - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("topology cpuset syntax error")); - return -1; -} - - /* Check if vcpupin with same vcpuid already exists. * Return 1 if exists, 0 if not. */ int diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 042b518..bbec034 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1899,13 +1899,6 @@ int virDomainDefFormatInternal(virDomainDefPtr def, int virDomainDefCompatibleDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev); -int virDomainCpuSetParse(const char *str, - char sep, - char *cpuset, - int maxcpu); -char *virDomainCpuSetFormat(char *cpuset, - int maxcpu); - int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, int *nvcpupin, unsigned char *cpumap, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d7cd356..47d62ca 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -306,8 +306,6 @@ virDomainControllerRemove; virDomainControllerTypeToString; virDomainCpuPlacementModeTypeFromString; virDomainCpuPlacementModeTypeToString; -virDomainCpuSetFormat; -virDomainCpuSetParse; virDomainDefAddImplicitControllers; virDomainDefCheckABIStability; virDomainDefClearDeviceAliases; -- 1.7.10.2
participants (2)
-
Eric Blake
-
Hu Tao