[libvirt] [PATCH 0/3] JSON parsing simplification
Add and use a new virJSONValueObjectGetObject() and friends, exposing and fixing a bug in our JSON parser along the way. Eric Blake (3): json: fully parse input string json: make it easier to type-check when getting from object qemu: simplify json parsing src/libvirt_private.syms | 3 + src/qemu/qemu_monitor_json.c | 164 ++++++++++++++++--------------------------- src/util/virjson.c | 80 +++++++++++---------- src/util/virjson.h | 8 ++- tests/jsontest.c | 129 ++++++++++++++++++++++++++++++++++ 5 files changed, 242 insertions(+), 142 deletions(-) -- 2.4.3
I was adding a JSON test, and was shocked to find out our parser treated the input string of "1" as invalid JSON. It turns out that YAJL specifically documents that it buffers input, and that if the last input read could be a prefix to a longer token, then you have to explicitly tell the parser that the buffer has ended before that token will be processed. It doesn't help that yajl 2 renamed the function from what it was in yajl 1. * src/util/virjson.c (virJSONValueFromString): Complete parse, in case buffer ends in possible token prefix. * tests/jsontest.c (mymain): Expose the problem. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/util/virjson.c | 4 +++- tests/jsontest.c | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 40ec613..7d4ece6 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -38,6 +38,7 @@ # define yajl_size_t size_t # else # define yajl_size_t unsigned int +# define yajl_complete_parse yajl_parse_complete # endif #endif @@ -1610,7 +1611,8 @@ virJSONValueFromString(const char *jsonstring) if (yajl_parse(hand, (const unsigned char *)jsonstring, - strlen(jsonstring)) != yajl_status_ok) { + strlen(jsonstring)) != yajl_status_ok || + yajl_complete_parse(hand) != yajl_status_ok) { unsigned char *errstr = yajl_get_error(hand, 1, (const unsigned char*)jsonstring, strlen(jsonstring)); diff --git a/tests/jsontest.c b/tests/jsontest.c index c080946..f226c92 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -304,6 +304,12 @@ mymain(void) DO_TEST_PARSE("string", "[ \"The meaning of life\" ]"); DO_TEST_PARSE_FAIL("unterminated string", "[ \"The meaning of lif ]"); + DO_TEST_PARSE("integer", "1"); + DO_TEST_PARSE("boolean", "true"); + DO_TEST_PARSE("null", "null"); + DO_TEST_PARSE_FAIL("incomplete keyword", "tr"); + DO_TEST_PARSE_FAIL("overdone keyword", "truest"); + DO_TEST_PARSE_FAIL("unknown keyword", "huh"); DO_TEST_PARSE_FAIL("object with numeric keys", "{ 1:1, 2:1, 3:2 }"); DO_TEST_PARSE_FAIL("unterminated object", "{ \"1\":1, \"2\":1, \"3\":2"); -- 2.4.3
While working in qemu_monitor_json, I repeatedly found myself getting a value then checking if it was an object. Add some wrappers to make this task easier. * src/util/virjson.c (virJSONValueObjectGetByType) (virJSONValueObjectGetObject, virJSONValueObjectGetArray): New functions. (virJSONValueObjectGetString, virJSONValueObjectGetNumberInt) (virJSONValueObjectGetNumberUint) (virJSONValueObjectGetNumberLong) (virJSONValueObjectGetNumberUlong) (virJSONValueObjectGetNumberDouble) (virJSONValueObjectGetBoolean): Simplify. (virJSONValueIsNull): Change return type. * src/util/virjson.h: Reflect changes. * src/libvirt_private.syms (virjson.h): Export them. * tests/jsontest.c (testJSONLookup): New test. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/libvirt_private.syms | 3 ++ src/util/virjson.c | 76 +++++++++++++++-------------- src/util/virjson.h | 8 ++- tests/jsontest.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 38 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 51044b0..1566d11 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1632,13 +1632,16 @@ virJSONValueObjectCreate; virJSONValueObjectCreateVArgs; virJSONValueObjectForeachKeyValue; virJSONValueObjectGet; +virJSONValueObjectGetArray; virJSONValueObjectGetBoolean; +virJSONValueObjectGetByType; virJSONValueObjectGetKey; virJSONValueObjectGetNumberDouble; virJSONValueObjectGetNumberInt; virJSONValueObjectGetNumberLong; virJSONValueObjectGetNumberUint; virJSONValueObjectGetNumberUlong; +virJSONValueObjectGetObject; virJSONValueObjectGetString; virJSONValueObjectGetValue; virJSONValueObjectHasKey; diff --git a/src/util/virjson.c b/src/util/virjson.c index 7d4ece6..29e2c39 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -766,6 +766,21 @@ virJSONValueObjectGet(virJSONValuePtr object, } +/* Return the value associated with KEY within OBJECT, but return NULL + * if the key is missing or if value is not the correct TYPE. */ +virJSONValuePtr +virJSONValueObjectGetByType(virJSONValuePtr object, + const char *key, + virJSONType type) +{ + virJSONValuePtr value = virJSONValueObjectGet(object, key); + + if (value && value->type == type) + return value; + return NULL; +} + + int virJSONValueObjectKeysNumber(virJSONValuePtr object) { @@ -1057,13 +1072,10 @@ virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap) } -int +bool virJSONValueIsNull(virJSONValuePtr val) { - if (val->type != VIR_JSON_TYPE_NULL) - return 0; - - return 1; + return val->type == VIR_JSON_TYPE_NULL; } @@ -1071,11 +1083,8 @@ const char * virJSONValueObjectGetString(virJSONValuePtr object, const char *key) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return NULL; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return NULL; @@ -1088,11 +1097,8 @@ virJSONValueObjectGetNumberInt(virJSONValuePtr object, const char *key, int *value) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return -1; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return -1; @@ -1105,11 +1111,8 @@ virJSONValueObjectGetNumberUint(virJSONValuePtr object, const char *key, unsigned int *value) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return -1; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return -1; @@ -1122,11 +1125,8 @@ virJSONValueObjectGetNumberLong(virJSONValuePtr object, const char *key, long long *value) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return -1; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return -1; @@ -1139,11 +1139,8 @@ virJSONValueObjectGetNumberUlong(virJSONValuePtr object, const char *key, unsigned long long *value) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return -1; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return -1; @@ -1156,11 +1153,8 @@ virJSONValueObjectGetNumberDouble(virJSONValuePtr object, const char *key, double *value) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return -1; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return -1; @@ -1173,11 +1167,8 @@ virJSONValueObjectGetBoolean(virJSONValuePtr object, const char *key, bool *value) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return -1; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return -1; @@ -1185,15 +1176,26 @@ virJSONValueObjectGetBoolean(virJSONValuePtr object, } +virJSONValuePtr +virJSONValueObjectGetObject(virJSONValuePtr object, const char *key) +{ + return virJSONValueObjectGetByType(object, key, VIR_JSON_TYPE_OBJECT); +} + + +virJSONValuePtr +virJSONValueObjectGetArray(virJSONValuePtr object, const char *key) +{ + return virJSONValueObjectGetByType(object, key, VIR_JSON_TYPE_ARRAY); +} + + int virJSONValueObjectIsNull(virJSONValuePtr object, const char *key) { - virJSONValuePtr val; - if (object->type != VIR_JSON_TYPE_OBJECT) - return -1; + virJSONValuePtr val = virJSONValueObjectGet(object, key); - val = virJSONValueObjectGet(object, key); if (!val) return -1; diff --git a/src/util/virjson.h b/src/util/virjson.h index e871b2e..a7df6e5 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -110,6 +110,8 @@ int virJSONValueArrayAppend(virJSONValuePtr object, virJSONValuePtr value); int virJSONValueObjectHasKey(virJSONValuePtr object, const char *key); virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr object, const char *key); +virJSONValuePtr virJSONValueObjectGetByType(virJSONValuePtr object, + const char *key, virJSONType type); bool virJSONValueIsArray(virJSONValuePtr array); int virJSONValueArraySize(const virJSONValue *array); @@ -129,7 +131,11 @@ 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); +bool virJSONValueIsNull(virJSONValuePtr object); +virJSONValuePtr virJSONValueObjectGetObject(virJSONValuePtr object, + const char *key); +virJSONValuePtr virJSONValueObjectGetArray(virJSONValuePtr object, + const char *key); const char *virJSONValueObjectGetString(virJSONValuePtr object, const char *key); int virJSONValueObjectGetNumberInt(virJSONValuePtr object, const char *key, int *value); diff --git a/tests/jsontest.c b/tests/jsontest.c index f226c92..34a07ee 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -119,6 +119,114 @@ testJSONAddRemove(const void *data) static int +testJSONLookup(const void *data) +{ + const struct testInfo *info = data; + virJSONValuePtr json; + virJSONValuePtr value = NULL; + char *result = NULL; + int rc; + int number; + const char *str; + int ret = -1; + + json = virJSONValueFromString(info->doc); + if (!json) { + VIR_TEST_VERBOSE("Fail to parse %s\n", info->doc); + ret = -1; + goto cleanup; + } + + value = virJSONValueObjectGetObject(json, "a"); + if (value) { + if (!info->pass) { + VIR_TEST_VERBOSE("lookup for 'a' in '%s' should have failed\n", + info->doc); + goto cleanup; + } else { + result = virJSONValueToString(value, false); + if (STRNEQ_NULLABLE(result, "{}")) { + VIR_TEST_VERBOSE("lookup for 'a' in '%s' found '%s' but " + "should have found '{}'\n", + info->doc, NULLSTR(result)); + goto cleanup; + } + VIR_FREE(result); + } + } else if (info->pass) { + VIR_TEST_VERBOSE("lookup for 'a' in '%s' should have succeeded\n", + info->doc); + goto cleanup; + } + + number = 2; + rc = virJSONValueObjectGetNumberInt(json, "b", &number); + if (rc == 0) { + if (!info->pass) { + VIR_TEST_VERBOSE("lookup for 'b' in '%s' should have failed\n", + info->doc); + goto cleanup; + } else if (number != 1) { + VIR_TEST_VERBOSE("lookup for 'b' in '%s' found %d but " + "should have found 1\n", + info->doc, number); + goto cleanup; + } + } else if (info->pass) { + VIR_TEST_VERBOSE("lookup for 'b' in '%s' should have succeeded\n", + info->doc); + goto cleanup; + } + + str = virJSONValueObjectGetString(json, "c"); + if (str) { + if (!info->pass) { + VIR_TEST_VERBOSE("lookup for 'c' in '%s' should have failed\n", + info->doc); + goto cleanup; + } else if (STRNEQ(str, "str")) { + VIR_TEST_VERBOSE("lookup for 'c' in '%s' found '%s' but " + "should have found 'str'\n", info->doc, str); + goto cleanup; + } + } else if (info->pass) { + VIR_TEST_VERBOSE("lookup for 'c' in '%s' should have succeeded\n", + info->doc); + goto cleanup; + } + + value = virJSONValueObjectGetArray(json, "d"); + if (value) { + if (!info->pass) { + VIR_TEST_VERBOSE("lookup for 'd' in '%s' should have failed\n", + info->doc); + goto cleanup; + } else { + result = virJSONValueToString(value, false); + if (STRNEQ_NULLABLE(result, "[]")) { + VIR_TEST_VERBOSE("lookup for 'd' in '%s' found '%s' but " + "should have found '[]'\n", + info->doc, NULLSTR(result)); + goto cleanup; + } + VIR_FREE(result); + } + } else if (info->pass) { + VIR_TEST_VERBOSE("lookup for 'd' in '%s' should have succeeded\n", + info->doc); + goto cleanup; + } + + ret = 0; + + cleanup: + virJSONValueFree(json); + VIR_FREE(result); + return ret; +} + + +static int testJSONCopy(const void *data) { const struct testInfo *info = data; @@ -319,6 +427,21 @@ mymain(void) "[ {[\"key1\", \"key2\"]: \"value\"} ]"); DO_TEST_PARSE_FAIL("object with unterminated key", "{ \"key:7 }"); + DO_TEST_FULL("lookup on array", Lookup, + "[ 1 ]", NULL, false); + DO_TEST_FULL("lookup on string", Lookup, + "\"str\"", NULL, false); + DO_TEST_FULL("lookup on integer", Lookup, + "1", NULL, false); + DO_TEST_FULL("lookup with missing key", Lookup, + "{ }", NULL, false); + DO_TEST_FULL("lookup with wrong type", Lookup, + "{ \"a\": 1, \"b\": \"str\", \"c\": [], \"d\": {} }", + NULL, false); + DO_TEST_FULL("lookup with correct type", Lookup, + "{ \"a\": {}, \"b\": 1, \"c\": \"str\", \"d\": [] }", + NULL, true); + return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.3
Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONIOProcessEvent) (qemuMonitorJSONCommandWithFd, qemuMonitorJSONHandleGraphics) (qemuMonitorJSONGetStatus, qemuMonitorJSONExtractCPUInfo) (qemuMonitorJSONGetVirtType, qemuMonitorJSONGetBalloonInfo) (qemuMonitorJSONGetMemoryStats) (qemuMonitorJSONDevGetBlockExtent) (qemuMonitorJSONGetOneBlockStatsInfo) (qemuMonitorJSONGetAllBlockStatsInfo) (qemuMonitorJSONBlockStatsUpdateCapacityOne) (qemuMonitorJSONBlockStatsUpdateCapacity) (qemuMonitorJSONGetBlockExtent) (qemuMonitorJSONGetMigrationStatusReply) (qemuMonitorJSONGetDumpGuestMemoryCapability) (qemuMonitorJSONAddFd, qemuMonitorJSONQueryRxFilterParse) (qemuMonitorJSONExtractChardevInfo) (qemuMonitorJSONDiskNameLookupOne) (qemuMonitorJSONDiskNameLookup) (qemuMonitorJSONGetAllBlockJobInfo) (qemuMonitorJSONBlockIoThrottleInfo, qemuMonitorJSONGetVersion) (qemuMonitorJSONGetMachines, qemuMonitorJSONGetCPUDefinitions) (qemuMonitorJSONGetCommands, qemuMonitorJSONGetEvents) (qemuMonitorJSONGetKVMState, qemuMonitorJSONGetObjectTypes) (qemuMonitorJSONGetObjectListPaths) (qemuMonitorJSONGetObjectProps, qemuMonitorJSONGetTargetArch) (qemuMonitorJSONGetMigrationCapabilities) (qemuMonitorJSONGetStringArray, qemuMonitorJSONAttachCharDev) (qemuMonitorJSONGetCPUx86Data, qemuMonitorJSONGetIOThreads) (qemuMonitorJSONGetMemoryDeviceInfo): Use shorter idioms. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor_json.c | 164 ++++++++++++++++--------------------------- 1 file changed, 61 insertions(+), 103 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 0ba549e..a530b53 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -154,12 +154,10 @@ qemuMonitorJSONIOProcessEvent(qemuMonitorPtr mon, if ((data = virJSONValueObjectGet(obj, "data"))) details = virJSONValueToString(data, false); if ((timestamp = virJSONValueObjectGet(obj, "timestamp"))) { - virJSONValuePtr elt; - - if ((elt = virJSONValueObjectGet(timestamp, "seconds"))) - ignore_value(virJSONValueGetNumberLong(elt, &seconds)); - if ((elt = virJSONValueObjectGet(timestamp, "microseconds"))) - ignore_value(virJSONValueGetNumberUint(elt, µs)); + ignore_value(virJSONValueObjectGetNumberLong(timestamp, "seconds", + &seconds)); + ignore_value(virJSONValueObjectGetNumberUint(timestamp, "microseconds", + µs)); } qemuMonitorEmitEvent(mon, type, seconds, micros, details); VIR_FREE(details); @@ -271,7 +269,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon, memset(&msg, 0, sizeof(msg)); - exe = virJSONValueObjectGet(cmd, "execute"); + exe = virJSONValueObjectGetObject(cmd, "execute"); if (exe) { if (!(id = qemuMonitorNextCommandID(mon))) goto cleanup; @@ -630,11 +628,11 @@ static void qemuMonitorJSONHandleGraphics(qemuMonitorPtr mon, virJSONValuePtr da virJSONValuePtr client; virJSONValuePtr server; - if (!(client = virJSONValueObjectGet(data, "client"))) { + if (!(client = virJSONValueObjectGetObject(data, "client"))) { VIR_WARN("missing client info in VNC event"); return; } - if (!(server = virJSONValueObjectGet(data, "server"))) { + if (!(server = virJSONValueObjectGetObject(data, "server"))) { VIR_WARN("missing server info in VNC event"); return; } @@ -1080,7 +1078,7 @@ qemuMonitorJSONGetStatus(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-status reply was missing return data")); goto cleanup; @@ -1183,18 +1181,12 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply, int *threads = NULL; int ncpus; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpu reply was missing return data")); goto cleanup; } - if (data->type != VIR_JSON_TYPE_ARRAY) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cpu information was not an array")); - goto cleanup; - } - if ((ncpus = virJSONValueArraySize(data)) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpu information was empty")); @@ -1282,7 +1274,7 @@ int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, virJSONValuePtr data; bool val = false; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("info kvm reply was missing return data")); ret = -1; @@ -1405,7 +1397,7 @@ qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, virJSONValuePtr data; unsigned long long mem; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("info balloon reply was missing return data")); ret = -1; @@ -1500,7 +1492,7 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) goto cleanup; - if ((data = virJSONValueObjectGet(reply, "error"))) { + if ((data = virJSONValueObjectGetObject(reply, "error"))) { const char *klass = virJSONValueObjectGetString(data, "class"); const char *desc = virJSONValueObjectGetString(data, "desc"); @@ -1515,7 +1507,7 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, if ((ret = qemuMonitorJSONCheckError(cmd, reply)) < 0) goto cleanup; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("qom-get reply was missing return data")); goto cleanup; @@ -1598,8 +1590,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, ret = -1; - devices = virJSONValueObjectGet(reply, "return"); - if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + if (!(devices = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("block info reply was missing device list")); goto cleanup; @@ -1686,15 +1677,11 @@ qemuMonitorJSONDevGetBlockExtent(virJSONValuePtr dev, virJSONValuePtr stats; virJSONValuePtr parent; - if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || - parent->type != VIR_JSON_TYPE_OBJECT) { + if ((parent = virJSONValueObjectGetObject(dev, "parent")) == NULL) return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT; - } - if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || - stats->type != VIR_JSON_TYPE_OBJECT) { + if ((stats = virJSONValueObjectGetObject(parent, "stats")) == NULL) return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS; - } if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", extent) < 0) { @@ -1724,8 +1711,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, if (VIR_ALLOC(bstats) < 0) goto cleanup; - if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || - stats->type != VIR_JSON_TYPE_OBJECT) { + if ((stats = virJSONValueObjectGetObject(dev, "stats")) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats stats entry was not " "in expected format")); @@ -1759,7 +1745,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, bstats = NULL; if (backingChain && - (backing = virJSONValueObjectGet(dev, "backing")) && + (backing = virJSONValueObjectGetObject(dev, "backing")) && qemuMonitorJSONGetOneBlockStatsInfo(backing, dev_name, depth + 1, hash, true) < 0) goto cleanup; @@ -1794,8 +1780,7 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - devices = virJSONValueObjectGet(reply, "return"); - if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + if (!(devices = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats reply was missing device list")); goto cleanup; @@ -1873,7 +1858,7 @@ qemuMonitorJSONBlockStatsUpdateCapacityOne(virJSONValuePtr image, bstats->physical = bstats->capacity; if (backingChain && - (backing = virJSONValueObjectGet(image, "backing-image"))) { + (backing = virJSONValueObjectGetObject(image, "backing-image"))) { ret = qemuMonitorJSONBlockStatsUpdateCapacityOne(backing, dev_name, depth + 1, @@ -1908,8 +1893,7 @@ qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - devices = virJSONValueObjectGet(reply, "return"); - if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + if (!(devices = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-block reply was missing device list")); goto cleanup; @@ -1936,8 +1920,8 @@ qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, } /* drive may be empty */ - if (!(inserted = virJSONValueObjectGet(dev, "inserted")) || - !(image = virJSONValueObjectGet(inserted, "image"))) + if (!(inserted = virJSONValueObjectGetObject(dev, "inserted")) || + !(image = virJSONValueObjectGetObject(inserted, "image"))) continue; if (qemuMonitorJSONBlockStatsUpdateCapacityOne(image, dev_name, 0, @@ -2010,8 +1994,7 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, goto cleanup; ret = -1; - devices = virJSONValueObjectGet(reply, "return"); - if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + if (!(devices = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats reply was missing device list")); goto cleanup; @@ -2483,7 +2466,7 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, int rc; double mbps; - if (!(ret = virJSONValueObjectGet(reply, "return"))) { + if (!(ret = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("info migration reply was missing return data")); return -1; @@ -2521,7 +2504,7 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, if (status->status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE || status->status == QEMU_MONITOR_MIGRATION_STATUS_CANCELLING || status->status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { - virJSONValuePtr ram = virJSONValueObjectGet(ret, "ram"); + virJSONValuePtr ram = virJSONValueObjectGetObject(ret, "ram"); if (!ram) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("migration was active, but no RAM info was set")); @@ -2564,7 +2547,7 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal-bytes", &status->ram_normal_bytes)); - virJSONValuePtr disk = virJSONValueObjectGet(ret, "disk"); + virJSONValuePtr disk = virJSONValueObjectGetObject(ret, "disk"); if (disk) { rc = virJSONValueObjectGetNumberUlong(disk, "transferred", &status->disk_transferred); @@ -2600,7 +2583,7 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, } } - virJSONValuePtr comp = virJSONValueObjectGet(ret, "xbzrle-cache"); + virJSONValuePtr comp = virJSONValueObjectGetObject(ret, "xbzrle-cache"); if (comp) { status->xbzrle_set = true; rc = virJSONValueObjectGetNumberUlong(comp, "cache-size", @@ -2757,15 +2740,13 @@ qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, ret = -1; - caps = virJSONValueObjectGet(reply, "return"); - if (!caps || caps->type != VIR_JSON_TYPE_OBJECT) { + if (!(caps = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing dump guest memory capabilities")); goto cleanup; } - formats = virJSONValueObjectGet(caps, "formats"); - if (!formats || formats->type != VIR_JSON_TYPE_ARRAY) { + if (!(formats = virJSONValueObjectGetArray(caps, "formats"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing supported dump formats")); goto cleanup; @@ -2999,16 +2980,14 @@ qemuMonitorJSONAddFd(qemuMonitorPtr mon, int fdset, int fd, const char *name) ret = qemuMonitorJSONCheckError(cmd, reply); } if (ret == 0) { - virJSONValuePtr data = virJSONValueObjectGet(reply, "return"); + virJSONValuePtr data = virJSONValueObjectGetObject(reply, "return"); - if (!data || data->type != VIR_JSON_TYPE_OBJECT) { + if (!data) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing return information")); goto error; } - data = virJSONValueObjectGet(data, "fd"); - if (!data || data->type != VIR_JSON_TYPE_NUMBER || - virJSONValueGetNumberInt(data, &ret) < 0) { + if (virJSONValueObjectGetNumberInt(data, "fd", &ret) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("incomplete return information")); goto error; @@ -3122,16 +3101,11 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg, if (!fil) goto cleanup; - if (!(returnArray = virJSONValueObjectGet(msg, "return"))) { + if (!(returnArray = virJSONValueObjectGetArray(msg, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-rx-filter reply was missing return data")); goto cleanup; } - if (returnArray->type != VIR_JSON_TYPE_ARRAY) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-rx-filter return data was not an array")); - goto cleanup; - } if (!(entry = virJSONValueArrayGet(returnArray, 0))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query -rx-filter return data missing array element")); @@ -3342,18 +3316,12 @@ qemuMonitorJSONExtractChardevInfo(virJSONValuePtr reply, size_t i; qemuMonitorChardevInfoPtr entry = NULL; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("character device reply was missing return data")); goto cleanup; } - if (data->type != VIR_JSON_TYPE_ARRAY) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("character device information was not an array")); - goto cleanup; - } - for (i = 0; i < virJSONValueArraySize(data); i++) { virJSONValuePtr chardev = virJSONValueArrayGet(data, i); const char *type; @@ -3849,7 +3817,7 @@ qemuMonitorJSONDiskNameLookupOne(virJSONValuePtr image, if (!top || !image) return NULL; if (top != target) { - backing = virJSONValueObjectGet(image, "backing-image"); + backing = virJSONValueObjectGetObject(image, "backing-image"); return qemuMonitorJSONDiskNameLookupOne(backing, top->backingStore, target); } @@ -3887,8 +3855,7 @@ qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon, if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - devices = virJSONValueObjectGet(reply, "return"); - if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + if (!(devices = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("block info reply was missing device list")); goto cleanup; @@ -3913,8 +3880,8 @@ qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon, } if (STREQ(thisdev, device)) { - if ((inserted = virJSONValueObjectGet(dev, "inserted")) && - (image = virJSONValueObjectGet(inserted, "image"))) { + if ((inserted = virJSONValueObjectGetObject(dev, "inserted")) && + (image = virJSONValueObjectGetObject(inserted, "image"))) { ret = qemuMonitorJSONDiskNameLookupOne(image, top, target); } break; @@ -4159,18 +4126,12 @@ qemuMonitorJSONGetAllBlockJobInfo(qemuMonitorPtr mon) if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - if ((data = virJSONValueObjectGet(reply, "return")) == NULL) { + if ((data = virJSONValueObjectGetArray(reply, "return")) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("reply was missing return data")); goto cleanup; } - if (data->type != VIR_JSON_TYPE_ARRAY) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unrecognized format of block job information")); - goto cleanup; - } - if ((nr_results = virJSONValueArraySize(data)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to determine array size")); @@ -4390,9 +4351,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, size_t i; bool found = false; - io_throttle = virJSONValueObjectGet(result, "return"); - - if (!io_throttle || io_throttle->type != VIR_JSON_TYPE_ARRAY) { + if (!(io_throttle = virJSONValueObjectGetArray(result, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(" block_io_throttle reply was missing device list")); goto cleanup; @@ -4421,8 +4380,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, continue; found = true; - if ((inserted = virJSONValueObjectGet(temp_dev, "inserted")) == NULL || - inserted->type != VIR_JSON_TYPE_OBJECT) { + if (!(inserted = virJSONValueObjectGetObject(temp_dev, "inserted"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("block_io_throttle inserted entry " "was not in expected format")); @@ -4609,13 +4567,13 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-version reply was missing 'return' data")); goto cleanup; } - if (!(qemu = virJSONValueObjectGet(data, "qemu"))) { + if (!(qemu = virJSONValueObjectGetObject(data, "qemu"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-version reply was missing 'qemu' data")); goto cleanup; @@ -4683,7 +4641,7 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-machines reply was missing return data")); goto cleanup; @@ -4795,7 +4753,7 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-cpu-definitions reply was missing return data")); goto cleanup; @@ -4863,7 +4821,7 @@ int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-commands reply was missing return data")); goto cleanup; @@ -4936,7 +4894,7 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-events reply was missing return data")); goto cleanup; @@ -5131,7 +5089,7 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-kvm reply was missing return data")); goto cleanup; @@ -5179,7 +5137,7 @@ int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("qom-list-types reply was missing return data")); goto cleanup; @@ -5250,7 +5208,7 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("qom-list reply was missing return data")); goto cleanup; @@ -5501,7 +5459,7 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("device-list-properties reply was missing return data")); goto cleanup; @@ -5564,7 +5522,7 @@ qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon) if (rv < 0) goto cleanup; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-target reply was missing return data")); goto cleanup; @@ -5616,7 +5574,7 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon, ret = -1; - if (!(caps = virJSONValueObjectGet(reply, "return")) || + if (!(caps = virJSONValueObjectGetArray(reply, "return")) || (n = virJSONValueArraySize(caps)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing migration capabilities")); @@ -5894,7 +5852,7 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s reply was missing return data"), qmpCmd); @@ -6092,7 +6050,7 @@ qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon, virJSONValuePtr data; const char *path; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("chardev-add reply was missing return data")); goto cleanup; @@ -6260,7 +6218,7 @@ qemuMonitorJSONGetCPUx86Data(qemuMonitorPtr mon, if (qemuMonitorJSONCheckError(cmd, reply)) goto cleanup; - data = virJSONValueObjectGet(reply, "return"); + data = virJSONValueObjectGetArray(reply, "return"); if ((n = virJSONValueArraySize(data)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -6297,7 +6255,7 @@ qemuMonitorJSONGetCPUx86Data(qemuMonitorPtr mon, if (qemuMonitorJSONCheckError(cmd, reply)) goto cleanup; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("qom-get reply was missing return data")); goto cleanup; @@ -6416,7 +6374,7 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-iothreads reply was missing return data")); goto cleanup; @@ -6514,7 +6472,7 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon, ret = -1; - if (!(data = virJSONValueObjectGet(reply, "return"))) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-memory-devices reply was missing return data")); goto cleanup; @@ -6542,7 +6500,7 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon, virJSONValuePtr dimminfo; const char *devalias; - if (!(dimminfo = virJSONValueObjectGet(elem, "data"))) { + if (!(dimminfo = virJSONValueObjectGetObject(elem, "data"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-memory-devices reply data doesn't " "contain enum data")); -- 2.4.3
On 06/20/2015 01:42 PM, Eric Blake wrote:
Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor_json.c | 164 ++++++++++++++++--------------------------- 1 file changed, 61 insertions(+), 103 deletions(-)
@@ -271,7 +269,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon,
memset(&msg, 0, sizeof(msg));
- exe = virJSONValueObjectGet(cmd, "execute"); + exe = virJSONValueObjectGetObject(cmd, "execute"); if (exe) { if (!(id = qemuMonitorNextCommandID(mon))) goto cleanup;
This hunk is spurious (the 'execute' member is a string, not an object); I've removed it locally from my tree. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 22.06.2015 15:52, Eric Blake wrote:
On 06/20/2015 01:42 PM, Eric Blake wrote:
Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor_json.c | 164 ++++++++++++++++--------------------------- 1 file changed, 61 insertions(+), 103 deletions(-)
@@ -271,7 +269,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon,
memset(&msg, 0, sizeof(msg));
- exe = virJSONValueObjectGet(cmd, "execute"); + exe = virJSONValueObjectGetObject(cmd, "execute"); if (exe) { if (!(id = qemuMonitorNextCommandID(mon))) goto cleanup;
This hunk is spurious (the 'execute' member is a string, not an object); I've removed it locally from my tree.
Agreed. ACK to all the patches then. Michal
On 06/22/2015 09:05 AM, Michal Privoznik wrote:
On 22.06.2015 15:52, Eric Blake wrote:
On 06/20/2015 01:42 PM, Eric Blake wrote:
Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_monitor_json.c | 164 ++++++++++++++++--------------------------- 1 file changed, 61 insertions(+), 103 deletions(-)
@@ -271,7 +269,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon,
memset(&msg, 0, sizeof(msg));
- exe = virJSONValueObjectGet(cmd, "execute"); + exe = virJSONValueObjectGetObject(cmd, "execute"); if (exe) { if (!(id = qemuMonitorNextCommandID(mon))) goto cleanup;
This hunk is spurious (the 'execute' member is a string, not an object); I've removed it locally from my tree.
Agreed. ACK to all the patches then.
Thanks; pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 06/22/2015 11:01 AM, Eric Blake wrote:
On 06/22/2015 09:05 AM, Michal Privoznik wrote:
On 22.06.2015 15:52, Eric Blake wrote:
On 06/20/2015 01:42 PM, Eric Blake wrote:
Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with.
Agreed. ACK to all the patches then.
Thanks; pushed.
And now seeing a test failure on 'jsontest' on a RHEL 6 box :( I'll investigate, since I just caused a build failure. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 06/22/2015 11:10 AM, Eric Blake wrote:
On 06/22/2015 11:01 AM, Eric Blake wrote:
On 06/22/2015 09:05 AM, Michal Privoznik wrote:
On 22.06.2015 15:52, Eric Blake wrote:
On 06/20/2015 01:42 PM, Eric Blake wrote:
Rather than grabbing an arbitrary JSON value and then checking if it has the right type, we might as well request the correct type to begin with.
Agreed. ACK to all the patches then.
Thanks; pushed.
And now seeing a test failure on 'jsontest' on a RHEL 6 box :(
I'll investigate, since I just caused a build failure.
Blech. Not only does RHEL 6 lack automatic trailing garbage detection added in yajl 2, it lacks yajl_get_bytes_consumed() added sometime after 1.0.7 and before 1.0.12. I've had to get creative on how to work around the issue, but I'll have patches ready for review shortly. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik