--- tests/Makefile.am | 8 ++++---- tests/{jsontest.c => virjsontest.c} | 0 2 files changed, 4 insertions(+), 4 deletions(-) rename tests/{jsontest.c => virjsontest.c} (100%) diff --git a/tests/Makefile.am b/tests/Makefile.am index 19986dc99..3596b5ff1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -328,7 +328,7 @@ test_programs += objectlocking endif WITH_CIL if WITH_YAJL -test_programs += jsontest +test_programs += virjsontest endif WITH_YAJL test_programs += \ @@ -1375,9 +1375,9 @@ virfirewalltest_SOURCES = \ virfirewalltest_LDADD = $(LDADDS) $(DBUS_LIBS) virfirewalltest_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) -jsontest_SOURCES = \ - jsontest.c testutils.h testutils.c -jsontest_LDADD = $(LDADDS) +virjsontest_SOURCES = \ + virjsontest.c testutils.h testutils.c +virjsontest_LDADD = $(LDADDS) utiltest_SOURCES = \ utiltest.c testutils.h testutils.c diff --git a/tests/jsontest.c b/tests/virjsontest.c similarity index 100% rename from tests/jsontest.c rename to tests/virjsontest.c -- 2.12.2

Allows testing whether a virJSONValue is an object. --- src/libvirt_private.syms | 1 + src/util/virjson.c | 10 ++++++++++ src/util/virjson.h | 2 ++ 3 files changed, 13 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c1e9471c5..d487b1f43 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1905,6 +1905,7 @@ virJSONValueGetString; virJSONValueHashFree; virJSONValueIsArray; virJSONValueIsNull; +virJSONValueIsObject; virJSONValueNewArray; virJSONValueNewArrayFromBitmap; virJSONValueNewBoolean; diff --git a/src/util/virjson.c b/src/util/virjson.c index b49b29b0f..efd6c3a0e 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -905,6 +905,16 @@ virJSONValueObjectGetValue(virJSONValuePtr object, bool +virJSONValueIsObject(virJSONValuePtr object) +{ + if (object) + return object->type == VIR_JSON_TYPE_OBJECT; + else + return NULL; +} + + +bool virJSONValueIsArray(virJSONValuePtr array) { return array->type == VIR_JSON_TYPE_ARRAY; diff --git a/src/util/virjson.h b/src/util/virjson.h index 14b74c061..c9d9752de 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -114,6 +114,8 @@ virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr object, const char *key); virJSONValuePtr virJSONValueObjectGetByType(virJSONValuePtr object, const char *key, virJSONType type); +bool virJSONValueIsObject(virJSONValuePtr object); + bool virJSONValueIsArray(virJSONValuePtr array); ssize_t virJSONValueArraySize(const virJSONValue *array); virJSONValuePtr virJSONValueArrayGet(virJSONValuePtr object, unsigned int element); -- 2.12.2

On Tue, Jun 27, 2017 at 17:55:40 +0200, Bjoern Walk wrote: [...] Thanks! It's a shame we don't have 'nullptr' like functionality in C.

The code will become more universal so it makes more sense for it to live with the rest of the JSON functions. --- src/libvirt_private.syms | 1 + src/util/virjson.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virjson.h | 2 ++ src/util/virstoragefile.c | 65 +---------------------------------------------- 4 files changed, 65 insertions(+), 64 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d487b1f43..9f64b8d93 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1931,6 +1931,7 @@ virJSONValueObjectAppendNumberUlong; virJSONValueObjectAppendString; virJSONValueObjectCreate; virJSONValueObjectCreateVArgs; +virJSONValueObjectDeflatten; virJSONValueObjectForeachKeyValue; virJSONValueObjectGet; virJSONValueObjectGetArray; diff --git a/src/util/virjson.c b/src/util/virjson.c index efd6c3a0e..8ab542432 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1965,3 +1965,64 @@ virJSONStringReformat(const char *jsonstr, virJSONValueFree(json); return ret; } + + +static int +virJSONValueObjectDeflattenWorker(const char *key, + virJSONValuePtr value, + void *opaque) +{ + virJSONValuePtr retobj = opaque; + virJSONValuePtr newval = NULL; + const char *newkey; + + if (!(newkey = STRSKIP(key, "file."))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("JSON object is neither nested nor flattened")); + return -1; + } + + if (!(newval = virJSONValueCopy(value))) + return -1; + + if (virJSONValueObjectAppend(retobj, newkey, newval) < 0) { + virJSONValueFree(newval); + return -1; + } + + return 0; +} + + +/** + * virJSONValueObjectDeflatten: + * + * In some cases it's possible to nest JSON objects by prefixing object members + * with the parent object name followed by the dot and then the attribute name + * rather than directly using a nested value object (e.g qemu's JSON + * pseudo-protocol in backing file definition). + * + * This function will attempt to reverse the process and provide a nested json + * hierarchy so that the parsers can be kept simple and we still can use the + * weird syntax some users might use. + * + * Currently this function will flatten out just the 'file.' prefix into a new + * tree. Any other syntax will be rejected. + */ +virJSONValuePtr +virJSONValueObjectDeflatten(virJSONValuePtr json) +{ + virJSONValuePtr ret; + + if (!(ret = virJSONValueNewObject())) + return NULL; + + if (virJSONValueObjectForeachKeyValue(json, + virJSONValueObjectDeflattenWorker, + ret) < 0) { + virJSONValueFree(ret); + return NULL; + } + + return ret; +} diff --git a/src/util/virjson.h b/src/util/virjson.h index c9d9752de..e89a776ab 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -186,4 +186,6 @@ virJSONValuePtr virJSONValueCopy(const virJSONValue *in); char *virJSONStringReformat(const char *jsonstr, bool pretty); +virJSONValuePtr virJSONValueObjectDeflatten(virJSONValuePtr json); + #endif /* __VIR_JSON_H_ */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index f0ed5c6bd..52c5301ff 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3244,69 +3244,6 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { }; -static int -virStorageSourceParseBackingJSONDeflattenWorker(const char *key, - virJSONValuePtr value, - void *opaque) -{ - virJSONValuePtr retobj = opaque; - virJSONValuePtr newval = NULL; - const char *newkey; - - if (!(newkey = STRSKIP(key, "file."))) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("JSON backing file syntax is neither nested nor " - "flattened")); - return -1; - } - - if (!(newval = virJSONValueCopy(value))) - return -1; - - if (virJSONValueObjectAppend(retobj, newkey, newval) < 0) { - virJSONValueFree(newval); - return -1; - } - - return 0; -} - - -/** - * virStorageSourceParseBackingJSONDeflatten: - * - * The json: pseudo-protocol syntax in qemu allows multiple approaches to - * describe nesting of the values. This is due to the lax handling of the string - * in qemu and the fact that internally qemu is flattening the values using '.'. - * - * This allows to specify nested json strings either using nested json objects - * or prefixing object members with the parent object name followed by the dot. - * - * This function will attempt to reverse the process and provide a nested json - * hierarchy so that the parsers can be kept simple and we still can use the - * weird syntax some users might use. - * - * Currently this function will flatten out just the 'file.' prefix into a new - * tree. Any other syntax will be rejected. - */ -static virJSONValuePtr -virStorageSourceParseBackingJSONDeflatten(virJSONValuePtr json) -{ - virJSONValuePtr ret; - - if (!(ret = virJSONValueNewObject())) - return NULL; - - if (virJSONValueObjectForeachKeyValue(json, - virStorageSourceParseBackingJSONDeflattenWorker, - ret) < 0) { - virJSONValueFree(ret); - return NULL; - } - - return ret; -} - static int virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, @@ -3320,7 +3257,7 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, int ret = -1; if (!(file = virJSONValueObjectGetObject(json, "file"))) { - if (!(fixedroot = virStorageSourceParseBackingJSONDeflatten(json))) + if (!(fixedroot = virJSONValueObjectDeflatten(json))) goto cleanup; file = fixedroot; -- 2.12.2

Currently the function would deflatten the object by dropping the 'file' prefix from the attributes. This does not really scale well or adhere to the documentation. Until we refactor the worker to properly deflatten everything we at least simulate it by adding the "file" wrapper object back. --- src/util/virjson.c | 19 +++++++++++++------ src/util/virstoragefile.c | 8 +++++++- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 8ab542432..a8e28cd1b 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -2012,17 +2012,24 @@ virJSONValueObjectDeflattenWorker(const char *key, virJSONValuePtr virJSONValueObjectDeflatten(virJSONValuePtr json) { - virJSONValuePtr ret; + virJSONValuePtr deflattened; + virJSONValuePtr ret = NULL; - if (!(ret = virJSONValueNewObject())) + if (!(deflattened = virJSONValueNewObject())) return NULL; if (virJSONValueObjectForeachKeyValue(json, virJSONValueObjectDeflattenWorker, - ret) < 0) { - virJSONValueFree(ret); - return NULL; - } + deflattened) < 0) + goto cleanup; + + if (virJSONValueObjectCreate(&ret, "a:file", deflattened, NULL) < 0) + goto cleanup; + + deflattened = NULL; + + cleanup: + virJSONValueFree(deflattened); return ret; } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 52c5301ff..d24502fbf 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3260,7 +3260,13 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, if (!(fixedroot = virJSONValueObjectDeflatten(json))) goto cleanup; - file = fixedroot; + if (!(file = virJSONValueObjectGetObject(fixedroot, "file"))) { + str = virJSONValueToString(json, false); + virReportError(VIR_ERR_INVALID_ARG, + _("JSON backing volume defintion '%s' lacks 'file' object"), + str); + goto cleanup; + } } if (!(drvname = virJSONValueObjectGetString(file, "driver"))) { -- 2.12.2

Add a few test cases to verify that the old behaviour does not break and that new one behaves sanely. --- tests/virjsondata/deflatten-basic-file-in.json | 8 +++ tests/virjsondata/deflatten-basic-file-out.json | 10 +++ tests/virjsondata/deflatten-basic-generic-in.json | 14 ++++ .../deflatten-concat-double-key-in.json | 7 ++ .../deflatten-concat-double-key-out.json | 9 +++ tests/virjsondata/deflatten-concat-in.json | 5 ++ tests/virjsondata/deflatten-concat-out.json | 9 +++ tests/virjsondata/deflatten-deep-file-in.json | 9 +++ tests/virjsondata/deflatten-deep-file-out.json | 11 ++++ tests/virjsondata/deflatten-deep-generic-in.json | 9 +++ tests/virjsondata/deflatten-double-key-in.json | 4 ++ tests/virjsondata/deflatten-double-key-out.json | 6 ++ tests/virjsondata/deflatten-nested-in.json | 16 +++++ tests/virjsondata/deflatten-nested-out.json | 18 ++++++ tests/virjsondata/deflatten-unflattened-in.json | 12 ++++ tests/virjsondata/deflatten-unflattened-out.json | 14 ++++ tests/virjsontest.c | 74 ++++++++++++++++++++++ 17 files changed, 235 insertions(+) create mode 100644 tests/virjsondata/deflatten-basic-file-in.json create mode 100644 tests/virjsondata/deflatten-basic-file-out.json create mode 100644 tests/virjsondata/deflatten-basic-generic-in.json create mode 100644 tests/virjsondata/deflatten-concat-double-key-in.json create mode 100644 tests/virjsondata/deflatten-concat-double-key-out.json create mode 100644 tests/virjsondata/deflatten-concat-in.json create mode 100644 tests/virjsondata/deflatten-concat-out.json create mode 100644 tests/virjsondata/deflatten-deep-file-in.json create mode 100644 tests/virjsondata/deflatten-deep-file-out.json create mode 100644 tests/virjsondata/deflatten-deep-generic-in.json create mode 100644 tests/virjsondata/deflatten-double-key-in.json create mode 100644 tests/virjsondata/deflatten-double-key-out.json create mode 100644 tests/virjsondata/deflatten-nested-in.json create mode 100644 tests/virjsondata/deflatten-nested-out.json create mode 100644 tests/virjsondata/deflatten-unflattened-in.json create mode 100644 tests/virjsondata/deflatten-unflattened-out.json diff --git a/tests/virjsondata/deflatten-basic-file-in.json b/tests/virjsondata/deflatten-basic-file-in.json new file mode 100644 index 000000000..54f8bcd88 --- /dev/null +++ b/tests/virjsondata/deflatten-basic-file-in.json @@ -0,0 +1,8 @@ +{ + "file.int": 1, + "file.string": "string", + "file.object": { + "data":"value", + "foo":"bar" + } +} diff --git a/tests/virjsondata/deflatten-basic-file-out.json b/tests/virjsondata/deflatten-basic-file-out.json new file mode 100644 index 000000000..1a48eda54 --- /dev/null +++ b/tests/virjsondata/deflatten-basic-file-out.json @@ -0,0 +1,10 @@ +{ + "file": { + "int": 1, + "string": "string", + "object": { + "data": "value", + "foo": "bar" + } + } +} diff --git a/tests/virjsondata/deflatten-basic-generic-in.json b/tests/virjsondata/deflatten-basic-generic-in.json new file mode 100644 index 000000000..08304287d --- /dev/null +++ b/tests/virjsondata/deflatten-basic-generic-in.json @@ -0,0 +1,14 @@ +{ + "foo.int": 1, + "bar.int": 1, + "foo.string": "string", + "foo.object": { + "data":"value", + "foo":"bar" + }, + "blurb.string": "string", + "blurb.object": { + "data":"value", + "foo":"bar" + } +} diff --git a/tests/virjsondata/deflatten-concat-double-key-in.json b/tests/virjsondata/deflatten-concat-double-key-in.json new file mode 100644 index 000000000..5bb020ca9 --- /dev/null +++ b/tests/virjsondata/deflatten-concat-double-key-in.json @@ -0,0 +1,7 @@ +{ + "file.nest":{ + "into":"is already here" + }, + "file.nest.into":2, + "file.nest.there":"too" +} diff --git a/tests/virjsondata/deflatten-concat-double-key-out.json b/tests/virjsondata/deflatten-concat-double-key-out.json new file mode 100644 index 000000000..5624ef123 --- /dev/null +++ b/tests/virjsondata/deflatten-concat-double-key-out.json @@ -0,0 +1,9 @@ +{ + "file": { + "nest": { + "into": "is already here" + }, + "nest.into": 2, + "nest.there": "too" + } +} diff --git a/tests/virjsondata/deflatten-concat-in.json b/tests/virjsondata/deflatten-concat-in.json new file mode 100644 index 000000000..91a824375 --- /dev/null +++ b/tests/virjsondata/deflatten-concat-in.json @@ -0,0 +1,5 @@ +{ + "file.nest":{}, + "file.nest.into":2, + "file.nest.there":"too" +} diff --git a/tests/virjsondata/deflatten-concat-out.json b/tests/virjsondata/deflatten-concat-out.json new file mode 100644 index 000000000..539d2cc30 --- /dev/null +++ b/tests/virjsondata/deflatten-concat-out.json @@ -0,0 +1,9 @@ +{ + "file": { + "nest": { + + }, + "nest.into": 2, + "nest.there": "too" + } +} diff --git a/tests/virjsondata/deflatten-deep-file-in.json b/tests/virjsondata/deflatten-deep-file-in.json new file mode 100644 index 000000000..f1b1586c0 --- /dev/null +++ b/tests/virjsondata/deflatten-deep-file-in.json @@ -0,0 +1,9 @@ +{ + "file.double.nest1":"some", + "file.double.nest2":"more", + "file.double.nest3": { + "even":"objects" + }, + "file.very.deeply.nested.object.chains.nest1":"some", + "file.very.deeply.nested.object.chains.nest2":"stuff" +} diff --git a/tests/virjsondata/deflatten-deep-file-out.json b/tests/virjsondata/deflatten-deep-file-out.json new file mode 100644 index 000000000..a5910c9f7 --- /dev/null +++ b/tests/virjsondata/deflatten-deep-file-out.json @@ -0,0 +1,11 @@ +{ + "file": { + "double.nest1": "some", + "double.nest2": "more", + "double.nest3": { + "even": "objects" + }, + "very.deeply.nested.object.chains.nest1": "some", + "very.deeply.nested.object.chains.nest2": "stuff" + } +} diff --git a/tests/virjsondata/deflatten-deep-generic-in.json b/tests/virjsondata/deflatten-deep-generic-in.json new file mode 100644 index 000000000..6a4edfb9b --- /dev/null +++ b/tests/virjsondata/deflatten-deep-generic-in.json @@ -0,0 +1,9 @@ +{ + "foo.double.nest1":"some", + "foo.double.nest2":"more", + "bar.double.nest3": { + "even":"objects" + }, + "foo.very.deeply.nested.object.chains.nest1":"some", + "foo.very.deeply.nested.object.chains.nest2":"stuff" +} diff --git a/tests/virjsondata/deflatten-double-key-in.json b/tests/virjsondata/deflatten-double-key-in.json new file mode 100644 index 000000000..7bca2921b --- /dev/null +++ b/tests/virjsondata/deflatten-double-key-in.json @@ -0,0 +1,4 @@ +{ + "file.nest":1, + "file.nest.into":2 +} diff --git a/tests/virjsondata/deflatten-double-key-out.json b/tests/virjsondata/deflatten-double-key-out.json new file mode 100644 index 000000000..ca6766e5b --- /dev/null +++ b/tests/virjsondata/deflatten-double-key-out.json @@ -0,0 +1,6 @@ +{ + "file": { + "nest": 1, + "nest.into": 2 + } +} diff --git a/tests/virjsondata/deflatten-nested-in.json b/tests/virjsondata/deflatten-nested-in.json new file mode 100644 index 000000000..38e69683e --- /dev/null +++ b/tests/virjsondata/deflatten-nested-in.json @@ -0,0 +1,16 @@ +{ + "file.nest": { + "even.objects":"can", + "even.contain":"some", + "even.more":{ + "deeply.nested":"objects", + "deeply.needing":"deflattening", + "deeply.some.even":"more", + "deeply.some.than":{ + "others.thought.was":"even", + "others.thought.completely":"necessary" + }, + "perhaps":"flat value" + } + } +} diff --git a/tests/virjsondata/deflatten-nested-out.json b/tests/virjsondata/deflatten-nested-out.json new file mode 100644 index 000000000..acdcd1fc8 --- /dev/null +++ b/tests/virjsondata/deflatten-nested-out.json @@ -0,0 +1,18 @@ +{ + "file": { + "nest": { + "even.objects": "can", + "even.contain": "some", + "even.more": { + "deeply.nested": "objects", + "deeply.needing": "deflattening", + "deeply.some.even": "more", + "deeply.some.than": { + "others.thought.was": "even", + "others.thought.completely": "necessary" + }, + "perhaps": "flat value" + } + } + } +} diff --git a/tests/virjsondata/deflatten-unflattened-in.json b/tests/virjsondata/deflatten-unflattened-in.json new file mode 100644 index 000000000..e8a68fe17 --- /dev/null +++ b/tests/virjsondata/deflatten-unflattened-in.json @@ -0,0 +1,12 @@ +{ + "file.blah": { + "any":"possible", + "combination":{ + "of":"json", + "nested":{ + "objects":"should", + "be":"possible" + } + } + } +} diff --git a/tests/virjsondata/deflatten-unflattened-out.json b/tests/virjsondata/deflatten-unflattened-out.json new file mode 100644 index 000000000..c02cb4864 --- /dev/null +++ b/tests/virjsondata/deflatten-unflattened-out.json @@ -0,0 +1,14 @@ +{ + "file": { + "blah": { + "any": "possible", + "combination": { + "of": "json", + "nested": { + "objects": "should", + "be": "possible" + } + } + } + } +} diff --git a/tests/virjsontest.c b/tests/virjsontest.c index b67f68ccb..d69b22bf3 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -9,6 +9,8 @@ #include "virjson.h" #include "testutils.h" +#define VIR_FROM_THIS VIR_FROM_NONE + struct testInfo { const char *doc; const char *expect; @@ -312,6 +314,65 @@ testJSONCopy(const void *data) static int +testJSONDeflatten(const void *data) +{ + const struct testInfo *info = data; + virJSONValuePtr injson = NULL; + virJSONValuePtr deflattened = NULL; + char *infile = NULL; + char *indata = NULL; + char *outfile = NULL; + char *actual = NULL; + int ret = -1; + + if (virAsprintf(&infile, "%s/virjsondata/deflatten-%s-in.json", + abs_srcdir, info->doc) < 0 || + virAsprintf(&outfile, "%s/virjsondata/deflatten-%s-out.json", + abs_srcdir, info->doc) < 0) + goto cleanup; + + if (virTestLoadFile(infile, &indata) < 0) + goto cleanup; + + if (!(injson = virJSONValueFromString(indata))) + goto cleanup; + + if ((deflattened = virJSONValueObjectDeflatten(injson))) { + if (!info->pass) { + if (virTestGetVerbose()) + fprintf(stderr, "deflattening should have failed\n"); + + goto cleanup; + } + } else { + if (!info->pass) + ret = 0; + + goto cleanup; + } + + if (!(actual = virJSONValueToString(deflattened, true))) + goto cleanup; + + if (virTestCompareToFile(actual, outfile) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virJSONValueFree(injson); + virJSONValueFree(deflattened); + VIR_FREE(infile); + VIR_FREE(outfile); + VIR_FREE(outfile); + VIR_FREE(actual); + + return ret; + +} + + +static int mymain(void) { int ret = 0; @@ -448,6 +509,19 @@ mymain(void) "{ \"a\": {}, \"b\": 1, \"c\": \"str\", \"d\": [] }", NULL, true); +#define DO_TEST_DEFLATTEN(name, pass) \ + DO_TEST_FULL(name, Deflatten, name, NULL, pass) + + DO_TEST_DEFLATTEN("unflattened", true); + DO_TEST_DEFLATTEN("basic-file", true); + DO_TEST_DEFLATTEN("basic-generic", false); + DO_TEST_DEFLATTEN("deep-file", true); + DO_TEST_DEFLATTEN("deep-generic", false); + DO_TEST_DEFLATTEN("nested", true); + DO_TEST_DEFLATTEN("double-key", true); + DO_TEST_DEFLATTEN("concat", true); + DO_TEST_DEFLATTEN("concat-double-key", true); + return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.12.2

As it turns out sometimes users pass in an arbitrarily nested structure e.g. for the qemu backing chains JSON pseudo protocol. This new implementation deflatens now a single object fully even with nested keys. Additionally it's not necessary now to stick with the "file." prefix for the properties. --- src/util/virjson.c | 67 ++++++++++++++++------ tests/virjsondata/deflatten-basic-generic-out.json | 20 +++++++ .../deflatten-concat-double-key-out.json | 9 --- tests/virjsondata/deflatten-concat-out.json | 7 +-- tests/virjsondata/deflatten-deep-file-out.json | 24 ++++++-- tests/virjsondata/deflatten-deep-generic-out.json | 27 +++++++++ tests/virjsondata/deflatten-double-key-out.json | 6 -- tests/virjsontest.c | 8 +-- 8 files changed, 121 insertions(+), 47 deletions(-) create mode 100644 tests/virjsondata/deflatten-basic-generic-out.json delete mode 100644 tests/virjsondata/deflatten-concat-double-key-out.json create mode 100644 tests/virjsondata/deflatten-deep-generic-out.json delete mode 100644 tests/virjsondata/deflatten-double-key-out.json diff --git a/src/util/virjson.c b/src/util/virjson.c index a8e28cd1b..635b78e3a 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1974,23 +1974,60 @@ virJSONValueObjectDeflattenWorker(const char *key, { virJSONValuePtr retobj = opaque; virJSONValuePtr newval = NULL; - const char *newkey; + virJSONValuePtr existobj; + char **tokens = NULL; + size_t ntokens = 0; + int ret = -1; - if (!(newkey = STRSKIP(key, "file."))) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("JSON object is neither nested nor flattened")); - return -1; + /* non-nested keys only need to be copied */ + if (!strchr(key, '.')) { + if (!(newval = virJSONValueCopy(value))) + return -1; + + if (virJSONValueObjectHasKey(retobj, key)) { + virReportError(VIR_ERR_INVALID_ARG, + _("can't deflatten colliding key '%s'"), key); + goto cleanup; + } + + if (virJSONValueObjectAppend(retobj, key, newval) < 0) + goto cleanup; + + return 0; } - if (!(newval = virJSONValueCopy(value))) - return -1; + if (!(tokens = virStringSplitCount(key, ".", 2, &ntokens))) + goto cleanup; - if (virJSONValueObjectAppend(retobj, newkey, newval) < 0) { - virJSONValueFree(newval); - return -1; + if (ntokens != 2) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid nested value key '%s'"), key); + goto cleanup; } - return 0; + if (!(existobj = virJSONValueObjectGet(retobj, tokens[0]))) { + if (!(existobj = virJSONValueNewObject())) + goto cleanup; + + if (virJSONValueObjectAppend(retobj, tokens[0], existobj) < 0) + goto cleanup; + + } else { + if (!virJSONValueIsObject(existobj)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("mixing nested objects and values is forbidden in " + "JSON deflattening")); + goto cleanup; + } + } + + ret = virJSONValueObjectDeflattenWorker(tokens[1], value, existobj); + + cleanup: + virStringListFreeCount(tokens, ntokens); + virJSONValueFree(newval); + + return ret; } @@ -2005,9 +2042,6 @@ virJSONValueObjectDeflattenWorker(const char *key, * This function will attempt to reverse the process and provide a nested json * hierarchy so that the parsers can be kept simple and we still can use the * weird syntax some users might use. - * - * Currently this function will flatten out just the 'file.' prefix into a new - * tree. Any other syntax will be rejected. */ virJSONValuePtr virJSONValueObjectDeflatten(virJSONValuePtr json) @@ -2023,10 +2057,7 @@ virJSONValueObjectDeflatten(virJSONValuePtr json) deflattened) < 0) goto cleanup; - if (virJSONValueObjectCreate(&ret, "a:file", deflattened, NULL) < 0) - goto cleanup; - - deflattened = NULL; + VIR_STEAL_PTR(ret, deflattened); cleanup: virJSONValueFree(deflattened); diff --git a/tests/virjsondata/deflatten-basic-generic-out.json b/tests/virjsondata/deflatten-basic-generic-out.json new file mode 100644 index 000000000..ab639aa48 --- /dev/null +++ b/tests/virjsondata/deflatten-basic-generic-out.json @@ -0,0 +1,20 @@ +{ + "foo": { + "int": 1, + "string": "string", + "object": { + "data": "value", + "foo": "bar" + } + }, + "bar": { + "int": 1 + }, + "blurb": { + "string": "string", + "object": { + "data": "value", + "foo": "bar" + } + } +} diff --git a/tests/virjsondata/deflatten-concat-double-key-out.json b/tests/virjsondata/deflatten-concat-double-key-out.json deleted file mode 100644 index 5624ef123..000000000 --- a/tests/virjsondata/deflatten-concat-double-key-out.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "file": { - "nest": { - "into": "is already here" - }, - "nest.into": 2, - "nest.there": "too" - } -} diff --git a/tests/virjsondata/deflatten-concat-out.json b/tests/virjsondata/deflatten-concat-out.json index 539d2cc30..be417e53a 100644 --- a/tests/virjsondata/deflatten-concat-out.json +++ b/tests/virjsondata/deflatten-concat-out.json @@ -1,9 +1,8 @@ { "file": { "nest": { - - }, - "nest.into": 2, - "nest.there": "too" + "into": 2, + "there": "too" + } } } diff --git a/tests/virjsondata/deflatten-deep-file-out.json b/tests/virjsondata/deflatten-deep-file-out.json index a5910c9f7..d4614eeaf 100644 --- a/tests/virjsondata/deflatten-deep-file-out.json +++ b/tests/virjsondata/deflatten-deep-file-out.json @@ -1,11 +1,23 @@ { "file": { - "double.nest1": "some", - "double.nest2": "more", - "double.nest3": { - "even": "objects" + "double": { + "nest1": "some", + "nest2": "more", + "nest3": { + "even": "objects" + } }, - "very.deeply.nested.object.chains.nest1": "some", - "very.deeply.nested.object.chains.nest2": "stuff" + "very": { + "deeply": { + "nested": { + "object": { + "chains": { + "nest1": "some", + "nest2": "stuff" + } + } + } + } + } } } diff --git a/tests/virjsondata/deflatten-deep-generic-out.json b/tests/virjsondata/deflatten-deep-generic-out.json new file mode 100644 index 000000000..7ea521a8f --- /dev/null +++ b/tests/virjsondata/deflatten-deep-generic-out.json @@ -0,0 +1,27 @@ +{ + "foo": { + "double": { + "nest1": "some", + "nest2": "more" + }, + "very": { + "deeply": { + "nested": { + "object": { + "chains": { + "nest1": "some", + "nest2": "stuff" + } + } + } + } + } + }, + "bar": { + "double": { + "nest3": { + "even": "objects" + } + } + } +} diff --git a/tests/virjsondata/deflatten-double-key-out.json b/tests/virjsondata/deflatten-double-key-out.json deleted file mode 100644 index ca6766e5b..000000000 --- a/tests/virjsondata/deflatten-double-key-out.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "file": { - "nest": 1, - "nest.into": 2 - } -} diff --git a/tests/virjsontest.c b/tests/virjsontest.c index d69b22bf3..b3ce6591a 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -514,13 +514,13 @@ mymain(void) DO_TEST_DEFLATTEN("unflattened", true); DO_TEST_DEFLATTEN("basic-file", true); - DO_TEST_DEFLATTEN("basic-generic", false); + DO_TEST_DEFLATTEN("basic-generic", true); DO_TEST_DEFLATTEN("deep-file", true); - DO_TEST_DEFLATTEN("deep-generic", false); + DO_TEST_DEFLATTEN("deep-generic", true); DO_TEST_DEFLATTEN("nested", true); - DO_TEST_DEFLATTEN("double-key", true); + DO_TEST_DEFLATTEN("double-key", false); DO_TEST_DEFLATTEN("concat", true); - DO_TEST_DEFLATTEN("concat-double-key", true); + DO_TEST_DEFLATTEN("concat-double-key", false); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.12.2

