[libvirt] [PATCH 0/2] Fix parsing of iSCSI LUN from JSON backing store strings

Peter Krempa (2): util: json: Add helper to return string or number properties as string util: storage: Parse 'lun' for iSCSI protocol from JSON as string or number src/util/virjson.c | 27 +++++++++++++++++++++++++++ src/util/virjson.h | 1 + src/util/virstoragefile.c | 13 +++++-------- tests/virstoragetest.c | 2 +- 4 files changed, 34 insertions(+), 9 deletions(-) -- 2.15.0

The helper is useful when in cases when JSON we have to parse may return one of the two due to historical reasons and the number value itself would be stored as a string. --- src/util/virjson.c | 27 +++++++++++++++++++++++++++ src/util/virjson.h | 1 + 2 files changed, 28 insertions(+) diff --git a/src/util/virjson.c b/src/util/virjson.c index 17b11f2b3d..14b68b8c93 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1208,6 +1208,33 @@ virJSONValueObjectGetString(virJSONValuePtr object, } +/** + * virJSONValueObjectGetStringOrNumber: + * @object: JSON value object + * @key: name of the property in @object to get + * + * Gets a property named @key from the JSON object @object. The value may be + * a number or a string and is returned as a string. In cases when the property + * is not present or is not a string or number NULL is returned. + */ +const char * +virJSONValueObjectGetStringOrNumber(virJSONValuePtr object, + const char *key) +{ + virJSONValuePtr val = virJSONValueObjectGet(object, key); + + if (!val) + return NULL; + + if (val->type == VIR_JSON_TYPE_STRING) + return val->data.string; + else if (val->type == VIR_JSON_TYPE_NUMBER) + return val->data.number; + + return NULL; +} + + int virJSONValueObjectGetNumberInt(virJSONValuePtr object, const char *key, diff --git a/src/util/virjson.h b/src/util/virjson.h index e89a776ab5..b76a3c3472 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -149,6 +149,7 @@ virJSONValuePtr virJSONValueObjectStealArray(virJSONValuePtr object, const char *key); const char *virJSONValueObjectGetString(virJSONValuePtr object, const char *key); +const char *virJSONValueObjectGetStringOrNumber(virJSONValuePtr object, const char *key); int virJSONValueObjectGetNumberInt(virJSONValuePtr object, const char *key, int *value); int virJSONValueObjectGetNumberUint(virJSONValuePtr object, const char *key, unsigned int *value); int virJSONValueObjectGetNumberLong(virJSONValuePtr object, const char *key, long long *value); -- 2.15.0

On Wed, Jan 31, 2018 at 12:07:31 +0100, Peter Krempa wrote:
The helper is useful when in cases when JSON we have to parse may return
s/when in/in/
one of the two due to historical reasons and the number value itself would be stored as a string. --- src/util/virjson.c | 27 +++++++++++++++++++++++++++ src/util/virjson.h | 1 + 2 files changed, 28 insertions(+)
ACK Jirka

While the QEMU QAPI schema describes 'lun' as a number, the code dealing with JSON strings does not strictly adhere to this schema and thus formats the number back as a string. Use the new helper to retrieve both possibilities. Note that the formatting code is okay and qemu will accept it as an int. Tweak also one of the test stringst to verify that both formats work with libvirt. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1540290 --- src/util/virstoragefile.c | 13 +++++-------- tests/virstoragetest.c | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 5780180a94..5f661c956c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2976,10 +2976,9 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, const char *transport = virJSONValueObjectGetString(json, "transport"); const char *portal = virJSONValueObjectGetString(json, "portal"); const char *target = virJSONValueObjectGetString(json, "target"); + const char *lun = virJSONValueObjectGetStringOrNumber(json, "lun"); const char *uri; char *port; - unsigned int lun = 0; - char *fulltarget = NULL; int ret = -1; /* legacy URI based syntax passed via 'filename' option */ @@ -2990,6 +2989,9 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, src->type = VIR_STORAGE_TYPE_NETWORK; src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + if (!lun) + lun = "0"; + if (VIR_ALLOC(src->hosts) < 0) goto cleanup; @@ -3026,17 +3028,12 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src, *port = '\0'; } - ignore_value(virJSONValueObjectGetNumberUint(json, "lun", &lun)); - - if (virAsprintf(&fulltarget, "%s/%u", target, lun) < 0) + if (virAsprintf(&src->path, "%s/%s", target, lun) < 0) goto cleanup; - VIR_STEAL_PTR(src->path, fulltarget); - ret = 0; cleanup: - VIR_FREE(fulltarget); return ret; } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 7a0d4a8260..1dc7608cee 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1572,7 +1572,7 @@ mymain(void) "\"transport\":\"tcp\"," "\"portal\":\"test.org:1234\"," "\"target\":\"iqn.2016-12.com.virttest:emulated-iscsi-noauth.target\"," - "\"lun\":6" + "\"lun\":\"6\"" "}" "}", "<source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/6'>\n" -- 2.15.0

On Wed, Jan 31, 2018 at 12:07:32 +0100, Peter Krempa wrote:
While the QEMU QAPI schema describes 'lun' as a number, the code dealing with JSON strings does not strictly adhere to this schema and thus formats the number back as a string. Use the new helper to retrieve both possibilities.
Note that the formatting code is okay and qemu will accept it as an int.
Tweak also one of the test stringst to verify that both formats work
s/stringst/strings/
with libvirt.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1540290 --- src/util/virstoragefile.c | 13 +++++-------- tests/virstoragetest.c | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-)
ACK Jirka
participants (2)
-
Jiri Denemark
-
Peter Krempa