On 09/06/2012 04:13 AM, Hu Tao wrote: ACK with one nit. Indentation is now off. -- Eric Blake eblake(a)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 Thu, Sep 06, 2012 at 11:17:58PM -0600, Eric Blake wrote: Thanks for suggestion. OK. Will improve it. OK. OK. OK. Oops. This is a copy&paste. Have a good night:) -- Thanks, Hu Tao

--- 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 | 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(&current); @@ -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/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