[PATCH 0/6] Parse back some legacy backing store strings
See patch 4 for code and 6 for what we try to parse back. Peter Krempa (6): virBitmapNewEmpty: Use g_new0 to allocate and remove error checking virJSONValueObjectDeflattenWorker: Refactor cleanup util: json: Extract deflattening of keys into a separate function virjson: Deflatten arrays generated by the json->commandline generator jsontest: Add test cases for deflattening of arrays tests: virstoragetest: validate that array deflattening works for gluster src/util/virbitmap.c | 14 +-- src/util/virhostcpu.c | 6 +- src/util/virjson.c | 105 ++++++++++++++---- src/util/virtpm.c | 3 +- tests/virbitmaptest.c | 8 +- .../deflatten-dotted-array-in.json | 27 +++++ .../deflatten-dotted-array-out.json | 43 +++++++ tests/virjsontest.c | 1 + tests/virstoragetest.c | 18 +++ 9 files changed, 182 insertions(+), 43 deletions(-) create mode 100644 tests/virjsondata/deflatten-dotted-array-in.json create mode 100644 tests/virjsondata/deflatten-dotted-array-out.json -- 2.24.1
virBitmapNewEmpty can't fail now so we can make it obvious and fix all callers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 14 +++----------- src/util/virhostcpu.c | 6 ++---- src/util/virtpm.c | 3 +-- tests/virbitmaptest.c | 8 ++------ 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index baf77fe556..d38a2dd7e9 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -110,17 +110,12 @@ virBitmapNew(size_t size) * * Allocate an empty bitmap. It can be used with self-expanding APIs. * - * Returns a pointer to the allocated bitmap or NULL if memory cannot be - * allocated. Reports libvirt errors. + * Returns a pointer to the allocated bitmap. */ virBitmapPtr virBitmapNewEmpty(void) { - virBitmapPtr ret; - - ignore_value(VIR_ALLOC(ret)); - - return ret; + return g_new0(virBitmap, 1); } @@ -610,16 +605,13 @@ virBitmapParse(const char *str, virBitmapPtr virBitmapParseUnlimited(const char *str) { - virBitmapPtr bitmap; + virBitmapPtr bitmap = virBitmapNewEmpty(); bool neg = false; const char *cur = str; char *tmp; size_t i; int start, last; - if (!(bitmap = virBitmapNewEmpty())) - return NULL; - if (!str) goto error; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 67d3e3308b..41033b7473 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -336,8 +336,7 @@ virHostCPUParseNode(const char *node, goto cleanup; /* enumerate sockets in the node */ - if (!(sockets_map = virBitmapNewEmpty())) - goto cleanup; + sockets_map = virBitmapNewEmpty(); while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) { if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1) @@ -373,8 +372,7 @@ virHostCPUParseNode(const char *node, goto cleanup; for (i = 0; i < sock_max; i++) - if (!(cores_maps[i] = virBitmapNewEmpty())) - goto cleanup; + cores_maps[i] = virBitmapNewEmpty(); /* Iterate over all CPUs in the node, in ascending order */ for (cpu = 0; cpu < npresent_cpus; cpu++) { diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 505a4230dd..c734bf941a 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -183,8 +183,7 @@ virTPMExecGetCaps(virCommandPtr cmd, if (virCommandRun(cmd, &exitstatus) < 0) return NULL; - if (!(bitmap = virBitmapNewEmpty())) - return NULL; + bitmap = virBitmapNewEmpty(); /* older version does not support --print-capabilties -- that's fine */ if (exitstatus != 0) { diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 2808d9c880..1c7dc1d610 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -193,8 +193,7 @@ test4(const void *data G_GNUC_UNUSED) /* 0. empty set */ - if (!(bitmap = virBitmapNewEmpty())) - goto error; + bitmap = virBitmapNewEmpty(); if (virBitmapNextSetBit(bitmap, -1) != -1) goto error; @@ -632,12 +631,9 @@ test11(const void *opaque) static int test12(const void *opaque G_GNUC_UNUSED) { - virBitmapPtr map = NULL; + virBitmapPtr map = virBitmapNewEmpty(); int ret = -1; - if (!(map = virBitmapNewEmpty())) - return -1; - TEST_MAP(0, ""); if (virBitmapSetBitExpand(map, 128) < 0) -- 2.24.1
Use automatic memory handling to remove the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virjson.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 2d7368b0b6..f308927fa0 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -2055,11 +2055,10 @@ virJSONValueObjectDeflattenWorker(const char *key, void *opaque) { virJSONValuePtr retobj = opaque; - virJSONValuePtr newval = NULL; + g_autoptr(virJSONValue) newval = NULL; virJSONValuePtr existobj; - char **tokens = NULL; + VIR_AUTOSTRINGLIST tokens = NULL; size_t ntokens = 0; - int ret = -1; /* non-nested keys only need to be copied */ if (!strchr(key, '.')) { @@ -2075,46 +2074,42 @@ virJSONValueObjectDeflattenWorker(const char *key, if (virJSONValueObjectHasKey(retobj, key)) { virReportError(VIR_ERR_INVALID_ARG, _("can't deflatten colliding key '%s'"), key); - goto cleanup; + return -1; } if (virJSONValueObjectAppend(retobj, key, newval) < 0) - goto cleanup; + return -1; + + newval = NULL; return 0; } if (!(tokens = virStringSplitCount(key, ".", 2, &ntokens))) - goto cleanup; + return -1; if (ntokens != 2) { virReportError(VIR_ERR_INVALID_ARG, _("invalid nested value key '%s'"), key); - goto cleanup; + return -1; } if (!(existobj = virJSONValueObjectGet(retobj, tokens[0]))) { existobj = virJSONValueNewObject(); if (virJSONValueObjectAppend(retobj, tokens[0], existobj) < 0) - goto cleanup; + return -1; } else { if (!virJSONValueIsObject(existobj)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("mixing nested objects and values is forbidden in " "JSON deflattening")); - goto cleanup; + return -1; } } - ret = virJSONValueObjectDeflattenWorker(tokens[1], value, existobj); - - cleanup: - virStringListFreeCount(tokens, ntokens); - virJSONValueFree(newval); - - return ret; + return virJSONValueObjectDeflattenWorker(tokens[1], value, existobj); } -- 2.24.1
Extract the code so that there's a clean separation once we'll want do do other steps. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virjson.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index f308927fa0..65d6521789 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -2049,6 +2049,10 @@ virJSONStringReformat(const char *jsonstr, } +static virJSONValuePtr +virJSONValueObjectDeflattenKeys(virJSONValuePtr json); + + static int virJSONValueObjectDeflattenWorker(const char *key, virJSONValuePtr value, @@ -2064,7 +2068,7 @@ virJSONValueObjectDeflattenWorker(const char *key, if (!strchr(key, '.')) { if (virJSONValueIsObject(value)) - newval = virJSONValueObjectDeflatten(value); + newval = virJSONValueObjectDeflattenKeys(value); else newval = virJSONValueCopy(value); @@ -2113,6 +2117,20 @@ virJSONValueObjectDeflattenWorker(const char *key, } +static virJSONValuePtr +virJSONValueObjectDeflattenKeys(virJSONValuePtr json) +{ + g_autoptr(virJSONValue) deflattened = virJSONValueNewObject(); + + if (virJSONValueObjectForeachKeyValue(json, + virJSONValueObjectDeflattenWorker, + deflattened) < 0) + return NULL; + + return g_steal_pointer(&deflattened); +} + + /** * virJSONValueObjectDeflatten: * @@ -2128,12 +2146,5 @@ virJSONValueObjectDeflattenWorker(const char *key, virJSONValuePtr virJSONValueObjectDeflatten(virJSONValuePtr json) { - g_autoptr(virJSONValue) deflattened = virJSONValueNewObject(); - - if (virJSONValueObjectForeachKeyValue(json, - virJSONValueObjectDeflattenWorker, - deflattened) < 0) - return NULL; - - return g_steal_pointer(&deflattened); + return virJSONValueObjectDeflattenKeys(json); } -- 2.24.1
For the few instances where we'd generate an array in dotted syntax we should be able to parse it back. Add another step in deflattening of the dotted syntax which reconstructs the arrays so that the backing store parser can parse it. https://bugzilla.redhat.com/show_bug.cgi?id=1466177 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virjson.c | 61 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 65d6521789..be472d49e4 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -2131,6 +2131,58 @@ virJSONValueObjectDeflattenKeys(virJSONValuePtr json) } +/** + * virJSONValueObjectDeflattenArrays: + * + * Reconstruct JSON arrays from objects which only have sequential numeric + * keys starting from 0. + */ +static void +virJSONValueObjectDeflattenArrays(virJSONValuePtr json) +{ + g_autofree virJSONValuePtr *arraymembers = NULL; + virJSONObjectPtr obj; + size_t i; + + if (!json || + json->type != VIR_JSON_TYPE_OBJECT) + return; + + obj = &json->data.object; + + arraymembers = g_new0(virJSONValuePtr, obj->npairs); + + for (i = 0; i < obj->npairs; i++) + virJSONValueObjectDeflattenArrays(obj->pairs[i].value); + + for (i = 0; i < obj->npairs; i++) { + virJSONObjectPairPtr pair = obj->pairs + i; + unsigned int keynum; + + if (virStrToLong_uip(pair->key, NULL, 10, &keynum) < 0) + return; + + if (keynum >= obj->npairs) + return; + + if (arraymembers[keynum]) + return; + + arraymembers[keynum] = pair->value; + } + + for (i = 0; i < obj->npairs; i++) + g_free(obj->pairs[i].key); + + g_free(json->data.object.pairs); + + i = obj->npairs; + json->type = VIR_JSON_TYPE_ARRAY; + json->data.array.nvalues = i; + json->data.array.values = g_steal_pointer(&arraymembers); +} + + /** * virJSONValueObjectDeflatten: * @@ -2146,5 +2198,12 @@ virJSONValueObjectDeflattenKeys(virJSONValuePtr json) virJSONValuePtr virJSONValueObjectDeflatten(virJSONValuePtr json) { - return virJSONValueObjectDeflattenKeys(json); + virJSONValuePtr deflattened; + + if (!(deflattened = virJSONValueObjectDeflattenKeys(json))) + return NULL; + + virJSONValueObjectDeflattenArrays(deflattened); + + return deflattened; } -- 2.24.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../deflatten-dotted-array-in.json | 27 ++++++++++++ .../deflatten-dotted-array-out.json | 43 +++++++++++++++++++ tests/virjsontest.c | 1 + 3 files changed, 71 insertions(+) create mode 100644 tests/virjsondata/deflatten-dotted-array-in.json create mode 100644 tests/virjsondata/deflatten-dotted-array-out.json diff --git a/tests/virjsondata/deflatten-dotted-array-in.json b/tests/virjsondata/deflatten-dotted-array-in.json new file mode 100644 index 0000000000..06486a8f38 --- /dev/null +++ b/tests/virjsondata/deflatten-dotted-array-in.json @@ -0,0 +1,27 @@ +{ + "valid": { + "0": "test", + "1": { "something": 123 }, + "2": true + }, + "outoforder": { + "1": { "something": 123 }, + "2": true, + "0": "test" + }, + "invalid-overflow": { + "1": { "something": 123 }, + "2": true, + "4": "test" + }, + "invalid-strings": { + "1": { "something": 123 }, + "2": true, + "test": "test" + }, + "nestedkeys": { + "test.0.test": 123, + "test.2.test": 123, + "test.1.test": 123 + } +} diff --git a/tests/virjsondata/deflatten-dotted-array-out.json b/tests/virjsondata/deflatten-dotted-array-out.json new file mode 100644 index 0000000000..b32b4b14a9 --- /dev/null +++ b/tests/virjsondata/deflatten-dotted-array-out.json @@ -0,0 +1,43 @@ +{ + "valid": [ + "test", + { + "something": 123 + }, + true + ], + "outoforder": [ + "test", + { + "something": 123 + }, + true + ], + "invalid-overflow": { + "1": { + "something": 123 + }, + "2": true, + "4": "test" + }, + "invalid-strings": { + "1": { + "something": 123 + }, + "2": true, + "test": "test" + }, + "nestedkeys": { + "test": [ + { + "test": 123 + }, + { + "test": 123 + }, + { + "test": 123 + } + ] + } +} diff --git a/tests/virjsontest.c b/tests/virjsontest.c index 9da0f9c90d..77ca6b449b 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -609,6 +609,7 @@ mymain(void) DO_TEST_DEFLATTEN("concat", true); DO_TEST_DEFLATTEN("concat-double-key", false); DO_TEST_DEFLATTEN("qemu-sheepdog", true); + DO_TEST_DEFLATTEN("dotted-array", true); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.24.1
Validate that we are able to parse back the dotted syntax arrays we were generating in the pre-blockdev era. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virstoragetest.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index c59511114d..547118951e 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1387,6 +1387,24 @@ mymain(void) " <host transport='unix' socket='/path/socket'/>\n" " <host name='example.com' port='24007'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"driver\": \"raw\"," + "\"file\": {\"server.0.host\": \"A.A.A.A\"," + "\"server.1.host\": \"B.B.B.B\"," + "\"server.2.host\": \"C.C.C.C\"," + "\"driver\": \"gluster\"," + "\"path\": \"raw\"," + "\"server.0.type\": \"tcp\"," + "\"server.1.type\": \"tcp\"," + "\"server.2.type\": \"tcp\"," + "\"server.0.port\": \"24007\"," + "\"server.1.port\": \"24007\"," + "\"server.2.port\": \"24007\"," + "\"volume\": \"vol1\"}}", + "<source protocol='gluster' name='vol1/raw'>\n" + " <host name='A.A.A.A' port='24007'/>\n" + " <host name='B.B.B.B' port='24007'/>\n" + " <host name='C.C.C.C' port='24007'/>\n" + "</source>\n"); TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nbd\"," "\"path\":\"/path/to/socket\"" "}" -- 2.24.1
On 19. 3. 2020 20:35, Peter Krempa wrote:
See patch 4 for code and 6 for what we try to parse back.
Peter Krempa (6): virBitmapNewEmpty: Use g_new0 to allocate and remove error checking virJSONValueObjectDeflattenWorker: Refactor cleanup util: json: Extract deflattening of keys into a separate function virjson: Deflatten arrays generated by the json->commandline generator jsontest: Add test cases for deflattening of arrays tests: virstoragetest: validate that array deflattening works for gluster
src/util/virbitmap.c | 14 +-- src/util/virhostcpu.c | 6 +- src/util/virjson.c | 105 ++++++++++++++---- src/util/virtpm.c | 3 +- tests/virbitmaptest.c | 8 +- .../deflatten-dotted-array-in.json | 27 +++++ .../deflatten-dotted-array-out.json | 43 +++++++ tests/virjsontest.c | 1 + tests/virstoragetest.c | 18 +++ 9 files changed, 182 insertions(+), 43 deletions(-) create mode 100644 tests/virjsondata/deflatten-dotted-array-in.json create mode 100644 tests/virjsondata/deflatten-dotted-array-out.json
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa