[libvirt] [PATCH 0/6] util: bitmap: Various fixes and improvements
Peter Krempa (6): util: bitmap: Rename 'max_bit' to 'nbits' util: bitmap: Fix function formatting and spacing util: bitmap: Add comments for functions which don't have them util: bitmap: Fix value of 'map_alloc' when shrinking bitmap util: bitmap: Use VIR_SHRINK_N in virBitmapShrink util: bitmap: Note that shrinking the bitmap requires clearing of unused bits src/conf/domain_conf.c | 3 +- src/util/virbitmap.c | 213 +++++++++++++++++++++++++++++++++++-------------- src/util/virbitmap.h | 2 +- src/util/virresctrl.c | 3 +- tests/virbitmaptest.c | 6 +- 5 files changed, 157 insertions(+), 70 deletions(-) -- 2.15.0
'max_bit' is misleading as the value is set to the first invalid bit as it's used as the number of bits in the bitmap. Rename it to a more descriptive name. --- src/util/virbitmap.c | 54 ++++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 33cae2f305..c1b97d90fb 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -42,7 +42,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE struct _virBitmap { - size_t max_bit; + size_t nbits; size_t map_len; size_t map_alloc; unsigned long *map; @@ -83,7 +83,7 @@ virBitmapNewQuiet(size_t size) return NULL; } - bitmap->max_bit = size; + bitmap->nbits = size; bitmap->map_len = sz; bitmap->map_alloc = sz; return bitmap; @@ -147,7 +147,7 @@ void virBitmapFree(virBitmapPtr bitmap) int virBitmapCopy(virBitmapPtr dst, virBitmapPtr src) { - if (dst->max_bit != src->max_bit) { + if (dst->nbits != src->nbits) { errno = EINVAL; return -1; } @@ -169,7 +169,7 @@ int virBitmapCopy(virBitmapPtr dst, virBitmapPtr src) */ int virBitmapSetBit(virBitmapPtr bitmap, size_t b) { - if (bitmap->max_bit <= b) + if (bitmap->nbits <= b) return -1; bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] |= VIR_BITMAP_BIT(b); @@ -197,7 +197,7 @@ static int virBitmapExpand(virBitmapPtr map, size_t b) return -1; } - map->max_bit = b + 1; + map->nbits = b + 1; map->map_len = new_len; return 0; @@ -216,7 +216,7 @@ static int virBitmapExpand(virBitmapPtr map, size_t b) */ int virBitmapSetBitExpand(virBitmapPtr bitmap, size_t b) { - if (bitmap->max_bit <= b && virBitmapExpand(bitmap, b) < 0) + if (bitmap->nbits <= b && virBitmapExpand(bitmap, b) < 0) return -1; bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] |= VIR_BITMAP_BIT(b); @@ -235,7 +235,7 @@ int virBitmapSetBitExpand(virBitmapPtr bitmap, size_t b) */ int virBitmapClearBit(virBitmapPtr bitmap, size_t b) { - if (bitmap->max_bit <= b) + if (bitmap->nbits <= b) return -1; bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] &= ~VIR_BITMAP_BIT(b); @@ -255,7 +255,7 @@ int virBitmapClearBit(virBitmapPtr bitmap, size_t b) */ int virBitmapClearBitExpand(virBitmapPtr bitmap, size_t b) { - if (bitmap->max_bit <= b) { + if (bitmap->nbits <= b) { if (virBitmapExpand(bitmap, b) < 0) return -1; } else { @@ -266,7 +266,7 @@ int virBitmapClearBitExpand(virBitmapPtr bitmap, size_t b) } -/* Helper function. caller must ensure b < bitmap->max_bit */ +/* Helper function. caller must ensure b < bitmap->nbits */ static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) { return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b)); @@ -284,7 +284,7 @@ static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) */ bool virBitmapIsBitSet(virBitmapPtr bitmap, size_t b) { - if (bitmap->max_bit <= b) + if (bitmap->nbits <= b) return false; return virBitmapIsSet(bitmap, b); @@ -303,7 +303,7 @@ bool virBitmapIsBitSet(virBitmapPtr bitmap, size_t b) */ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) { - if (bitmap->max_bit <= b) + if (bitmap->nbits <= b) return -1; *result = virBitmapIsSet(bitmap, b); @@ -350,14 +350,14 @@ virBitmapToString(virBitmapPtr bitmap, if (!trim) return ret; - if (bitmap->max_bit != bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT) { + if (bitmap->nbits != bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT) { char *tmp = ret; if (prefix) tmp += 2; len = strlen(tmp); - sz = VIR_DIV_UP(bitmap->max_bit, 4); + sz = VIR_DIV_UP(bitmap->nbits, 4); diff = len - sz; if (diff) @@ -692,7 +692,7 @@ virBitmapPtr virBitmapNewCopy(virBitmapPtr src) { virBitmapPtr dst; - if ((dst = virBitmapNew(src->max_bit)) == NULL) + if ((dst = virBitmapNew(src->nbits)) == NULL) return NULL; if (virBitmapCopy(dst, src) != 0) { @@ -818,7 +818,7 @@ bool virBitmapEqual(virBitmapPtr b1, virBitmapPtr b2) if (!b1 || !b2) return false; - if (b1->max_bit > b2->max_bit) { + if (b1->nbits > b2->nbits) { tmp = b1; b1 = b2; b2 = tmp; @@ -841,7 +841,7 @@ bool virBitmapEqual(virBitmapPtr b1, virBitmapPtr b2) size_t virBitmapSize(virBitmapPtr bitmap) { - return bitmap->max_bit; + return bitmap->nbits; } /** @@ -852,7 +852,7 @@ size_t virBitmapSize(virBitmapPtr bitmap) */ void virBitmapSetAll(virBitmapPtr bitmap) { - int tail = bitmap->max_bit % VIR_BITMAP_BITS_PER_UNIT; + int tail = bitmap->nbits % VIR_BITMAP_BITS_PER_UNIT; memset(bitmap->map, 0xff, bitmap->map_len * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT)); @@ -887,7 +887,7 @@ bool virBitmapIsAllSet(virBitmapPtr bitmap) int unusedBits; size_t sz; - unusedBits = bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap->max_bit; + unusedBits = bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap->nbits; sz = bitmap->map_len; if (unusedBits > 0) @@ -946,7 +946,7 @@ virBitmapNextSetBit(virBitmapPtr bitmap, ssize_t pos) pos++; - if (pos >= bitmap->max_bit) + if (pos >= bitmap->nbits) return -1; nl = pos / VIR_BITMAP_BITS_PER_UNIT; @@ -983,7 +983,7 @@ virBitmapLastSetBit(virBitmapPtr bitmap) if (bitmap->map_len == 0) return -1; - unusedBits = bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap->max_bit; + unusedBits = bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap->nbits; sz = bitmap->map_len - 1; if (unusedBits > 0) { @@ -1035,7 +1035,7 @@ virBitmapNextClearBit(virBitmapPtr bitmap, ssize_t pos) pos++; - if (pos >= bitmap->max_bit) + if (pos >= bitmap->nbits) return -1; nl = pos / VIR_BITMAP_BITS_PER_UNIT; @@ -1048,7 +1048,7 @@ virBitmapNextClearBit(virBitmapPtr bitmap, ssize_t pos) if (nl == bitmap->map_len - 1) { /* Ensure tail bits are ignored. */ - int tail = bitmap->max_bit % VIR_BITMAP_BITS_PER_UNIT; + int tail = bitmap->nbits % VIR_BITMAP_BITS_PER_UNIT; if (tail) bits &= -1UL >> (VIR_BITMAP_BITS_PER_UNIT - tail); @@ -1140,7 +1140,7 @@ virBitmapOverlaps(virBitmapPtr b1, { size_t i; - if (b1->max_bit > b2->max_bit) { + if (b1->nbits > b2->nbits) { virBitmapPtr tmp = b1; b1 = b2; b2 = tmp; @@ -1216,11 +1216,11 @@ virBitmapShrink(virBitmapPtr map, if (!map) return 0; - if (map->max_bit >= b) - map->max_bit = b; + if (map->nbits >= b) + map->nbits = b; - nl = map->max_bit / VIR_BITMAP_BITS_PER_UNIT; - nb = map->max_bit % VIR_BITMAP_BITS_PER_UNIT; + nl = map->nbits / VIR_BITMAP_BITS_PER_UNIT; + nb = map->nbits % VIR_BITMAP_BITS_PER_UNIT; map->map[nl] &= ((1UL << nb) - 1); nl++; -- 2.15.0
--- src/util/virbitmap.c | 107 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 83 insertions(+), 24 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index c1b97d90fb..4229aa286e 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -136,7 +136,8 @@ virBitmapNewEmpty(void) * * Free @bitmap previously allocated by virBitmapNew. */ -void virBitmapFree(virBitmapPtr bitmap) +void +virBitmapFree(virBitmapPtr bitmap) { if (bitmap) { VIR_FREE(bitmap->map); @@ -145,7 +146,9 @@ void virBitmapFree(virBitmapPtr bitmap) } -int virBitmapCopy(virBitmapPtr dst, virBitmapPtr src) +int +virBitmapCopy(virBitmapPtr dst, + virBitmapPtr src) { if (dst->nbits != src->nbits) { errno = EINVAL; @@ -167,7 +170,9 @@ int virBitmapCopy(virBitmapPtr dst, virBitmapPtr src) * * Returns 0 on if bit is successfully set, -1 on error. */ -int virBitmapSetBit(virBitmapPtr bitmap, size_t b) +int +virBitmapSetBit(virBitmapPtr bitmap, + size_t b) { if (bitmap->nbits <= b) return -1; @@ -176,6 +181,7 @@ int virBitmapSetBit(virBitmapPtr bitmap, size_t b) return 0; } + /** * virBitmapExpand: * @map: Pointer to bitmap @@ -186,7 +192,9 @@ int virBitmapSetBit(virBitmapPtr bitmap, size_t b) * * Returns 0 on success, -1 on error. */ -static int virBitmapExpand(virBitmapPtr map, size_t b) +static int +virBitmapExpand(virBitmapPtr map, + size_t b) { size_t new_len = VIR_DIV_UP(b + 1, VIR_BITMAP_BITS_PER_UNIT); @@ -214,7 +222,9 @@ static int virBitmapExpand(virBitmapPtr map, size_t b) * * Returns 0 on if bit is successfully set, -1 on error. */ -int virBitmapSetBitExpand(virBitmapPtr bitmap, size_t b) +int +virBitmapSetBitExpand(virBitmapPtr bitmap, + size_t b) { if (bitmap->nbits <= b && virBitmapExpand(bitmap, b) < 0) return -1; @@ -233,7 +243,9 @@ int virBitmapSetBitExpand(virBitmapPtr bitmap, size_t b) * * Returns 0 on if bit is successfully clear, -1 on error. */ -int virBitmapClearBit(virBitmapPtr bitmap, size_t b) +int +virBitmapClearBit(virBitmapPtr bitmap, + size_t b) { if (bitmap->nbits <= b) return -1; @@ -253,7 +265,9 @@ int virBitmapClearBit(virBitmapPtr bitmap, size_t b) * * Returns 0 on if bit is successfully cleared, -1 on error. */ -int virBitmapClearBitExpand(virBitmapPtr bitmap, size_t b) +int +virBitmapClearBitExpand(virBitmapPtr bitmap, + size_t b) { if (bitmap->nbits <= b) { if (virBitmapExpand(bitmap, b) < 0) @@ -267,11 +281,13 @@ int virBitmapClearBitExpand(virBitmapPtr bitmap, size_t b) /* Helper function. caller must ensure b < bitmap->nbits */ -static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) +static bool +virBitmapIsSet(virBitmapPtr bitmap, size_t b) { return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b)); } + /** * virBitmapIsBitSet: * @bitmap: Pointer to bitmap @@ -282,7 +298,9 @@ static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) * If @b is in the range of @bitmap, returns the value of the bit. * Otherwise false is returned. */ -bool virBitmapIsBitSet(virBitmapPtr bitmap, size_t b) +bool +virBitmapIsBitSet(virBitmapPtr bitmap, + size_t b) { if (bitmap->nbits <= b) return false; @@ -290,6 +308,7 @@ bool virBitmapIsBitSet(virBitmapPtr bitmap, size_t b) return virBitmapIsSet(bitmap, b); } + /** * virBitmapGetBit: * @bitmap: Pointer to bitmap @@ -301,7 +320,10 @@ bool virBitmapIsBitSet(virBitmapPtr bitmap, size_t b) * On success, @result will contain the setting of @b and 0 is * returned. On failure, -1 is returned and @result is unchanged. */ -int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) +int +virBitmapGetBit(virBitmapPtr bitmap, + size_t b, + bool *result) { if (bitmap->nbits <= b) return -1; @@ -310,6 +332,7 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result) return 0; } + /** * virBitmapToString: * @bitmap: Pointer to bitmap @@ -367,6 +390,7 @@ virBitmapToString(virBitmapPtr bitmap, return ret; } + /** * virBitmapFormat: * @bitmap: the bitmap @@ -381,7 +405,8 @@ virBitmapToString(virBitmapPtr bitmap, * Returns the string on success or NULL otherwise. Caller should call * VIR_FREE to free the string. */ -char *virBitmapFormat(virBitmapPtr bitmap) +char * +virBitmapFormat(virBitmapPtr bitmap) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool first = true; @@ -426,6 +451,7 @@ char *virBitmapFormat(virBitmapPtr bitmap) return virBufferContentAndReset(&buf); } + /** * virBitmapParseSeparator: * @str: points to a string representing a human-readable bitmap @@ -546,6 +572,7 @@ virBitmapParseSeparator(const char *str, return -1; } + /** * virBitmapParse: * @str: points to a string representing a human-readable bitmap @@ -569,6 +596,7 @@ virBitmapParse(const char *str, return virBitmapParseSeparator(str, '\0', bitmap, bitmapSize); } + /** * virBitmapParseUnlimited: * @str: points to a string representing a human-readable bitmap @@ -679,6 +707,7 @@ virBitmapParseUnlimited(const char *str) return NULL; } + /** * virBitmapNewCopy: * @src: the source bitmap. @@ -688,7 +717,8 @@ virBitmapParseUnlimited(const char *str) * returns the copied bitmap on success, or NULL otherwise. Caller * should call virBitmapFree to free the returned bitmap. */ -virBitmapPtr virBitmapNewCopy(virBitmapPtr src) +virBitmapPtr +virBitmapNewCopy(virBitmapPtr src) { virBitmapPtr dst; @@ -703,6 +733,7 @@ virBitmapPtr virBitmapNewCopy(virBitmapPtr src) return dst; } + /** * virBitmapNewData: * @data: the data @@ -714,7 +745,9 @@ virBitmapPtr virBitmapNewCopy(virBitmapPtr src) * Returns a pointer to the allocated bitmap or NULL if * memory cannot be allocated. */ -virBitmapPtr virBitmapNewData(const void *data, int len) +virBitmapPtr +virBitmapNewData(const void *data, + int len) { virBitmapPtr bitmap; size_t i, j; @@ -738,6 +771,7 @@ virBitmapPtr virBitmapNewData(const void *data, int len) return bitmap; } + /** * virBitmapToData: * @data: the data @@ -749,7 +783,10 @@ virBitmapPtr virBitmapNewData(const void *data, int len) * * Returns 0 on success, -1 otherwise. */ -int virBitmapToData(virBitmapPtr bitmap, unsigned char **data, int *dataLen) +int +virBitmapToData(virBitmapPtr bitmap, + unsigned char **data, + int *dataLen) { ssize_t len; @@ -768,6 +805,7 @@ int virBitmapToData(virBitmapPtr bitmap, unsigned char **data, int *dataLen) return 0; } + /** * virBitmapToDataBuf: * @bytes: pointer to memory to fill @@ -777,9 +815,10 @@ int virBitmapToData(virBitmapPtr bitmap, unsigned char **data, int *dataLen) * Data consists of sequential bytes, with lower bytes containing * lower bits. */ -void virBitmapToDataBuf(virBitmapPtr bitmap, - unsigned char *bytes, - size_t len) +void +virBitmapToDataBuf(virBitmapPtr bitmap, + unsigned char *bytes, + size_t len) { unsigned long *l; size_t i, j; @@ -797,6 +836,7 @@ void virBitmapToDataBuf(virBitmapPtr bitmap, } } + /** * virBitmapEqual: * @b1: bitmap 1 @@ -807,7 +847,9 @@ void virBitmapToDataBuf(virBitmapPtr bitmap, * Returns true if two bitmaps have exactly the same set of bits set, * otherwise false. */ -bool virBitmapEqual(virBitmapPtr b1, virBitmapPtr b2) +bool +virBitmapEqual(virBitmapPtr b1, + virBitmapPtr b2) { virBitmapPtr tmp; size_t i; @@ -839,11 +881,14 @@ bool virBitmapEqual(virBitmapPtr b1, virBitmapPtr b2) return true; } -size_t virBitmapSize(virBitmapPtr bitmap) + +size_t +virBitmapSize(virBitmapPtr bitmap) { return bitmap->nbits; } + /** * virBitmapSetAll: * @bitmap: the bitmap @@ -863,25 +908,29 @@ void virBitmapSetAll(virBitmapPtr bitmap) -1UL >> (VIR_BITMAP_BITS_PER_UNIT - tail); } + /** * virBitmapClearAll: * @bitmap: the bitmap * * clear all bits in @bitmap. */ -void virBitmapClearAll(virBitmapPtr bitmap) +void +virBitmapClearAll(virBitmapPtr bitmap) { memset(bitmap->map, 0, bitmap->map_len * (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) +bool +virBitmapIsAllSet(virBitmapPtr bitmap) { size_t i; int unusedBits; @@ -906,13 +955,15 @@ bool virBitmapIsAllSet(virBitmapPtr bitmap) return true; } + /** * virBitmapIsAllClear: * @bitmap: the bitmap to check * * check if all bits in @bitmap are clear */ -bool virBitmapIsAllClear(virBitmapPtr bitmap) +bool +virBitmapIsAllClear(virBitmapPtr bitmap) { size_t i; @@ -923,6 +974,7 @@ bool virBitmapIsAllClear(virBitmapPtr bitmap) return true; } + /** * virBitmapNextSetBit: * @bitmap: the bitmap @@ -935,7 +987,8 @@ bool virBitmapIsAllClear(virBitmapPtr bitmap) * Returns the position of the found bit, or -1 if no bit found. */ ssize_t -virBitmapNextSetBit(virBitmapPtr bitmap, ssize_t pos) +virBitmapNextSetBit(virBitmapPtr bitmap, + ssize_t pos) { size_t nl; size_t nb; @@ -963,6 +1016,7 @@ virBitmapNextSetBit(virBitmapPtr bitmap, ssize_t pos) return ffsl(bits) - 1 + nl * VIR_BITMAP_BITS_PER_UNIT; } + /** * virBitmapLastSetBit: * @bitmap: the bitmap @@ -1012,6 +1066,7 @@ virBitmapLastSetBit(virBitmapPtr bitmap) return -1; } + /** * virBitmapNextClearBit: * @bitmap: the bitmap @@ -1024,7 +1079,8 @@ virBitmapLastSetBit(virBitmapPtr bitmap) * Returns the position of the found bit, or -1 if no bit found. */ ssize_t -virBitmapNextClearBit(virBitmapPtr bitmap, ssize_t pos) +virBitmapNextClearBit(virBitmapPtr bitmap, + ssize_t pos) { size_t nl; size_t nb; @@ -1059,6 +1115,7 @@ virBitmapNextClearBit(virBitmapPtr bitmap, ssize_t pos) return ffsl(bits) - 1 + nl * VIR_BITMAP_BITS_PER_UNIT; } + /* Return the number of bits currently set in the map. */ size_t virBitmapCountBits(virBitmapPtr bitmap) @@ -1134,6 +1191,7 @@ virBitmapDataFormat(const void *data, return ret; } + bool virBitmapOverlaps(virBitmapPtr b1, virBitmapPtr b2) @@ -1154,6 +1212,7 @@ virBitmapOverlaps(virBitmapPtr b1, return false; } + /** * virBitmapIntersect: * @a: bitmap, modified to contain result -- 2.15.0
virBitmap code is thoroughly documented. Add docs for the few functions missing them. --- src/util/virbitmap.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 4229aa286e..0973731e3a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -146,6 +146,14 @@ virBitmapFree(virBitmapPtr bitmap) } +/** + * virBitmapCopy: + * @dst: destination bitmap + * @src: source bitmap + * + * Copies contents of @src to @dst. @dst must have the same size as @src. + * Returns -1 if the size is not the same or 0 on success. + */ int virBitmapCopy(virBitmapPtr dst, virBitmapPtr src) @@ -882,6 +890,12 @@ virBitmapEqual(virBitmapPtr b1, } +/** + * virBitmapSize: + * @bitmap: virBitmap to inspect + * + * Returns number of bits @bitmap can store. + */ size_t virBitmapSize(virBitmapPtr bitmap) { @@ -1116,7 +1130,12 @@ virBitmapNextClearBit(virBitmapPtr bitmap, } -/* Return the number of bits currently set in the map. */ +/** + * virBitmapCountBits: + * @bitmap: bitmap to inspect + * + * Return the number of bits currently set in @bitmap. + */ size_t virBitmapCountBits(virBitmapPtr bitmap) { @@ -1192,6 +1211,14 @@ virBitmapDataFormat(const void *data, } +/** + * virBitmapOverlaps: + * @b1: virBitmap to inspect + * @b2: virBitmap to inspect + * + * Returns true if at least one bit with the same index is set both in @b1 and + * @b2. + */ bool virBitmapOverlaps(virBitmapPtr b1, virBitmapPtr b2) -- 2.15.0
The virBitmap code uses VIR_RESIZE_N to do quadratic scaling, which means that along with the number of requested map elements we also need to keep the number of actually allocated elements for the scaling algorithm to work properly. The shrinking code did not fix 'map_alloc' thus virResizeN might actually not expand the bitmap properly after called on a previously shrunk bitmap. --- src/util/virbitmap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 0973731e3a..d1e5a9d1ea 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -1317,5 +1317,6 @@ virBitmapShrink(virBitmapPtr map, return -1; map->map_len = nl; + map->map_alloc = nl; return 0; } -- 2.15.0
The function only reduces the size of the bitmap thus we can use the appropriate shrinking function which also does not have any return value. Since virBitmapShrink now does not return any value callers need to be fixed as well. --- src/conf/domain_conf.c | 3 +-- src/util/virbitmap.c | 20 ++++++++++---------- src/util/virbitmap.h | 2 +- src/util/virresctrl.c | 3 +-- tests/virbitmaptest.c | 6 ++---- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e827b2a810..34aae82f15 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18453,8 +18453,7 @@ virDomainCachetuneDefParse(virDomainDefPtr def, /* We need to limit the bitmap to number of vCPUs. If there's nothing left, * then we can just clean up and return 0 immediately */ - if (virBitmapShrink(vcpus, def->maxvcpus) < 0) - goto cleanup; + virBitmapShrink(vcpus, def->maxvcpus); if (virBitmapIsAllClear(vcpus)) { ret = 0; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index d1e5a9d1ea..82b1f76427 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -1292,15 +1292,16 @@ virBitmapSubtract(virBitmapPtr a, * Reduces the bitmap to size @b. Nothing will change if the size is already * smaller than or equal to @b. */ -int +void virBitmapShrink(virBitmapPtr map, size_t b) { + size_t toremove; size_t nl = 0; size_t nb = 0; if (!map) - return 0; + return; if (map->nbits >= b) map->nbits = b; @@ -1309,14 +1310,13 @@ virBitmapShrink(virBitmapPtr map, nb = map->nbits % VIR_BITMAP_BITS_PER_UNIT; map->map[nl] &= ((1UL << nb) - 1); - nl++; - if (nl == map->map_len) - return 0; + toremove = map->map_alloc - (nl + 1); - if (VIR_REALLOC_N(map->map, nl) < 0) - return -1; + if (toremove == 0) + return; - map->map_len = nl; - map->map_alloc = nl; - return 0; + VIR_SHRINK_N(map->map, map->map_alloc, toremove); + + /* length needs to be fixed as well */ + map->map_len = map->map_alloc; } diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 5a3362a19f..2464814055 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -153,6 +153,6 @@ void virBitmapIntersect(virBitmapPtr a, virBitmapPtr b) void virBitmapSubtract(virBitmapPtr a, virBitmapPtr b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virBitmapShrink(virBitmapPtr map, size_t b); +void virBitmapShrink(virBitmapPtr map, size_t b); #endif diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index ef388757a7..70426199ce 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -941,8 +941,7 @@ virResctrlAllocParseProcessCache(virResctrlInfoPtr resctrl, if (!mask) return -1; - if (virBitmapShrink(mask, resctrl->levels[level]->types[type]->bits) < 0) - goto cleanup; + virBitmapShrink(mask, resctrl->levels[level]->types[type]->bits); if (virResctrlAllocUpdateMask(alloc, level, type, cache_id, mask) < 0) goto cleanup; diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index fffecdf1f6..2fbafc0a76 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -656,12 +656,10 @@ test12(const void *opaque ATTRIBUTE_UNUSED) TEST_MAP(1024, "34,1023"); - if (virBitmapShrink(map, 35) < 0) - goto cleanup; + virBitmapShrink(map, 35); TEST_MAP(35, "34"); - if (virBitmapShrink(map, 34) < 0) - goto cleanup; + virBitmapShrink(map, 34); TEST_MAP(34, ""); ret = 0; -- 2.15.0
Note the fact that the unused portion of the last element in the bitmap needs to be cleared, since we use functions which process only full-size elements and don't really deal with individual bits. --- src/util/virbitmap.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 82b1f76427..0cc5292d8c 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -45,6 +45,10 @@ struct _virBitmap { size_t nbits; size_t map_len; size_t map_alloc; + + /* Note that code below depends on the fact that unused bits of the bitmap + * are not set. Any function decreasing the size of the map needs clear + * bits which don't belong to the bitmap any more. */ unsigned long *map; }; -- 2.15.0
On Mon, Feb 05, 2018 at 02:17:41PM +0100, Peter Krempa wrote:
Peter Krempa (6): util: bitmap: Rename 'max_bit' to 'nbits' util: bitmap: Fix function formatting and spacing util: bitmap: Add comments for functions which don't have them util: bitmap: Fix value of 'map_alloc' when shrinking bitmap util: bitmap: Use VIR_SHRINK_N in virBitmapShrink util: bitmap: Note that shrinking the bitmap requires clearing of unused bits
src/conf/domain_conf.c | 3 +- src/util/virbitmap.c | 213 +++++++++++++++++++++++++++++++++++-------------- src/util/virbitmap.h | 2 +- src/util/virresctrl.c | 3 +- tests/virbitmaptest.c | 6 +- 5 files changed, 157 insertions(+), 70 deletions(-)
-- 2.15.0
ACK series, thanks for the fixes.
participants (2)
-
Martin Kletzander -
Peter Krempa