If a value of the first level object contains more objects needing deflattening which would be wrapped in an actual object the function would not recurse into them. By this simple addition we can fully deflatten the objects. --- src/util/virjson.c | 8 +++++++- tests/virjsondata/deflatten-nested-out.json | 32 +++++++++++++++++++---------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 635b78e3a..0c7beafab 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1981,7 +1981,13 @@ virJSONValueObjectDeflattenWorker(const char *key, /* non-nested keys only need to be copied */ if (!strchr(key, '.')) { - if (!(newval = virJSONValueCopy(value))) + + if (virJSONValueIsObject(value)) + newval = virJSONValueObjectDeflatten(value); + else + newval = virJSONValueCopy(value); + + if (!newval) return -1; if (virJSONValueObjectHasKey(retobj, key)) { diff --git a/tests/virjsondata/deflatten-nested-out.json b/tests/virjsondata/deflatten-nested-out.json index acdcd1fc8..f23ed8fd5 100644 --- a/tests/virjsondata/deflatten-nested-out.json +++ b/tests/virjsondata/deflatten-nested-out.json @@ -1,17 +1,27 @@ { "file": { "nest": { - "even.objects": "can", - "even.contain": "some", - "even.more": { - "deeply.nested": "objects", - "deeply.needing": "deflattening", - "deeply.some.even": "more", - "deeply.some.than": { - "others.thought.was": "even", - "others.thought.completely": "necessary" - }, - "perhaps": "flat value" + "even": { + "objects": "can", + "contain": "some", + "more": { + "deeply": { + "nested": "objects", + "needing": "deflattening", + "some": { + "even": "more", + "than": { + "others": { + "thought": { + "was": "even", + "completely": "necessary" + } + } + } + } + }, + "perhaps": "flat value" + } } } } -- 2.12.2

Now that the JSON deflattener is working sanely we can always attempt the deflattening so that we can then parse the tree as expected. --- src/util/virstoragefile.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d24502fbf..f3dc860ae 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3249,24 +3249,22 @@ static int virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, virJSONValuePtr json) { - virJSONValuePtr fixedroot = NULL; + virJSONValuePtr deflattened = NULL; virJSONValuePtr file; const char *drvname; char *str = NULL; size_t i; int ret = -1; - if (!(file = virJSONValueObjectGetObject(json, "file"))) { - if (!(fixedroot = virJSONValueObjectDeflatten(json))) - goto cleanup; + if (!(deflattened = virJSONValueObjectDeflatten(json))) + goto cleanup; - if (!(file = virJSONValueObjectGetObject(fixedroot, "file"))) { - str = virJSONValueToString(json, false); - virReportError(VIR_ERR_INVALID_ARG, - _("JSON backing volume defintion '%s' lacks 'file' object"), - str); - goto cleanup; - } + if (!(file = virJSONValueObjectGetObject(deflattened, "file"))) { + str = virJSONValueToString(json, false); + virReportError(VIR_ERR_INVALID_ARG, + _("JSON backing volume defintion '%s' lacks 'file' object"), + str); + goto cleanup; } if (!(drvname = virJSONValueObjectGetString(file, "driver"))) { @@ -3290,7 +3288,7 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, cleanup: VIR_FREE(str); - virJSONValueFree(fixedroot); + virJSONValueFree(deflattened); return ret; } -- 2.12.2

Sheepdog and possibly others use nested objects for network server and thus could be specified in a way that libvirt would not parse. Validates that https://bugzilla.redhat.com/show_bug.cgi?id=1464821 is fixed properly. --- tests/virjsondata/deflatten-qemu-sheepdog-in.json | 11 +++++++++++ tests/virjsondata/deflatten-qemu-sheepdog-out.json | 13 +++++++++++++ tests/virjsontest.c | 1 + tests/virstoragetest.c | 10 ++++++++++ 4 files changed, 35 insertions(+) create mode 100644 tests/virjsondata/deflatten-qemu-sheepdog-in.json create mode 100644 tests/virjsondata/deflatten-qemu-sheepdog-out.json diff --git a/tests/virjsondata/deflatten-qemu-sheepdog-in.json b/tests/virjsondata/deflatten-qemu-sheepdog-in.json new file mode 100644 index 000000000..7c0286300 --- /dev/null +++ b/tests/virjsondata/deflatten-qemu-sheepdog-in.json @@ -0,0 +1,11 @@ +{ + "driver": "raw", + "file": { + "server.host": "10.10.10.10", + "server.port": "7000", + "tag": "", + "driver": "sheepdog", + "server.type": "inet", + "vdi": "Alice" + } +} diff --git a/tests/virjsondata/deflatten-qemu-sheepdog-out.json b/tests/virjsondata/deflatten-qemu-sheepdog-out.json new file mode 100644 index 000000000..258b44a76 --- /dev/null +++ b/tests/virjsondata/deflatten-qemu-sheepdog-out.json @@ -0,0 +1,13 @@ +{ + "driver": "raw", + "file": { + "server": { + "host": "10.10.10.10", + "port": "7000", + "type": "inet" + }, + "tag": "", + "driver": "sheepdog", + "vdi": "Alice" + } +} diff --git a/tests/virjsontest.c b/tests/virjsontest.c index b3ce6591a..5f89a3e1c 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -521,6 +521,7 @@ mymain(void) DO_TEST_DEFLATTEN("double-key", false); DO_TEST_DEFLATTEN("concat", true); DO_TEST_DEFLATTEN("concat-double-key", false); + DO_TEST_DEFLATTEN("qemu-sheepdog", true); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index f34408395..90fcf36ca 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1575,6 +1575,16 @@ mymain(void) "<source protocol='sheepdog' name='test'>\n" " <host name='example.com' port='321'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"driver\": \"raw\"," + "\"file\": {\"server.host\": \"10.10.10.10\"," + "\"server.port\": \"7000\"," + "\"tag\": \"\"," + "\"driver\": \"sheepdog\"," + "\"server.type\": \"inet\"," + "\"vdi\": \"Alice\"}}", + "<source protocol='sheepdog' name='Alice'>\n" + " <host name='10.10.10.10' port='7000'/>\n" + "</source>\n"); #endif /* WITH_YAJL */ cleanup: -- 2.12.2