[libvirt] [PATCHv2 00/13] memory hotplug: Preliminary fixes and cleanups
This series was split out from my memory hotplug series that is not quite ready yet. The aim of this series is to refactor the code that creates commandline for the memory-backend-* qemu object so that it can be later reused for hotplug via monitor. Additionally this series also fixes a bug in NUMA memory initialisation where qemu doesn't support a combination of old and new approach. Version 2 fixes bugs found by first round of review. Patch 4/13 is new in this series. Please see individual patches for changes. (If the change line isn't present the patch was not reviewed) Peter Krempa (13): conf: numatune: Extract code for requesting memory nodeset from formatting test: utils: Add helpers for automatic numbering of test cases util: json: make value object creator universal by supporting adding util: bitmap: Add option to allocate bitmap without reporting error util: json: Add functions to convert JSON arrays from/to virBitmaps util: json: add helper to iterate JSON object key=value pairs qemu: command: Add helper to format -object strings from JSON representation qemu: Extract code to setup memory backing objects qemu: command: Shuffle around formating of alias for memory backend objs qemu: command: Unify values for boolean values when formating memory backends qemu: command: Switch to bytes when formatting size for memory backends qemu: command: Refactor NUMA backend object formatting to use JSON objs qemu: command: Don't combine old and modern NUMA node creation src/conf/numatune_conf.c | 33 +- src/conf/numatune_conf.h | 5 + src/libvirt_private.syms | 7 + src/qemu/qemu_command.c | 547 +++++++++++++++------ src/qemu/qemu_command.h | 4 + src/util/virbitmap.c | 42 +- src/util/virbitmap.h | 1 + src/util/virjson.c | 222 ++++++++- src/util/virjson.h | 17 + tests/Makefile.am | 13 +- tests/qemucommandutiltest.c | 118 +++++ .../qemuxml2argv-hugepages-pages.args | 20 +- .../qemuxml2argv-hugepages-pages2.args | 8 +- .../qemuxml2argv-hugepages-pages3.args | 7 +- .../qemuxml2argv-hugepages-shared.args | 20 +- .../qemuxml2argv-numatune-memnode-no-memory.args | 6 +- .../qemuxml2argv-numatune-memnode.args | 8 +- tests/testutils.c | 52 ++ tests/testutils.h | 3 + 19 files changed, 912 insertions(+), 221 deletions(-) create mode 100644 tests/qemucommandutiltest.c -- 2.2.2
Extract the logic to determine which nodeset has to be used for a domain from the formatting step so that it can be reused separately when the nodeset is used in a different way. --- src/conf/numatune_conf.c | 33 ++++++++++++++++++++++++++------- src/conf/numatune_conf.h | 5 +++++ src/libvirt_private.syms | 1 + 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c index 323cd59..7f322ea 100644 --- a/src/conf/numatune_conf.c +++ b/src/conf/numatune_conf.c @@ -400,13 +400,14 @@ virDomainNumatuneFormatNodeset(virDomainNumatunePtr numatune, cellid)); } + int -virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune, - virBitmapPtr auto_nodeset, - char **mask, - int cellid) +virDomainNumatuneMaybeGetNodeset(virDomainNumatunePtr numatune, + virBitmapPtr auto_nodeset, + virBitmapPtr *retNodeset, + int cellid) { - *mask = NULL; + *retNodeset = NULL; if (!numatune) return 0; @@ -424,8 +425,26 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune, return -1; } - *mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid); - if (!*mask) + *retNodeset = virDomainNumatuneGetNodeset(numatune, auto_nodeset, cellid); + + return 0; +} + + +int +virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune, + virBitmapPtr auto_nodeset, + char **mask, + int cellid) +{ + virBitmapPtr nodeset; + + if (virDomainNumatuneMaybeGetNodeset(numatune, auto_nodeset, &nodeset, + cellid) < 0) + return -1; + + if (nodeset && + !(*mask = virBitmapFormat(nodeset))) return -1; return 0; diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h index 7ca7f97..28c4ce2 100644 --- a/src/conf/numatune_conf.h +++ b/src/conf/numatune_conf.h @@ -70,6 +70,11 @@ virBitmapPtr virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune, virBitmapPtr auto_nodeset, int cellid); +int virDomainNumatuneMaybeGetNodeset(virDomainNumatunePtr numatune, + virBitmapPtr auto_nodeset, + virBitmapPtr *retNodeset, + int cellid); + /* * Formatters */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bd7870f..24fd4da 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -626,6 +626,7 @@ virDomainNumatuneGetNodeset; virDomainNumatuneHasPerNodeBinding; virDomainNumatuneHasPlacementAuto; virDomainNumatuneMaybeFormatNodeset; +virDomainNumatuneMaybeGetNodeset; virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; virDomainNumatuneNodesetIsAvailable; -- 2.2.2
Adding or reordering test cases is usually a pain due to static test case names that are then passed to virtTestRun(). To ease the numbering of test cases, this patch adds two simple helpers that generate the test names according to the order they are run. The test name can be configured via the reset function. This will allow us to freely add test cases in middle of test groups without the need to re-number the rest of test cases. --- Notes: Version 2: - fixed typo in commit message - fixed typo in comment in code - added note that long messages may be truncated tests/testutils.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/testutils.h | 3 +++ 2 files changed, 55 insertions(+) diff --git a/tests/testutils.c b/tests/testutils.c index 9a79f98..f2db4a0 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -986,3 +986,55 @@ virDomainXMLOptionPtr virTestGenericDomainXMLConfInit(void) &virTestGenericPrivateDataCallbacks, NULL); } + + +static int virtTestCounter; +static char virtTestCounterStr[128]; +static char *virtTestCounterPrefixEndOffset; + + +/** + * virtTestCounterReset: + * @prefix: name of the test group + * + * Resets the counter and sets up the test group name to use with + * virtTestCounterNext(). This function is not thread safe. + * + * Note: The buffer for the assembled message is 128 bytes long. Longer test + * case names (including the number index) will be silently truncated. + */ +void +virtTestCounterReset(const char *prefix) +{ + virtTestCounter = 0; + + ignore_value(virStrcpyStatic(virtTestCounterStr, prefix)); + virtTestCounterPrefixEndOffset = strchrnul(virtTestCounterStr, '\0'); +} + + +/** + * virtTestCounterNext: + * + * This function is designed to ease test creation and reordering by adding + * a way to do automagic test case numbering. + * + * Returns string consisting of test name prefix configured via + * virtTestCounterReset() and a number that increments in every call of this + * function. This function is not thread safe. + * + * Note: The buffer for the assembled message is 128 bytes long. Longer test + * case names (including the number index) will be silently truncated. + */ +const char +*virtTestCounterNext(void) +{ + size_t len = ARRAY_CARDINALITY(virtTestCounterStr); + + /* calculate length of the rest of the string */ + len -= (virtTestCounterPrefixEndOffset - virtTestCounterStr); + + snprintf(virtTestCounterPrefixEndOffset, len, "%d", ++virtTestCounter); + + return virtTestCounterStr; +} diff --git a/tests/testutils.h b/tests/testutils.h index d78818d..155b30f 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -82,6 +82,9 @@ char *virtTestLogContentAndReset(void); void virtTestQuiesceLibvirtErrors(bool always); +void virtTestCounterReset(const char *prefix); +const char *virtTestCounterNext(void); + int virtTestMain(int argc, char **argv, int (*func)(void)); -- 2.2.2
To allow constructing of value objects stepwise explode the helper into separate steps and allow appending into existing value objects. --- Notes: Version 2: - Fix freeing of the created JSON object on error -> use JSON freeing func src/libvirt_private.syms | 2 ++ src/util/virjson.c | 76 +++++++++++++++++++++++++++++++----------------- src/util/virjson.h | 5 ++++ 3 files changed, 57 insertions(+), 26 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 24fd4da..7af8ee0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1534,6 +1534,8 @@ virJSONValueNewNumberUlong; virJSONValueNewObject; virJSONValueNewString; virJSONValueNewStringLen; +virJSONValueObjectAdd; +virJSONValueObjectAddVArgs; virJSONValueObjectAppend; virJSONValueObjectAppendBoolean; virJSONValueObjectAppendNull; diff --git a/src/util/virjson.c b/src/util/virjson.c index 9f2e1cf..fe3357a 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -64,12 +64,11 @@ struct _virJSONParser { /** - * virJSONValueObjectCreateVArgs: - * @obj: returns the created JSON object - * @...: a key-value argument pairs, terminated by NULL + * virJSONValueObjectAddVArgs: + * @obj: JSON object to add the values to + * @args: a key-value argument pairs, terminated by NULL * - * Creates a JSON value object filled with key-value pairs supplied as variable - * argument list. + * Adds the key-value pairs supplied as variable argument list to @obj. * * Keys look like s:name the first letter is a type code: * Explanation of type codes: @@ -104,22 +103,17 @@ struct _virJSONParser { * The value corresponds to the selected type. * * Returns -1 on error. 1 on success, if at least one key:pair was valid 0 - * in case of no error but nothing was filled (@obj will be NULL). + * in case of no error but nothing was filled. */ int -virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) +virJSONValueObjectAddVArgs(virJSONValuePtr obj, + va_list args) { - virJSONValuePtr jargs = NULL; char type; char *key; int ret = -1; int rc; - *obj = NULL; - - if (!(jargs = virJSONValueNewObject())) - goto cleanup; - while ((key = va_arg(args, char *)) != NULL) { if (strlen(key) < 3) { @@ -146,7 +140,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) key); goto cleanup; } - rc = virJSONValueObjectAppendString(jargs, key, val); + rc = virJSONValueObjectAppendString(obj, key, val); } break; case 'z': @@ -165,7 +159,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) if (!val && (type == 'z' || type == 'y')) continue; - rc = virJSONValueObjectAppendNumberInt(jargs, key, val); + rc = virJSONValueObjectAppendNumberInt(obj, key, val); } break; case 'p': @@ -175,7 +169,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) if (!val && type == 'p') continue; - rc = virJSONValueObjectAppendNumberUint(jargs, key, val); + rc = virJSONValueObjectAppendNumberUint(obj, key, val); } break; case 'Z': @@ -194,7 +188,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) if (!val && (type == 'Z' || type == 'Y')) continue; - rc = virJSONValueObjectAppendNumberLong(jargs, key, val); + rc = virJSONValueObjectAppendNumberLong(obj, key, val); } break; case 'P': @@ -209,12 +203,12 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) if (!val && type == 'P') continue; - rc = virJSONValueObjectAppendNumberLong(jargs, key, val); + rc = virJSONValueObjectAppendNumberLong(obj, key, val); } break; case 'd': { double val = va_arg(args, double); - rc = virJSONValueObjectAppendNumberDouble(jargs, key, val); + rc = virJSONValueObjectAppendNumberDouble(obj, key, val); } break; case 'B': @@ -224,11 +218,11 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) if (!val && type == 'B') continue; - rc = virJSONValueObjectAppendBoolean(jargs, key, val); + rc = virJSONValueObjectAppendBoolean(obj, key, val); } break; case 'n': { - rc = virJSONValueObjectAppendNull(jargs, key); + rc = virJSONValueObjectAppendNull(obj, key); } break; case 'A': @@ -245,7 +239,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) goto cleanup; } - rc = virJSONValueObjectAppend(jargs, key, val); + rc = virJSONValueObjectAppend(obj, key, val); } break; default: @@ -259,17 +253,47 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) } /* verify that we added at least one key-value pair */ - if (virJSONValueObjectKeysNumber(jargs) == 0) { + if (virJSONValueObjectKeysNumber(obj) == 0) { ret = 0; goto cleanup; } - *obj = jargs; - jargs = NULL; ret = 1; cleanup: - virJSONValueFree(jargs); + return ret; +} + + +int +virJSONValueObjectAdd(virJSONValuePtr obj, ...) +{ + va_list args; + int ret; + + va_start(args, obj); + ret = virJSONValueObjectAddVArgs(obj, args); + va_end(args); + + return ret; +} + + +int +virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, + va_list args) +{ + int ret; + + if (!(*obj = virJSONValueNewObject())) + return -1; + + /* free the object on error, or if no value objects were added */ + if ((ret = virJSONValueObjectAddVArgs(*obj, args)) <= 0) { + virJSONValueFree(*obj); + *obj = NULL; + } + return ret; } diff --git a/src/util/virjson.h b/src/util/virjson.h index c0aefba..fc05ad9 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -85,6 +85,11 @@ int virJSONValueObjectCreate(virJSONValuePtr *obj, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL; int virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args) ATTRIBUTE_NONNULL(1); +int virJSONValueObjectAdd(virJSONValuePtr obj, ...) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL; +int virJSONValueObjectAddVArgs(virJSONValuePtr obj, va_list args) + ATTRIBUTE_NONNULL(1); + virJSONValuePtr virJSONValueNewString(const char *data); virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length); -- 2.2.2
The virBitmapNew() function reports only OOM errors. Split out the internals into a "quiet" function and add a wrapper that reports the error. --- Notes: Version 2: - new in series src/libvirt_private.syms | 1 + src/util/virbitmap.c | 42 +++++++++++++++++++++++++++++++----------- src/util/virbitmap.h | 1 + 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7af8ee0..cc1ffed 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1048,6 +1048,7 @@ virBitmapLastSetBit; virBitmapNew; virBitmapNewCopy; virBitmapNewData; +virBitmapNewQuiet; virBitmapNextClearBit; virBitmapNextSetBit; virBitmapOverlaps; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index d5b0035..b531be5 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -54,31 +54,29 @@ struct _virBitmap { /** - * virBitmapNew: + * virBitmapNewQuiet: * @size: number of bits * * Allocate a bitmap capable of containing @size bits. * - * Returns a pointer to the allocated bitmap or NULL if - * memory cannot be allocated. + * Returns a pointer to the allocated bitmap or NULL if memory cannot be + * allocated. Does not report libvirt errors. */ -virBitmapPtr virBitmapNew(size_t size) +virBitmapPtr +virBitmapNewQuiet(size_t size) { virBitmapPtr bitmap; size_t sz; - if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size || size == 0) { - virReportOOMError(); + if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size || size == 0) return NULL; - } - sz = (size + VIR_BITMAP_BITS_PER_UNIT - 1) / - VIR_BITMAP_BITS_PER_UNIT; + sz = (size + VIR_BITMAP_BITS_PER_UNIT - 1) / VIR_BITMAP_BITS_PER_UNIT; - if (VIR_ALLOC(bitmap) < 0) + if (VIR_ALLOC_QUIET(bitmap) < 0) return NULL; - if (VIR_ALLOC_N(bitmap->map, sz) < 0) { + if (VIR_ALLOC_N_QUIET(bitmap->map, sz) < 0) { VIR_FREE(bitmap); return NULL; } @@ -88,6 +86,28 @@ virBitmapPtr virBitmapNew(size_t size) return bitmap; } + +/** + * virBitmapNew: + * @size: number of bits + * + * Allocate a bitmap capable of containing @size bits. + * + * Returns a pointer to the allocated bitmap or NULL if memory cannot be + * allocated. Reports libvirt errors. + */ +virBitmapPtr +virBitmapNew(size_t size) +{ + virBitmapPtr ret; + + if (!(ret = virBitmapNewQuiet(size))) + virReportOOMError(); + + return ret; +} + + /** * virBitmapFree: * @bitmap: previously allocated bitmap diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index a347f0a..136e819 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -35,6 +35,7 @@ typedef virBitmap *virBitmapPtr; /* * Allocate a bitmap capable of containing @size bits. */ +virBitmapPtr virBitmapNewQuiet(size_t size) ATTRIBUTE_RETURN_CHECK; virBitmapPtr virBitmapNew(size_t size) ATTRIBUTE_RETURN_CHECK; /* -- 2.2.2
To be able to easily represent nodesets and other data stored in virBitmaps in libvirt, this patch introduces a set of helpers that allow to convert the bitmap to and from JSON value objects. --- Notes: Version 2: - consistently don't report any errors, as this function is being used in probe-and fallback mode src/libvirt_private.syms | 2 + src/util/virjson.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virjson.h | 4 ++ 3 files changed, 119 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc1ffed..ab32eaa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1516,6 +1516,7 @@ virJSONValueArrayGet; virJSONValueArraySize; virJSONValueFree; virJSONValueFromString; +virJSONValueGetArrayAsBitmap; virJSONValueGetBoolean; virJSONValueGetNumberDouble; virJSONValueGetNumberInt; @@ -1525,6 +1526,7 @@ virJSONValueGetNumberUlong; virJSONValueGetString; virJSONValueIsNull; virJSONValueNewArray; +virJSONValueNewArrayFromBitmap; virJSONValueNewBoolean; virJSONValueNewNull; virJSONValueNewNumberDouble; diff --git a/src/util/virjson.c b/src/util/virjson.c index fe3357a..1bb2653 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -99,6 +99,8 @@ struct _virJSONParser { * * a: json object, must be non-NULL * A: json object, omitted if NULL + * m: a bitmap represented as a JSON array, must be non-NULL + * M: a bitmap represented as a JSON array, omitted if NULL * * The value corresponds to the selected type. * @@ -242,6 +244,28 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj, rc = virJSONValueObjectAppend(obj, key, val); } break; + case 'M': + case 'm': { + virBitmapPtr map = va_arg(args, virBitmapPtr); + virJSONValuePtr jsonMap; + + if (!map) { + if (type == 'M') + continue; + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("argument key '%s' must not have null value"), + key); + goto cleanup; + } + + if (!(jsonMap = virJSONValueNewArrayFromBitmap(map))) + goto cleanup; + + if ((rc = virJSONValueObjectAppend(obj, key, jsonMap)) < 0) + virJSONValueFree(jsonMap); + } break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported data type '%c' for arg '%s'"), type, key - 2); @@ -943,6 +967,95 @@ virJSONValueGetBoolean(virJSONValuePtr val, } +/** + * virJSONValueGetArrayAsBitmap: + * @val: JSON array to convert to bitmap + * @bitmap: New bitmap is allocated filled and returned via this argument + * + * Attempts a conversion of a JSON array to a bitmap. The members of the array + * must be non-negative integers for the conversion to succeed. This function + * does not report libvirt errors so that it can be used to probe that the + * array can be represented as a bitmap. + * + * Returns 0 on success and fills @bitmap; -1 on error and @bitmap is set to + * NULL. + */ +int +virJSONValueGetArrayAsBitmap(const virJSONValue *val, + virBitmapPtr *bitmap) +{ + int ret = -1; + virJSONValuePtr elem; + size_t i; + unsigned long long *elems = NULL; + unsigned long long maxelem = 0; + + *bitmap = NULL; + + if (val->type != VIR_JSON_TYPE_ARRAY) + return -1; + + if (VIR_ALLOC_N_QUIET(elems, val->data.array.nvalues) < 0) + return -1; + + /* first pass converts array members to numbers and finds the maximum */ + for (i = 0; i < val->data.array.nvalues; i++) { + elem = val->data.array.values[i]; + + if (elem->type != VIR_JSON_TYPE_NUMBER || + virStrToLong_ullp(elem->data.number, NULL, 10, &elems[i]) < 0) + goto cleanup; + + if (elems[i] > maxelem) + maxelem = elems[i]; + } + + if (!(*bitmap = virBitmapNewQuiet(maxelem + 1))) + goto cleanup; + + /* second pass sets the correct bits in the map */ + for (i = 0; i < val->data.array.nvalues; i++) + ignore_value(virBitmapSetBit(*bitmap, elems[i])); + + ret = 0; + + cleanup: + VIR_FREE(elems); + + return ret; +} + + +virJSONValuePtr +virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap) +{ + virJSONValuePtr ret; + ssize_t pos = -1; + + if (!(ret = virJSONValueNewArray())) + return NULL; + + if (!bitmap) + return ret; + + while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) { + virJSONValuePtr newelem; + + if (!(newelem = virJSONValueNewNumberLong(pos)) || + virJSONValueArrayAppend(ret, newelem) < 0) { + virJSONValueFree(newelem); + goto error; + } + } + + return ret; + + error: + virJSONValueFree(ret); + return NULL; +} + + int virJSONValueIsNull(virJSONValuePtr val) { diff --git a/src/util/virjson.h b/src/util/virjson.h index fc05ad9..57010b0 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -25,6 +25,7 @@ # define __VIR_JSON_H_ # include "internal.h" +# include "virbitmap.h" # include <stdarg.h> @@ -102,6 +103,7 @@ virJSONValuePtr virJSONValueNewBoolean(int boolean); virJSONValuePtr virJSONValueNewNull(void); virJSONValuePtr virJSONValueNewArray(void); virJSONValuePtr virJSONValueNewObject(void); +virJSONValuePtr virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap); int virJSONValueObjectAppend(virJSONValuePtr object, const char *key, virJSONValuePtr value); int virJSONValueArrayAppend(virJSONValuePtr object, virJSONValuePtr value); @@ -125,6 +127,8 @@ int virJSONValueGetNumberLong(virJSONValuePtr object, long long *value); int virJSONValueGetNumberUlong(virJSONValuePtr object, unsigned long long *value); int virJSONValueGetNumberDouble(virJSONValuePtr object, double *value); int virJSONValueGetBoolean(virJSONValuePtr object, bool *value); +int virJSONValueGetArrayAsBitmap(const virJSONValue *val, virBitmapPtr *bitmap) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virJSONValueIsNull(virJSONValuePtr object); const char *virJSONValueObjectGetString(virJSONValuePtr object, const char *key); -- 2.2.2
This helper eases iterating all key=value pairs stored in a JSON object. Usually we pick only certain known keys from a JSON object, but this will allow to walk complete objects and have the callback act on those. --- Notes: Version 2:- - fixed typo in commit message - ACKed by Eric src/libvirt_private.syms | 1 + src/util/virjson.c | 33 +++++++++++++++++++++++++++++++++ src/util/virjson.h | 8 ++++++++ 3 files changed, 42 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ab32eaa..cf5ccaf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1550,6 +1550,7 @@ virJSONValueObjectAppendNumberUlong; virJSONValueObjectAppendString; virJSONValueObjectCreate; virJSONValueObjectCreateVArgs; +virJSONValueObjectForeachKeyValue; virJSONValueObjectGet; virJSONValueObjectGetBoolean; virJSONValueObjectGetKey; diff --git a/src/util/virjson.c b/src/util/virjson.c index 1bb2653..c8d761f 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1200,6 +1200,39 @@ virJSONValueObjectIsNull(virJSONValuePtr object, } +/** + * virJSONValueObjectForeachKeyValue: + * @object: JSON object to iterate + * @cb: callback to call on key-value pairs contained in the object + * @opaque: generic data for the callback + * + * Iterates all key=value pairs in @object. Iteration breaks if @cb returns + * negative value. + * + * Returns 0 if all elements were iterated, -2 if @cb returned negative value + * during iteration and -1 on generic errors. + */ +int +virJSONValueObjectForeachKeyValue(virJSONValuePtr object, + virJSONValueObjectIteratorFunc cb, + void *opaque) +{ + size_t i; + + if (object->type != VIR_JSON_TYPE_OBJECT) + return -1; + + for (i = 0; i < object->data.object.npairs; i++) { + virJSONObjectPairPtr elem = object->data.object.pairs + i; + + if (cb(elem->key, elem->value, opaque) < 0) + return -2; + } + + return 0; +} + + #if WITH_YAJL static int virJSONParserInsertValue(virJSONParserPtr parser, diff --git a/src/util/virjson.h b/src/util/virjson.h index 57010b0..9bb7461 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -157,4 +157,12 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring); char *virJSONValueToString(virJSONValuePtr object, bool pretty); +typedef int (*virJSONValueObjectIteratorFunc)(const char *key, + const virJSONValue *value, + void *opaque); + +int virJSONValueObjectForeachKeyValue(virJSONValuePtr object, + virJSONValueObjectIteratorFunc cb, + void *opaque); + #endif /* __VIR_JSON_H_ */ -- 2.2.2
Unlike -device, qemu uses a JSON object to add backend "objects" via the monitor rather than the string that would be passed on the commandline. To be able to reuse code parts that configure backends for various devices, this patch adds a helper that will allow generating the command line representations from the JSON property object. --- src/qemu/qemu_command.c | 111 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 ++ tests/Makefile.am | 13 ++++- tests/qemucommandutiltest.c | 118 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 tests/qemucommandutiltest.c diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 100deed..6f298ac 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -416,6 +416,117 @@ qemuDomainSupportsNetdev(virDomainDefPtr def, return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV); } + +static int +qemuBuildObjectCommandLinePropsInternal(const char *key, + const virJSONValue *value, + virBufferPtr buf, + bool nested) +{ + virJSONValuePtr elem; + virBitmapPtr bitmap = NULL; + ssize_t pos = -1; + ssize_t end; + size_t i; + + switch ((virJSONType) value->type) { + case VIR_JSON_TYPE_STRING: + virBufferAsprintf(buf, ",%s=%s", key, value->data.string); + break; + + case VIR_JSON_TYPE_NUMBER: + virBufferAsprintf(buf, ",%s=%s", key, value->data.number); + break; + + case VIR_JSON_TYPE_BOOLEAN: + if (value->data.boolean) + virBufferAsprintf(buf, ",%s=yes", key); + else + virBufferAsprintf(buf, ",%s=no", key); + + break; + + case VIR_JSON_TYPE_ARRAY: + if (nested) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("nested -object property arrays are not supported")); + return -1; + } + + if (virJSONValueGetArrayAsBitmap(value, &bitmap) == 0) { + while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) { + if ((end = virBitmapNextClearBit(bitmap, pos)) < 0) + end = virBitmapLastSetBit(bitmap) + 1; + + if (end - 1 > pos) { + virBufferAsprintf(buf, ",%s=%zd-%zd", key, pos, end - 1); + pos = end; + } else { + virBufferAsprintf(buf, ",%s=%zd", key, pos); + } + } + } else { + /* fallback, treat the array as a non-bitmap, adding the key + * for each member */ + for (i = 0; i < virJSONValueArraySize(value); i++) { + elem = virJSONValueArrayGet((virJSONValuePtr)value, i); + + /* recurse to avoid duplicating code */ + if (qemuBuildObjectCommandLinePropsInternal(key, elem, buf, + true) < 0) + return -1; + } + } + break; + + case VIR_JSON_TYPE_OBJECT: + case VIR_JSON_TYPE_NULL: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NULL and OBJECT JSON types can't be converted to " + "commandline string")); + return -1; + } + + virBitmapFree(bitmap); + return 0; +} + + +static int +qemuBuildObjectCommandLineProps(const char *key, + const virJSONValue *value, + void *opaque) +{ + return qemuBuildObjectCommandLinePropsInternal(key, value, opaque, false); +} + + +char * +qemuBuildObjectCommandlineFromJSON(const char *type, + const char *alias, + virJSONValuePtr props) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *ret = NULL; + + virBufferAsprintf(&buf, "%s,id=%s", type, alias); + + if (virJSONValueObjectForeachKeyValue(props, + qemuBuildObjectCommandLineProps, + &buf) < 0) + goto cleanup; + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + ret = virBufferContentAndReset(&buf); + + cleanup: + virBufferFreeAndReset(&buf); + return ret; +} + + /** * qemuOpenVhostNet: * @def: domain definition diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 8eaf1e4..ae36bd8 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -69,6 +69,10 @@ struct _qemuBuildCommandLineCallbacks { extern qemuBuildCommandLineCallbacks buildCommandLineCallbacks; +char *qemuBuildObjectCommandlineFromJSON(const char *type, + const char *alias, + virJSONValuePtr props); + virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virQEMUDriverPtr driver, virDomainDefPtr def, diff --git a/tests/Makefile.am b/tests/Makefile.am index 1d838a5..938270c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -237,7 +237,8 @@ if WITH_QEMU test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ - qemuagenttest qemucapabilitiestest qemucaps2xmltest + qemuagenttest qemucapabilitiestest qemucaps2xmltest \ + qemucommandutiltest endif WITH_QEMU if WITH_LXC @@ -586,6 +587,14 @@ qemucapabilitiestest_SOURCES = \ qemucapabilitiestest_LDADD = libqemumonitortestutils.la \ $(qemu_LDADDS) $(LDADDS) +qemucommandutiltest_SOURCES = \ + qemucommandutiltest.c \ + testutils.c testutils.h \ + testutilsqemu.c testutilsqemu.h \ + $(NULL) +qemucommandutiltest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS) + qemucaps2xmltest_SOURCES = \ qemucaps2xmltest.c \ testutils.c testutils.h \ @@ -616,7 +625,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemumonitortest.c testutilsqemu.c testutilsqemu.h \ qemumonitorjsontest.c qemuhotplugtest.c \ qemuagenttest.c qemucapabilitiestest.c \ - qemucaps2xmltest.c \ + qemucaps2xmltest.c qemucommandutiltest.c \ $(QEMUMONITORTESTUTILS_SOURCES) endif ! WITH_QEMU diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c new file mode 100644 index 0000000..8c52f02 --- /dev/null +++ b/tests/qemucommandutiltest.c @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2015 Red Hat, Inc. + * + * 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 "qemu/qemu_command.h" +#include "util/virjson.h" +#include "testutils.h" +#include "testutilsqemu.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +typedef struct +{ + const char *props; + const char *expectprops; +} testQemuCommandBuildObjectFromJSONData; + +static int +testQemuCommandBuildObjectFromJSON(const void *opaque) +{ + const testQemuCommandBuildObjectFromJSONData *data = opaque; + virJSONValuePtr val = NULL; + char *expect = NULL; + char *result = NULL; + int ret = -1; + + if (!(val = virJSONValueFromString(data->props))) { + fprintf(stderr, "Failed to parse JSON string '%s'", data->props); + return -1; + } + + if (virAsprintf(&expect, "testobject,id=testalias%s%s", + data->expectprops ? "," : "", + data->expectprops ? data->expectprops : "") < 0) + return -1; + + result = qemuBuildObjectCommandlineFromJSON("testobject", + "testalias", + val); + + if (STRNEQ_NULLABLE(expect, result)) { + fprintf(stderr, "\nFailed to create object string. " + "\nExpected:\n'%s'\nGot:\n'%s'", + NULLSTR(expect), NULLSTR(result)); + goto cleanup; + } + + ret = 0; + cleanup: + virJSONValueFree(val); + VIR_FREE(result); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + testQemuCommandBuildObjectFromJSONData data1; + +#if !WITH_YAJL + fputs("libvirt not compiled with yajl, skipping this test\n", stderr); + return EXIT_AM_SKIP; +#endif + + virtTestCounterReset("testQemuCommandBuildObjectFromJSON"); + +#define DO_TEST_COMMAND_OBJECT_FROM_JSON(PROPS, EXPECT) \ + do { \ + data1.props = PROPS; \ + data1.expectprops = EXPECT; \ + if (virtTestRun(virtTestCounterNext(), \ + testQemuCommandBuildObjectFromJSON, \ + &data1) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_COMMAND_OBJECT_FROM_JSON("{}", NULL); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"string\":\"qwer\"}", "string=qwer"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"number\":1234}", "number=1234"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"boolean\":true}", "boolean=yes"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"boolean\":false}", "boolean=no"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"bitmap\":[]}", NULL); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"bitmap\":[0]}", "bitmap=0"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"bitmap\":[1,3,5]}", + "bitmap=1,bitmap=3,bitmap=5"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"bitmap\":[0,1,2,3]}", "bitmap=0-3"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"bitmap\":[1,2,3,5]}", + "bitmap=1-3,bitmap=5"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"bitmap\":[1,2,3,5,7,8,9]}", + "bitmap=1-3,bitmap=5,bitmap=7-9"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"array\":[\"bleah\",\"qwerty\",1]}", + "array=bleah,array=qwerty,array=1"); + DO_TEST_COMMAND_OBJECT_FROM_JSON("{\"boolean\":true,\"hyphen-name\":1234,\"some_string\":\"bleah\"}", + "boolean=yes,hyphen-name=1234,some_string=bleah"); + + return ret; + +} + +VIRT_TEST_MAIN(mymain) -- 2.2.2
Extract the memory backend device code into a separate function so that it can be later easily refactored and reused. Few small changes for future reusability, namely: - new (currently unused) parameter for user specified page size - size of the memory is specified in kibibytes, divided up in the function - new (currently unused) parameter for user specifed source nodeset - option to enforce capability check --- src/qemu/qemu_command.c | 382 +++++++++++++++++++++++++++++++----------------- 1 file changed, 250 insertions(+), 132 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f298ac..c65f187 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4502,6 +4502,243 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } +/** + * qemuBuildMemoryBackendStr: + * @size: size of the memory device in kibibytes + * @pagesize: size of the requested memory page in KiB, 0 for default + * @guestNode: NUMA node in the guest that the memory object will be attached to + * @hostNodes: map of host nodes to alloc the memory in, NULL for default + * @autoNodeset: fallback nodeset in case of automatic numa placement + * @def: domain definition object + * @qemuCaps: qemu capabilities object + * @cfg: qemu driver config object + * @aliasPrefix: prefix string of the alias (to allow for multiple frontents) + * @id: index of the device (to construct the alias) + * @backendStr: returns the object string + * + * Formats the configuration string for the memory device backend according + * to the configuration. @pagesize and @hostNodes can be used to override the + * default source configuration, both are optional. + * + * Returns 0 on success, 1 if only the implicit memory-device-ram with no + * other configuration was used (to detect legacy configurations). Returns + * -1 in case of an error. + */ +static int +qemuBuildMemoryBackendStr(unsigned long long size, + unsigned long long pagesize, + int guestNode, + virBitmapPtr userNodeset, + virBitmapPtr autoNodeset, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg, + const char *aliasPrefix, + size_t id, + char **backendStr, + bool force) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainHugePagePtr master_hugepage = NULL; + virDomainHugePagePtr hugepage = NULL; + virDomainNumatuneMemMode mode; + const long system_page_size = sysconf(_SC_PAGESIZE) / 1024; + virMemAccess memAccess = def->cpu->cells[guestNode].memAccess; + size_t i; + char *mem_path = NULL; + char *nodemask = NULL; + char *tmpmask = NULL, *next = NULL; + int ret = -1; + + *backendStr = NULL; + + mode = virDomainNumatuneGetMode(def->numatune, guestNode); + + if (pagesize == 0 || pagesize != system_page_size) { + /* Find the huge page size we want to use */ + for (i = 0; i < def->mem.nhugepages; i++) { + bool thisHugepage = false; + + hugepage = &def->mem.hugepages[i]; + + if (!hugepage->nodemask) { + master_hugepage = hugepage; + continue; + } + + if (virBitmapGetBit(hugepage->nodemask, guestNode, + &thisHugepage) < 0) { + /* Ignore this error. It's not an error after all. Well, + * the nodemask for this <page/> can contain lower NUMA + * nodes than we are querying in here. */ + continue; + } + + if (thisHugepage) { + /* Hooray, we've found the page size */ + break; + } + } + + if (i == def->mem.nhugepages) { + /* We have not found specific huge page to be used with this + * NUMA node. Use the generic setting then (<page/> without any + * @nodemask) if possible. */ + hugepage = master_hugepage; + } + + if (hugepage) + pagesize = hugepage->size; + + if (hugepage && hugepage->size == system_page_size) { + /* However, if user specified to use "huge" page + * of regular system page size, it's as if they + * hasn't specified any huge pages at all. */ + hugepage = NULL; + } + } + + if (hugepage) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support hugepage memory backing")); + goto cleanup; + } + + /* Now lets see, if the huge page we want to use is even mounted + * and ready to use */ + for (i = 0; i < cfg->nhugetlbfs; i++) { + if (cfg->hugetlbfs[i].size == hugepage->size) + break; + } + + if (i == cfg->nhugetlbfs) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find any usable hugetlbfs mount for %llu KiB"), + pagesize); + goto cleanup; + } + + VIR_FREE(mem_path); + if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) + goto cleanup; + + virBufferAsprintf(&buf, "memory-backend-file,prealloc=yes,mem-path=%s", + mem_path); + + switch (memAccess) { + case VIR_MEM_ACCESS_SHARED: + virBufferAddLit(&buf, ",share=on"); + break; + + case VIR_MEM_ACCESS_PRIVATE: + virBufferAddLit(&buf, ",share=off"); + break; + + case VIR_MEM_ACCESS_DEFAULT: + case VIR_MEM_ACCESS_LAST: + break; + } + } else { + if (memAccess) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Shared memory mapping is supported " + "only with hugepages")); + goto cleanup; + } + + virBufferAddLit(&buf, "memory-backend-ram"); + } + + virBufferAsprintf(&buf, ",size=%lluM,id=%s%zu", size / 1024, + aliasPrefix, id); + + if (userNodeset) { + if (!(nodemask = virBitmapFormat(userNodeset))) + goto cleanup; + } else { + if (virDomainNumatuneMaybeFormatNodeset(def->numatune, autoNodeset, + &nodemask, guestNode) < 0) + goto cleanup; + } + + if (nodemask) { + if (strchr(nodemask, ',') && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disjoint NUMA node ranges are not supported " + "with this QEMU")); + goto cleanup; + } + + for (tmpmask = nodemask; tmpmask; tmpmask = next) { + if ((next = strchr(tmpmask, ','))) + *(next++) = '\0'; + virBufferAddLit(&buf, ",host-nodes="); + virBufferAdd(&buf, tmpmask, -1); + } + + virBufferAsprintf(&buf, ",policy=%s", qemuNumaPolicyTypeToString(mode)); + } + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + *backendStr = virBufferContentAndReset(&buf); + + if (!hugepage) { + if ((nodemask || force) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support the " + "memory-backend-ram object")); + goto cleanup; + } + + /* report back that using the new backend is not necessary to achieve + * the desired configuration */ + if (!nodemask) { + ret = 1; + goto cleanup; + } + } + + ret = 0; + + cleanup: + virBufferFreeAndReset(&buf); + VIR_FREE(nodemask); + VIR_FREE(mem_path); + + return ret; +} + + +static int +qemuBuildMemoryCellBackendStr(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virQEMUDriverConfigPtr cfg, + size_t cell, + virBitmapPtr auto_nodeset, + char **backendStr) +{ + int ret; + + ret = qemuBuildMemoryBackendStr(def->cpu->cells[cell].mem, 0, cell, + NULL, auto_nodeset, + def, qemuCaps, cfg, + "ram-node", cell, + backendStr, false); + + if (ret == 1) { + VIR_FREE(*backendStr); + return 0; + } + + return ret; +} + + char * qemuBuildNicStr(virDomainNetDefPtr net, const char *prefix, @@ -6796,14 +7033,12 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virDomainDefPtr def, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - virBitmapPtr nodeset) + virBitmapPtr auto_nodeset) { - size_t i, j; + size_t i; virBuffer buf = VIR_BUFFER_INITIALIZER; - virDomainHugePagePtr master_hugepage = NULL; char *cpumask = NULL, *tmpmask = NULL, *next = NULL; - char *nodemask = NULL; - char *mem_path = NULL; + char *backendStr = NULL; int ret = -1; const long system_page_size = sysconf(_SC_PAGESIZE) / 1024; @@ -6825,7 +7060,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, goto cleanup; } - if (!virDomainNumatuneNodesetIsAvailable(def->numatune, nodeset)) + if (!virDomainNumatuneNodesetIsAvailable(def->numatune, auto_nodeset)) goto cleanup; for (i = 0; i < def->mem.nhugepages; i++) { @@ -6853,13 +7088,11 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } for (i = 0; i < def->cpu->ncells; i++) { - virDomainHugePagePtr hugepage = NULL; unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); def->cpu->cells[i].mem = cellmem * 1024; - virMemAccess memAccess = def->cpu->cells[i].memAccess; VIR_FREE(cpumask); - VIR_FREE(nodemask); + VIR_FREE(backendStr); if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask))) goto cleanup; @@ -6872,133 +7105,19 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, goto cleanup; } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { - virDomainNumatuneMemMode mode; - const char *policy = NULL; - - mode = virDomainNumatuneGetMode(def->numatune, i); - policy = qemuNumaPolicyTypeToString(mode); - - /* Find the huge page size we want to use */ - for (j = 0; j < def->mem.nhugepages; j++) { - bool thisHugepage = false; - - hugepage = &def->mem.hugepages[j]; - - if (!hugepage->nodemask) { - master_hugepage = hugepage; - continue; - } - - if (virBitmapGetBit(hugepage->nodemask, i, &thisHugepage) < 0) { - /* Ignore this error. It's not an error after all. Well, - * the nodemask for this <page/> can contain lower NUMA - * nodes than we are querying in here. */ - continue; - } - - if (thisHugepage) { - /* Hooray, we've found the page size */ - break; - } - } - - if (j == def->mem.nhugepages) { - /* We have not found specific huge page to be used with this - * NUMA node. Use the generic setting then (<page/> without any - * @nodemask) if possible. */ - hugepage = master_hugepage; - } - - if (hugepage && hugepage->size == system_page_size) { - /* However, if user specified to use "huge" page - * of regular system page size, it's as if they - * hasn't specified any huge pages at all. */ - hugepage = NULL; - } - - if (hugepage) { - /* Now lets see, if the huge page we want to use is even mounted - * and ready to use */ - - for (j = 0; j < cfg->nhugetlbfs; j++) { - if (cfg->hugetlbfs[j].size == hugepage->size) - break; - } - - if (j == cfg->nhugetlbfs) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find any usable hugetlbfs mount for %llu KiB"), - hugepage->size); - goto cleanup; - } - - VIR_FREE(mem_path); - if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[j]))) - goto cleanup; - - virBufferAsprintf(&buf, - "memory-backend-file,prealloc=yes,mem-path=%s", - mem_path); - - switch (memAccess) { - case VIR_MEM_ACCESS_SHARED: - virBufferAddLit(&buf, ",share=on"); - break; - - case VIR_MEM_ACCESS_PRIVATE: - virBufferAddLit(&buf, ",share=off"); - break; - - case VIR_MEM_ACCESS_DEFAULT: - case VIR_MEM_ACCESS_LAST: - break; - } - - } else { - if (memAccess) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Shared memory mapping is supported " - "only with hugepages")); - goto cleanup; - } - virBufferAddLit(&buf, "memory-backend-ram"); - } - - virBufferAsprintf(&buf, ",size=%lluM,id=ram-node%zu", cellmem, i); - - if (virDomainNumatuneMaybeFormatNodeset(def->numatune, nodeset, - &nodemask, i) < 0) + if (qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i, + auto_nodeset, &backendStr) < 0) goto cleanup; - if (nodemask) { - if (strchr(nodemask, ',') && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disjoint NUMA node ranges are not supported " - "with this QEMU")); - goto cleanup; - } - - for (tmpmask = nodemask; tmpmask; tmpmask = next) { - if ((next = strchr(tmpmask, ','))) - *(next++) = '\0'; - virBufferAddLit(&buf, ",host-nodes="); - virBufferAdd(&buf, tmpmask, -1); - } - - virBufferAsprintf(&buf, ",policy=%s", policy); - } - - if (hugepage || nodemask) { + if (backendStr) { virCommandAddArg(cmd, "-object"); - virCommandAddArgBuffer(cmd, &buf); - } else { - virBufferFreeAndReset(&buf); + virCommandAddArg(cmd, backendStr); } } else { - if (memAccess) { + if (def->cpu->cells[i].memAccess) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Shared memory mapping is not supported " "with this QEMU")); @@ -7016,7 +7135,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virBufferAdd(&buf, tmpmask, -1); } - if (hugepage || nodemask) + if (backendStr) virBufferAsprintf(&buf, ",memdev=ram-node%zu", i); else virBufferAsprintf(&buf, ",mem=%llu", cellmem); @@ -7027,8 +7146,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, cleanup: VIR_FREE(cpumask); - VIR_FREE(nodemask); - VIR_FREE(mem_path); + VIR_FREE(backendStr); virBufferFreeAndReset(&buf); return ret; } -- 2.2.2
On Fri, Jan 30, 2015 at 11:34:31AM +0100, Peter Krempa wrote:
Extract the memory backend device code into a separate function so that it can be later easily refactored and reused.
Few small changes for future reusability, namely: - new (currently unused) parameter for user specified page size - size of the memory is specified in kibibytes, divided up in the function - new (currently unused) parameter for user specifed source nodeset - option to enforce capability check --- src/qemu/qemu_command.c | 382 +++++++++++++++++++++++++++++++----------------- 1 file changed, 250 insertions(+), 132 deletions(-)
+ + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + *backendStr = virBufferContentAndReset(&buf); + + if (!hugepage) { + if ((nodemask || force) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support the " + "memory-backend-ram object")); + goto cleanup; + }
This check wasn't present in the old function. It would be nicer to do code movement from new code. Jan
Move the alias as the second formated argument and tweak the tests so that a future refactor that will change the order doesn't break tests. --- src/qemu/qemu_command.c | 9 ++++----- .../qemuxml2argvdata/qemuxml2argv-hugepages-pages.args | 16 ++++++++-------- .../qemuxml2argv-hugepages-pages2.args | 8 ++++---- .../qemuxml2argv-hugepages-pages3.args | 4 ++-- .../qemuxml2argv-hugepages-shared.args | 18 ++++++++++-------- .../qemuxml2argv-numatune-memnode-no-memory.args | 2 +- .../qemuxml2argv-numatune-memnode.args | 6 +++--- 7 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c65f187..2d89c2d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4623,8 +4623,8 @@ qemuBuildMemoryBackendStr(unsigned long long size, if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) goto cleanup; - virBufferAsprintf(&buf, "memory-backend-file,prealloc=yes,mem-path=%s", - mem_path); + virBufferAsprintf(&buf, "memory-backend-file,id=%s%zu", aliasPrefix, id); + virBufferAsprintf(&buf, ",prealloc=yes,mem-path=%s", mem_path); switch (memAccess) { case VIR_MEM_ACCESS_SHARED: @@ -4647,11 +4647,10 @@ qemuBuildMemoryBackendStr(unsigned long long size, goto cleanup; } - virBufferAddLit(&buf, "memory-backend-ram"); + virBufferAsprintf(&buf, "memory-backend-ram,id=%s%zu", aliasPrefix, id); } - virBufferAsprintf(&buf, ",size=%lluM,id=%s%zu", size / 1024, - aliasPrefix, id); + virBufferAsprintf(&buf, ",size=%lluM", size / 1024); if (userNodeset) { if (!(nodemask = virBitmapFormat(userNodeset))) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args index 042683a..b954fbc 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 4096 -smp 4 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ -size=1024M,id=ram-node0,host-nodes=0-3,policy=bind \ +-object memory-backend-file,id=ram-node0,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages2M/libvirt/qemu,\ -size=1024M,id=ram-node1,host-nodes=0-3,policy=bind \ +-object memory-backend-file,id=ram-node1,prealloc=yes,\ +mem-path=/dev/hugepages2M/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ -size=1024M,id=ram-node2,host-nodes=0-3,policy=bind \ +-object memory-backend-file,id=ram-node2,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ -size=1024M,id=ram-node3,host-nodes=3,policy=bind \ +-object memory-backend-file,id=ram-node3,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=3,policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args index 9211bc6..90ab776 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args @@ -1,10 +1,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 1024 -smp 2 \ --object memory-backend-file,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=256M,id=ram-node0 \ +-object memory-backend-file,id=ram-node0,prealloc=yes,\ +mem-path=/dev/hugepages2M/libvirt/qemu,size=256M \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-file,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=768M,id=ram-node1 \ +-object memory-backend-file,id=ram-node1,prealloc=yes,\ +mem-path=/dev/hugepages2M/libvirt/qemu,size=768M \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args index f81947e..b932520 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args @@ -1,8 +1,8 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 1024 -smp 2 \ -numa node,nodeid=0,cpus=0,mem=256 \ --object memory-backend-file,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=768M,id=ram-node1 \ +-object memory-backend-file,id=ram-node1,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu,size=768M \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args index a7c7d92..b51de1b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args @@ -1,16 +1,18 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 4096 -smp 4 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ -size=1024M,id=ram-node0,host-nodes=0-3,policy=bind \ +-object memory-backend-file,id=ram-node0,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages2M/libvirt/qemu,\ -share=on,size=1024M,id=ram-node1,host-nodes=0-3,policy=bind \ +-object memory-backend-file,id=ram-node1,prealloc=yes,\ +mem-path=/dev/hugepages2M/libvirt/qemu,share=on,size=1024M,host-nodes=0-3,\ +policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ -share=off,size=1024M,id=ram-node2,host-nodes=0-3,policy=bind \ +-object memory-backend-file,id=ram-node2,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu,share=off,size=1024M,host-nodes=0-3,\ +policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ --object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ -size=1024M,id=ram-node3,host-nodes=3,policy=bind \ +-object memory-backend-file,id=ram-node3,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=3,policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args index 2addf97..4f1f2aa 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args @@ -1,6 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/kvm -S -M pc -m 64 -smp 2 \ --object memory-backend-ram,size=32M,id=ram-node0,host-nodes=3,policy=preferred \ +-object memory-backend-ram,id=ram-node0,size=32M,host-nodes=3,policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -numa node,nodeid=1,cpus=1,mem=32 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args index e4beb98..18007d5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args @@ -1,10 +1,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/kvm -S -M pc -m 24104 -smp 32 \ --object memory-backend-ram,size=20M,id=ram-node0,host-nodes=3,policy=preferred \ +-object memory-backend-ram,id=ram-node0,size=20M,host-nodes=3,policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-ram,size=645M,id=ram-node1,host-nodes=0-7,policy=bind \ +-object memory-backend-ram,id=ram-node1,size=645M,host-nodes=0-7,policy=bind \ -numa node,nodeid=1,cpus=1-27,cpus=29,memdev=ram-node1 \ --object memory-backend-ram,size=23440M,id=ram-node2,\ +-object memory-backend-ram,id=ram-node2,size=23440M,\ host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind \ -numa node,nodeid=2,cpus=28,cpus=30-31,memdev=ram-node2 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -- 2.2.2
QEMU's qapi visitor code allows yes/on/y for true and no/off/n for false value of boolean properities. Unify the used style so that we can generate it later and fix test cases. --- src/qemu/qemu_command.c | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2d89c2d..9e0b178 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4628,11 +4628,11 @@ qemuBuildMemoryBackendStr(unsigned long long size, switch (memAccess) { case VIR_MEM_ACCESS_SHARED: - virBufferAddLit(&buf, ",share=on"); + virBufferAddLit(&buf, ",share=yes"); break; case VIR_MEM_ACCESS_PRIVATE: - virBufferAddLit(&buf, ",share=off"); + virBufferAddLit(&buf, ",share=no"); break; case VIR_MEM_ACCESS_DEFAULT: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args index b51de1b..27f476f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args @@ -4,11 +4,11 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,share=on,size=1024M,host-nodes=0-3,\ +mem-path=/dev/hugepages2M/libvirt/qemu,share=yes,size=1024M,host-nodes=0-3,\ policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -object memory-backend-file,id=ram-node2,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,share=off,size=1024M,host-nodes=0-3,\ +mem-path=/dev/hugepages1G/libvirt/qemu,share=no,size=1024M,host-nodes=0-3,\ policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ -object memory-backend-file,id=ram-node3,prealloc=yes,\ -- 2.2.2
QEMU's command line visitor as well as the JSON interface take bytes by default for memory object sizes. Convert mebibytes to bytes so that we can later refactor the existing code for hotplug purposes. --- src/qemu/qemu_command.c | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args | 12 ++++++++---- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args | 14 ++++++++------ .../qemuxml2argv-numatune-memnode-no-memory.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args | 8 +++++--- 7 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9e0b178..0c343b6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4650,7 +4650,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, virBufferAsprintf(&buf, "memory-backend-ram,id=%s%zu", aliasPrefix, id); } - virBufferAsprintf(&buf, ",size=%lluM", size / 1024); + virBufferAsprintf(&buf, ",size=%llu", size * 1024); if (userNodeset) { if (!(nodemask = virBitmapFormat(userNodeset))) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args index b954fbc..46ec751 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args @@ -1,16 +1,20 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 4096 -smp 4 \ -object memory-backend-file,id=ram-node0,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\ +policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ +mem-path=/dev/hugepages2M/libvirt/qemu,size=1073741824,host-nodes=0-3,\ +policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -object memory-backend-file,id=ram-node2,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\ +policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ -object memory-backend-file,id=ram-node3,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=3,policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=3,\ +policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args index 90ab776..0488800 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args @@ -1,10 +1,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 1024 -smp 2 \ -object memory-backend-file,id=ram-node0,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=256M \ +mem-path=/dev/hugepages2M/libvirt/qemu,size=268435456 \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=768M \ +mem-path=/dev/hugepages2M/libvirt/qemu,size=805306368 \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args index b932520..3bca26c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args @@ -2,7 +2,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 1024 -smp 2 \ -numa node,nodeid=0,cpus=0,mem=256 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=768M \ +mem-path=/dev/hugepages1G/libvirt/qemu,size=805306368 \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args index 27f476f..a6e4d95 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args @@ -1,18 +1,20 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 4096 -smp 4 \ -object memory-backend-file,id=ram-node0,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=0-3,policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\ +policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,share=yes,size=1024M,host-nodes=0-3,\ -policy=bind \ +mem-path=/dev/hugepages2M/libvirt/qemu,share=yes,size=1073741824,\ +host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -object memory-backend-file,id=ram-node2,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,share=no,size=1024M,host-nodes=0-3,\ -policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu,share=no,size=1073741824,\ +host-nodes=0-3,policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ -object memory-backend-file,id=ram-node3,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1024M,host-nodes=3,policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=3,\ +policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args index 4f1f2aa..f0e35b7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/kvm -S -M pc -m 64 -smp 2 \ --object memory-backend-ram,id=ram-node0,size=32M,host-nodes=3,policy=preferred \ +-object memory-backend-ram,id=ram-node0,size=33554432,host-nodes=3,\ +policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -numa node,nodeid=1,cpus=1,mem=32 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args index 18007d5..513d657 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args @@ -1,10 +1,12 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/kvm -S -M pc -m 24104 -smp 32 \ --object memory-backend-ram,id=ram-node0,size=20M,host-nodes=3,policy=preferred \ +-object memory-backend-ram,id=ram-node0,size=20971520,host-nodes=3,\ +policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --object memory-backend-ram,id=ram-node1,size=645M,host-nodes=0-7,policy=bind \ +-object memory-backend-ram,id=ram-node1,size=676331520,host-nodes=0-7,\ +policy=bind \ -numa node,nodeid=1,cpus=1-27,cpus=29,memdev=ram-node1 \ --object memory-backend-ram,id=ram-node2,size=23440M,\ +-object memory-backend-ram,id=ram-node2,size=24578621440,\ host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind \ -numa node,nodeid=2,cpus=28,cpus=30-31,memdev=ram-node2 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -- 2.2.2
With the new JSON to argv formatter we are now able to represent the memory backend definitions in the JSON object format that is reusable for monitor use (hotplug) and then convert it into the shell string. This will avoid having two separate instances of the same code that would create the different formats. Previous refactors now allow to make this step without changes to the test suite. --- Notes: Version 2: - fixed bug in error checking reported by John src/qemu/qemu_command.c | 109 +++++++++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0c343b6..3da1ade 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4533,12 +4533,10 @@ qemuBuildMemoryBackendStr(unsigned long long size, virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virQEMUDriverConfigPtr cfg, - const char *aliasPrefix, - size_t id, - char **backendStr, + const char **backendType, + virJSONValuePtr *backendProps, bool force) { - virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainHugePagePtr master_hugepage = NULL; virDomainHugePagePtr hugepage = NULL; virDomainNumatuneMemMode mode; @@ -4546,11 +4544,15 @@ qemuBuildMemoryBackendStr(unsigned long long size, virMemAccess memAccess = def->cpu->cells[guestNode].memAccess; size_t i; char *mem_path = NULL; - char *nodemask = NULL; - char *tmpmask = NULL, *next = NULL; + virBitmapPtr nodemask = NULL; int ret = -1; + virJSONValuePtr props = NULL; - *backendStr = NULL; + *backendProps = NULL; + *backendType = NULL; + + if (!(props = virJSONValueNewObject())) + return -1; mode = virDomainNumatuneGetMode(def->numatune, guestNode); @@ -4623,16 +4625,23 @@ qemuBuildMemoryBackendStr(unsigned long long size, if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) goto cleanup; - virBufferAsprintf(&buf, "memory-backend-file,id=%s%zu", aliasPrefix, id); - virBufferAsprintf(&buf, ",prealloc=yes,mem-path=%s", mem_path); + *backendType = "memory-backend-file"; + + if (virJSONValueObjectAdd(props, + "b:prealloc", true, + "s:mem-path", mem_path, + NULL) < 0) + goto cleanup; switch (memAccess) { case VIR_MEM_ACCESS_SHARED: - virBufferAddLit(&buf, ",share=yes"); + if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0) + goto cleanup; break; case VIR_MEM_ACCESS_PRIVATE: - virBufferAddLit(&buf, ",share=no"); + if (virJSONValueObjectAdd(props, "b:share", false, NULL) < 0) + goto cleanup; break; case VIR_MEM_ACCESS_DEFAULT: @@ -4647,43 +4656,30 @@ qemuBuildMemoryBackendStr(unsigned long long size, goto cleanup; } - virBufferAsprintf(&buf, "memory-backend-ram,id=%s%zu", aliasPrefix, id); + *backendType = "memory-backend-ram"; } - virBufferAsprintf(&buf, ",size=%llu", size * 1024); + if (virJSONValueObjectAdd(props, "U:size", size * 1024, NULL) < 0) + goto cleanup; if (userNodeset) { - if (!(nodemask = virBitmapFormat(userNodeset))) - goto cleanup; + nodemask = userNodeset; } else { - if (virDomainNumatuneMaybeFormatNodeset(def->numatune, autoNodeset, - &nodemask, guestNode) < 0) + if (virDomainNumatuneMaybeGetNodeset(def->numatune, autoNodeset, + &nodemask, guestNode) < 0) goto cleanup; } if (nodemask) { - if (strchr(nodemask, ',') && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disjoint NUMA node ranges are not supported " - "with this QEMU")); + if (virJSONValueObjectAdd(props, + "m:host-nodes", nodemask, + "S:policy", qemuNumaPolicyTypeToString(mode), + NULL) < 0) goto cleanup; - } - - for (tmpmask = nodemask; tmpmask; tmpmask = next) { - if ((next = strchr(tmpmask, ','))) - *(next++) = '\0'; - virBufferAddLit(&buf, ",host-nodes="); - virBufferAdd(&buf, tmpmask, -1); - } - - virBufferAsprintf(&buf, ",policy=%s", qemuNumaPolicyTypeToString(mode)); } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - - *backendStr = virBufferContentAndReset(&buf); + *backendProps = props; + props = NULL; if (!hugepage) { if ((nodemask || force) && @@ -4705,8 +4701,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, ret = 0; cleanup: - virBufferFreeAndReset(&buf); - VIR_FREE(nodemask); + virJSONValueFree(props); VIR_FREE(mem_path); return ret; @@ -4721,19 +4716,39 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, virBitmapPtr auto_nodeset, char **backendStr) { - int ret; + virJSONValuePtr props = NULL; + char *alias = NULL; + const char *backendType; + int ret = -1; + int rc; - ret = qemuBuildMemoryBackendStr(def->cpu->cells[cell].mem, 0, cell, - NULL, auto_nodeset, - def, qemuCaps, cfg, - "ram-node", cell, - backendStr, false); + *backendStr = NULL; - if (ret == 1) { - VIR_FREE(*backendStr); - return 0; + if (virAsprintf(&alias, "ram-node%zu", cell) < 0) + goto cleanup; + + if ((rc = qemuBuildMemoryBackendStr(def->cpu->cells[cell].mem, 0, cell, + NULL, auto_nodeset, + def, qemuCaps, cfg, + &backendType, &props, false)) < 0) + goto cleanup; + + if (rc == 1) { + ret = 0; + goto cleanup; } + if (!(*backendStr = qemuBuildObjectCommandlineFromJSON(backendType, + alias, + props))) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(alias); + virJSONValueFree(props); + return ret; } -- 2.2.2
Change done by commit f309db1f4d51009bad0d32e12efc75530b66836b wrongly assumes that qemu can start with a combination of NUMA nodes specified with the "memdev" option and the appropriate backends, and the legacy way by specifying only "mem" as a size argument. QEMU rejects such commandline though: $ /usr/bin/qemu-system-x86_64 -S -M pc -m 1024 -smp 2 \ -numa node,nodeid=0,cpus=0,mem=256 \ -object memory-backend-ram,id=ram-node1,size=12345 \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 qemu-system-x86_64: -numa node,nodeid=1,cpus=1,memdev=ram-node1: qemu: memdev option must be specified for either all or no nodes To fix this issue we need to check if any of the nodes requires the new definition with the backend and if so, then all other nodes have to use it too. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182467 --- src/qemu/qemu_command.c | 72 ++++++++++++---------- .../qemuxml2argv-hugepages-pages3.args | 3 +- .../qemuxml2argv-numatune-memnode-no-memory.args | 3 +- 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3da1ade..ec4f35b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4733,17 +4733,12 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, &backendType, &props, false)) < 0) goto cleanup; - if (rc == 1) { - ret = 0; - goto cleanup; - } - if (!(*backendStr = qemuBuildObjectCommandlineFromJSON(backendType, alias, props))) goto cleanup; - ret = 0; + ret = rc; cleanup: VIR_FREE(alias); @@ -7052,7 +7047,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, size_t i; virBuffer buf = VIR_BUFFER_INITIALIZER; char *cpumask = NULL, *tmpmask = NULL, *next = NULL; - char *backendStr = NULL; + char **nodeBackends = NULL; + bool needBackend = false; + int rc; int ret = -1; const long system_page_size = sysconf(_SC_PAGESIZE) / 1024; @@ -7101,35 +7098,24 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } } + if (VIR_ALLOC_N(nodeBackends, def->cpu->ncells) < 0) + goto cleanup; + + /* using of -numa memdev= cannot be combined with -numa mem=, thus we + * need to check which approach to use */ for (i = 0; i < def->cpu->ncells; i++) { unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); def->cpu->cells[i].mem = cellmem * 1024; - VIR_FREE(cpumask); - VIR_FREE(backendStr); - - if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask))) - goto cleanup; - - if (strchr(cpumask, ',') && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disjoint NUMA cpu ranges are not supported " - "with this QEMU")); - goto cleanup; - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { - if (qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i, - auto_nodeset, &backendStr) < 0) + if ((rc = qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i, + auto_nodeset, + &nodeBackends[i])) < 0) goto cleanup; - if (backendStr) { - virCommandAddArg(cmd, "-object"); - virCommandAddArg(cmd, backendStr); - } + if (rc == 0) + needBackend = true; } else { if (def->cpu->cells[i].memAccess) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -7138,6 +7124,23 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, goto cleanup; } } + } + + for (i = 0; i < def->cpu->ncells; i++) { + VIR_FREE(cpumask); + if (!(cpumask = virBitmapFormat(def->cpu->cells[i].cpumask))) + goto cleanup; + + if (strchr(cpumask, ',') && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disjoint NUMA cpu ranges are not supported " + "with this QEMU")); + goto cleanup; + } + + if (needBackend) + virCommandAddArgList(cmd, "-object", nodeBackends[i], NULL); virCommandAddArg(cmd, "-numa"); virBufferAsprintf(&buf, "node,nodeid=%zu", i); @@ -7149,10 +7152,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virBufferAdd(&buf, tmpmask, -1); } - if (backendStr) + if (needBackend) virBufferAsprintf(&buf, ",memdev=ram-node%zu", i); else - virBufferAsprintf(&buf, ",mem=%llu", cellmem); + virBufferAsprintf(&buf, ",mem=%llu", def->cpu->cells[i].mem / 1024); virCommandAddArgBuffer(cmd, &buf); } @@ -7160,7 +7163,14 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, cleanup: VIR_FREE(cpumask); - VIR_FREE(backendStr); + + if (nodeBackends) { + for (i = 0; i < def->cpu->ncells; i++) + VIR_FREE(nodeBackends[i]); + + VIR_FREE(nodeBackends); + } + virBufferFreeAndReset(&buf); return ret; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args index 3bca26c..bf37a69 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 1024 -smp 2 \ --numa node,nodeid=0,cpus=0,mem=256 \ +-object memory-backend-ram,id=ram-node0,size=268435456 \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ mem-path=/dev/hugepages1G/libvirt/qemu,size=805306368 \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args index f0e35b7..f9a35ac 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args @@ -3,6 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -object memory-backend-ram,id=ram-node0,size=33554432,host-nodes=3,\ policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ --numa node,nodeid=1,cpus=1,mem=32 \ +-object memory-backend-ram,id=ram-node1,size=33554432 \ +-numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -net none -serial none -parallel none -- 2.2.2
On Fri, Jan 30, 2015 at 11:34:23AM +0100, Peter Krempa wrote:
This series was split out from my memory hotplug series that is not quite ready yet. The aim of this series is to refactor the code that creates commandline for the memory-backend-* qemu object so that it can be later reused for hotplug via monitor. Additionally this series also fixes a bug in NUMA memory initialisation where qemu doesn't support a combination of old and new approach.
Version 2 fixes bugs found by first round of review. Patch 4/13 is new in this series. Please see individual patches for changes. (If the change line isn't present the patch was not reviewed)
Peter Krempa (13): conf: numatune: Extract code for requesting memory nodeset from formatting test: utils: Add helpers for automatic numbering of test cases util: json: make value object creator universal by supporting adding util: bitmap: Add option to allocate bitmap without reporting error util: json: Add functions to convert JSON arrays from/to virBitmaps util: json: add helper to iterate JSON object key=value pairs qemu: command: Add helper to format -object strings from JSON representation qemu: Extract code to setup memory backing objects qemu: command: Shuffle around formating of alias for memory backend objs qemu: command: Unify values for boolean values when formating memory backends qemu: command: Switch to bytes when formatting size for memory backends qemu: command: Refactor NUMA backend object formatting to use JSON objs qemu: command: Don't combine old and modern NUMA node creation
src/conf/numatune_conf.c | 33 +- src/conf/numatune_conf.h | 5 + src/libvirt_private.syms | 7 + src/qemu/qemu_command.c | 547 +++++++++++++++------ src/qemu/qemu_command.h | 4 + src/util/virbitmap.c | 42 +- src/util/virbitmap.h | 1 + src/util/virjson.c | 222 ++++++++- src/util/virjson.h | 17 + tests/Makefile.am | 13 +- tests/qemucommandutiltest.c | 118 +++++ .../qemuxml2argv-hugepages-pages.args | 20 +- .../qemuxml2argv-hugepages-pages2.args | 8 +- .../qemuxml2argv-hugepages-pages3.args | 7 +- .../qemuxml2argv-hugepages-shared.args | 20 +- .../qemuxml2argv-numatune-memnode-no-memory.args | 6 +- .../qemuxml2argv-numatune-memnode.args | 8 +- tests/testutils.c | 52 ++ tests/testutils.h | 3 + 19 files changed, 912 insertions(+), 221 deletions(-) create mode 100644 tests/qemucommandutiltest.c
ACK series. Jan
On Fri, Jan 30, 2015 at 15:07:33 +0100, Ján Tomko wrote:
On Fri, Jan 30, 2015 at 11:34:23AM +0100, Peter Krempa wrote:
This series was split out from my memory hotplug series that is not quite ready yet. The aim of this series is to refactor the code that creates commandline for the memory-backend-* qemu object so that it can be later reused for hotplug via monitor. Additionally this series also fixes a bug in NUMA memory initialisation where qemu doesn't support a combination of old and new approach.
Version 2 fixes bugs found by first round of review. Patch 4/13 is new in this series. Please see individual patches for changes. (If the change line isn't present the patch was not reviewed)
Peter Krempa (13): conf: numatune: Extract code for requesting memory nodeset from formatting test: utils: Add helpers for automatic numbering of test cases util: json: make value object creator universal by supporting adding util: bitmap: Add option to allocate bitmap without reporting error util: json: Add functions to convert JSON arrays from/to virBitmaps util: json: add helper to iterate JSON object key=value pairs qemu: command: Add helper to format -object strings from JSON representation qemu: Extract code to setup memory backing objects qemu: command: Shuffle around formating of alias for memory backend objs qemu: command: Unify values for boolean values when formating memory backends qemu: command: Switch to bytes when formatting size for memory backends qemu: command: Refactor NUMA backend object formatting to use JSON objs qemu: command: Don't combine old and modern NUMA node creation
src/conf/numatune_conf.c | 33 +- src/conf/numatune_conf.h | 5 + src/libvirt_private.syms | 7 + src/qemu/qemu_command.c | 547 +++++++++++++++------ src/qemu/qemu_command.h | 4 + src/util/virbitmap.c | 42 +- src/util/virbitmap.h | 1 + src/util/virjson.c | 222 ++++++++- src/util/virjson.h | 17 + tests/Makefile.am | 13 +- tests/qemucommandutiltest.c | 118 +++++ .../qemuxml2argv-hugepages-pages.args | 20 +- .../qemuxml2argv-hugepages-pages2.args | 8 +- .../qemuxml2argv-hugepages-pages3.args | 7 +- .../qemuxml2argv-hugepages-shared.args | 20 +- .../qemuxml2argv-numatune-memnode-no-memory.args | 6 +- .../qemuxml2argv-numatune-memnode.args | 8 +- tests/testutils.c | 52 ++ tests/testutils.h | 3 + 19 files changed, 912 insertions(+), 221 deletions(-) create mode 100644 tests/qemucommandutiltest.c
ACK series.
Jan
Pushed; Thanks. Peter
participants (2)
-
Ján Tomko -
Peter Krempa