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 mid of test groups without the need to re-number the rest of test cases. --- tests/testutils.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ tests/testutils.h | 3 +++ 2 files changed, 49 insertions(+) diff --git a/tests/testutils.c b/tests/testutils.c index 9a79f98..c7d2615 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -986,3 +986,49 @@ 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. + */ +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 numbring. + * + * 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. + */ +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. --- src/libvirt_private.syms | 2 ++ src/util/virjson.c | 74 +++++++++++++++++++++++++++++++----------------- src/util/virjson.h | 5 ++++ 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2bbce03..cc74e35 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1533,6 +1533,8 @@ virJSONValueNewNumberUlong; virJSONValueNewObject; virJSONValueNewString; virJSONValueNewStringLen; +virJSONValueObjectAdd; +virJSONValueObjectAddVArgs; virJSONValueObjectAppend; virJSONValueObjectAppendBoolean; virJSONValueObjectAppendNull; diff --git a/src/util/virjson.c b/src/util/virjson.c index 9f2e1cf..9eb1bff 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,45 @@ 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) + VIR_FREE(*obj); + 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

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. --- 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 cc74e35..70c81a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1514,6 +1514,7 @@ virJSONValueArrayGet; virJSONValueArraySize; virJSONValueFree; virJSONValueFromString; +virJSONValueGetArrayAsBitmap; virJSONValueGetBoolean; virJSONValueGetNumberDouble; virJSONValueGetNumberInt; @@ -1523,6 +1524,7 @@ virJSONValueGetNumberUlong; virJSONValueGetString; virJSONValueIsNull; virJSONValueNewArray; +virJSONValueNewArrayFromBitmap; virJSONValueNewBoolean; virJSONValueNewNull; virJSONValueNewNumberDouble; diff --git a/src/util/virjson.c b/src/util/virjson.c index 9eb1bff..3e00650 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); @@ -941,6 +965,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 (except for out-of-memory) 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(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 = virBitmapNew(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 easies 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. --- 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 70c81a8..196b837 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1548,6 +1548,7 @@ virJSONValueObjectAppendNumberUlong; virJSONValueObjectAppendString; virJSONValueObjectCreate; virJSONValueObjectCreateVArgs; +virJSONValueObjectForeachKeyValue; virJSONValueObjectGet; virJSONValueObjectGetBoolean; virJSONValueObjectGetKey; diff --git a/src/util/virjson.c b/src/util/virjson.c index 3e00650..ec1778d 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1198,6 +1198,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

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

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. --- 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..bb58a09 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)) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(alias); + virJSONValueFree(props); + return ret; } -- 2.2.2

On 01/28/2015 05:30 AM, Peter Krempa wrote: Ran changes through my Coverity checker... Issue found with this change... Coverity Error: CONSTANT_EXPRESSION_RESULT 4735 (1) Event result_independent_of_operands: "!(*backendStr = qemuBuildObjectCommandlineFromJSON(backendType, alias, props)) < 0" is always false regardless of the values of its operands. This occurs as the logical operand of if. 4736 if (!(*backendStr = qemuBuildObjectCommandlineFromJSON(backendType, 4737 alias, 4738 props)) < 0) ^^^^ Looks like the " < 0" is unnecessary And of course it cglames the goto cleanup is DEADCODE because of that.