[PATCH] nodedev: fix error when no defined mdev exist
Commit e9b534905f4 introduced an error when parsing an empty list returned from mdevctl. This occurs e.g. if nodedev-undefine is used to undefine the last defined mdev which cuases the following error messages libvirtd[33143]: internal error: Unexpected format for mdevctl response libvirtd[33143]: internal error: failed to query mdevs from mdevctl: libvirtd[33143]: mdevctl failed to updated mediated devices Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 68 ++++++++++--------- .../mdevctl-list-empty.json | 1 + .../mdevctl-list-empty.out.xml | 0 tests/nodedevmdevctltest.c | 1 + 4 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b4dd57e5f4..31525f2312 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1123,51 +1123,53 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, /* mdevctl list --dumpjson produces an output that is an array that * contains only a single object which contains a property for each parent * device */ - if (virJSONValueArraySize(json_devicelist) != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unexpected format for mdevctl response")); - goto error; - } - - obj = virJSONValueArrayGet(json_devicelist, 0); - - if (!virJSONValueIsObject(obj)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("device list is not an object")); - goto error; - } - - n = virJSONValueObjectKeysNumber(obj); - for (i = 0; i < n; i++) { - const char *parent; - virJSONValue *child_array; - int nchildren; + if (virJSONValueArraySize(json_devicelist) > 0) { + if (virJSONValueArraySize(json_devicelist) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected format for mdevctl response")); + goto error; + } - /* The key of each object property is the name of a parent device - * which maps to an array of child devices */ - parent = virJSONValueObjectGetKey(obj, i); - child_array = virJSONValueObjectGetValue(obj, i); + obj = virJSONValueArrayGet(json_devicelist, 0); - if (!virJSONValueIsArray(child_array)) { + if (!virJSONValueIsObject(obj)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Parent device's JSON object data is not an array")); + _("device list is not an object")); goto error; } - nchildren = virJSONValueArraySize(child_array); + n = virJSONValueObjectKeysNumber(obj); + for (i = 0; i < n; i++) { + const char *parent; + virJSONValue *child_array; + int nchildren; - for (j = 0; j < nchildren; j++) { - g_autoptr(virNodeDeviceDef) child = NULL; - virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + /* The key of each object property is the name of a parent device + * which maps to an array of child devices */ + parent = virJSONValueObjectGetKey(obj, i); + child_array = virJSONValueObjectGetValue(obj, i); - if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + if (!virJSONValueIsArray(child_array)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to parse child device")); + _("Parent device's JSON object data is not an array")); goto error; } - if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) - goto error; + nchildren = virJSONValueArraySize(child_array); + + for (j = 0; j < nchildren; j++) { + g_autoptr(virNodeDeviceDef) child = NULL; + virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + + if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to parse child device")); + goto error; + } + + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) + goto error; + } } } diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 8ba1d2da70..e246de4d87 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -360,6 +360,7 @@ mymain(void) DO_TEST_LIST_DEFINED(); + DO_TEST_PARSE_JSON("mdevctl-list-empty"); DO_TEST_PARSE_JSON("mdevctl-list-multiple"); DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); -- 2.31.1
On 7/9/21 3:39 PM, Boris Fiuczynski wrote:
Commit e9b534905f4 introduced an error when parsing an empty list returned from mdevctl.
This occurs e.g. if nodedev-undefine is used to undefine the last defined mdev which cuases the following error messages
libvirtd[33143]: internal error: Unexpected format for mdevctl response libvirtd[33143]: internal error: failed to query mdevs from mdevctl: libvirtd[33143]: mdevctl failed to updated mediated devices
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 68 ++++++++++--------- .../mdevctl-list-empty.json | 1 + .../mdevctl-list-empty.out.xml | 0 tests/nodedevmdevctltest.c | 1 + 4 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b4dd57e5f4..31525f2312 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1123,51 +1123,53 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, /* mdevctl list --dumpjson produces an output that is an array that * contains only a single object which contains a property for each parent * device */ - if (virJSONValueArraySize(json_devicelist) != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unexpected format for mdevctl response")); - goto error; - } - - obj = virJSONValueArrayGet(json_devicelist, 0); - - if (!virJSONValueIsObject(obj)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("device list is not an object")); - goto error; - } - - n = virJSONValueObjectKeysNumber(obj); - for (i = 0; i < n; i++) { - const char *parent; - virJSONValue *child_array; - int nchildren; + if (virJSONValueArraySize(json_devicelist) > 0) {
This seems now to break nodedev-create which created transient devices since the related objects now also vanish.
+ if (virJSONValueArraySize(json_devicelist) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected format for mdevctl response")); + goto error; + }
- /* The key of each object property is the name of a parent device - * which maps to an array of child devices */ - parent = virJSONValueObjectGetKey(obj, i); - child_array = virJSONValueObjectGetValue(obj, i); + obj = virJSONValueArrayGet(json_devicelist, 0);
- if (!virJSONValueIsArray(child_array)) { + if (!virJSONValueIsObject(obj)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Parent device's JSON object data is not an array")); + _("device list is not an object")); goto error; }
- nchildren = virJSONValueArraySize(child_array); + n = virJSONValueObjectKeysNumber(obj); + for (i = 0; i < n; i++) { + const char *parent; + virJSONValue *child_array; + int nchildren;
- for (j = 0; j < nchildren; j++) { - g_autoptr(virNodeDeviceDef) child = NULL; - virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + /* The key of each object property is the name of a parent device + * which maps to an array of child devices */ + parent = virJSONValueObjectGetKey(obj, i); + child_array = virJSONValueObjectGetValue(obj, i);
- if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + if (!virJSONValueIsArray(child_array)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to parse child device")); + _("Parent device's JSON object data is not an array")); goto error; }
- if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) - goto error; + nchildren = virJSONValueArraySize(child_array); + + for (j = 0; j < nchildren; j++) { + g_autoptr(virNodeDeviceDef) child = NULL; + virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + + if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to parse child device")); + goto error; + } + + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) + goto error; + } } }
diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 8ba1d2da70..e246de4d87 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -360,6 +360,7 @@ mymain(void)
DO_TEST_LIST_DEFINED();
+ DO_TEST_PARSE_JSON("mdevctl-list-empty"); DO_TEST_PARSE_JSON("mdevctl-list-multiple");
DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 7/9/21 5:27 PM, Boris Fiuczynski wrote:
- virJSONValue *child_array; - int nchildren; + if (virJSONValueArraySize(json_devicelist) > 0) {
This seems now to break nodedev-create which created transient devices since the related objects now also vanish.
I guess that must have been a late Friday outage... My testing was erroneous and I did not use the newly created uuid thinking it was statically defined in the used XML to create the mdev which it wasn't. :( Sorry about the confusion.
+ if (virJSONValueArraySize(json_devicelist) != 1) {
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Fri, 9 Jul 2021 15:39:47 +0200 Boris Fiuczynski <fiuczy@linux.ibm.com> wrote:
Commit e9b534905f4 introduced an error when parsing an empty list returned from mdevctl.
This occurs e.g. if nodedev-undefine is used to undefine the last defined mdev which cuases the following error messages
libvirtd[33143]: internal error: Unexpected format for mdevctl response libvirtd[33143]: internal error: failed to query mdevs from mdevctl: libvirtd[33143]: mdevctl failed to updated mediated devices
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 68 ++++++++++--------- .../mdevctl-list-empty.json | 1 + .../mdevctl-list-empty.out.xml | 0 tests/nodedevmdevctltest.c | 1 + 4 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b4dd57e5f4..31525f2312 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1123,51 +1123,53 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, /* mdevctl list --dumpjson produces an output that is an array that * contains only a single object which contains a property for each parent * device */ - if (virJSONValueArraySize(json_devicelist) != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unexpected format for mdevctl response")); - goto error; - } - - obj = virJSONValueArrayGet(json_devicelist, 0); - - if (!virJSONValueIsObject(obj)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("device list is not an object")); - goto error; - } - - n = virJSONValueObjectKeysNumber(obj); - for (i = 0; i < n; i++) { - const char *parent; - virJSONValue *child_array; - int nchildren; + if (virJSONValueArraySize(json_devicelist) > 0) { + if (virJSONValueArraySize(json_devicelist) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected format for mdevctl response")); + goto error; + }
- /* The key of each object property is the name of a parent device - * which maps to an array of child devices */ - parent = virJSONValueObjectGetKey(obj, i); - child_array = virJSONValueObjectGetValue(obj, i); + obj = virJSONValueArrayGet(json_devicelist, 0);
- if (!virJSONValueIsArray(child_array)) { + if (!virJSONValueIsObject(obj)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Parent device's JSON object data is not an array")); + _("device list is not an object")); goto error; }
- nchildren = virJSONValueArraySize(child_array); + n = virJSONValueObjectKeysNumber(obj); + for (i = 0; i < n; i++) { + const char *parent; + virJSONValue *child_array; + int nchildren;
- for (j = 0; j < nchildren; j++) { - g_autoptr(virNodeDeviceDef) child = NULL; - virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + /* The key of each object property is the name of a parent device + * which maps to an array of child devices */ + parent = virJSONValueObjectGetKey(obj, i); + child_array = virJSONValueObjectGetValue(obj, i);
- if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + if (!virJSONValueIsArray(child_array)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to parse child device")); + _("Parent device's JSON object data is not an array")); goto error; }
- if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) - goto error; + nchildren = virJSONValueArraySize(child_array); + + for (j = 0; j < nchildren; j++) { + g_autoptr(virNodeDeviceDef) child = NULL; + virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + + if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to parse child device")); + goto error; + } + + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) + goto error; + } } }
diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 8ba1d2da70..e246de4d87 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -360,6 +360,7 @@ mymain(void)
DO_TEST_LIST_DEFINED();
+ DO_TEST_PARSE_JSON("mdevctl-list-empty"); DO_TEST_PARSE_JSON("mdevctl-list-multiple");
DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
Thanks for catching this. I'd prefer to simplify this patch by returning early if there are no elements in the array. That reduces the diff to about 5 total lines changed, including the new test: diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b4dd57e5f4..f63a65e26a 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1120,6 +1120,9 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, goto error; } + if (virJSONValueArraySize(json_devicelist) == 0) + return 0; + /* mdevctl list --dumpjson produces an output that is an array that * contains only a single object which contains a property for each parent * device */ diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 8ba1d2da70..e246de4d87 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -360,6 +360,7 @@ mymain(void) DO_TEST_LIST_DEFINED(); + DO_TEST_PARSE_JSON("mdevctl-list-empty"); DO_TEST_PARSE_JSON("mdevctl-list-multiple"); DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
On 7/9/21 6:25 PM, Jonathon Jongsma wrote:
On Fri, 9 Jul 2021 15:39:47 +0200 Boris Fiuczynski <fiuczy@linux.ibm.com> wrote:
Commit e9b534905f4 introduced an error when parsing an empty list returned from mdevctl.
This occurs e.g. if nodedev-undefine is used to undefine the last defined mdev which cuases the following error messages
libvirtd[33143]: internal error: Unexpected format for mdevctl response libvirtd[33143]: internal error: failed to query mdevs from mdevctl: libvirtd[33143]: mdevctl failed to updated mediated devices
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/node_device/node_device_driver.c | 68 ++++++++++--------- .../mdevctl-list-empty.json | 1 + .../mdevctl-list-empty.out.xml | 0 tests/nodedevmdevctltest.c | 1 + 4 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b4dd57e5f4..31525f2312 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1123,51 +1123,53 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, /* mdevctl list --dumpjson produces an output that is an array that * contains only a single object which contains a property for each parent * device */ - if (virJSONValueArraySize(json_devicelist) != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unexpected format for mdevctl response")); - goto error; - } - - obj = virJSONValueArrayGet(json_devicelist, 0); - - if (!virJSONValueIsObject(obj)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("device list is not an object")); - goto error; - } - - n = virJSONValueObjectKeysNumber(obj); - for (i = 0; i < n; i++) { - const char *parent; - virJSONValue *child_array; - int nchildren; + if (virJSONValueArraySize(json_devicelist) > 0) { + if (virJSONValueArraySize(json_devicelist) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected format for mdevctl response")); + goto error; + }
- /* The key of each object property is the name of a parent device - * which maps to an array of child devices */ - parent = virJSONValueObjectGetKey(obj, i); - child_array = virJSONValueObjectGetValue(obj, i); + obj = virJSONValueArrayGet(json_devicelist, 0);
- if (!virJSONValueIsArray(child_array)) { + if (!virJSONValueIsObject(obj)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Parent device's JSON object data is not an array")); + _("device list is not an object")); goto error; }
- nchildren = virJSONValueArraySize(child_array); + n = virJSONValueObjectKeysNumber(obj); + for (i = 0; i < n; i++) { + const char *parent; + virJSONValue *child_array; + int nchildren;
- for (j = 0; j < nchildren; j++) { - g_autoptr(virNodeDeviceDef) child = NULL; - virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + /* The key of each object property is the name of a parent device + * which maps to an array of child devices */ + parent = virJSONValueObjectGetKey(obj, i); + child_array = virJSONValueObjectGetValue(obj, i);
- if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + if (!virJSONValueIsArray(child_array)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to parse child device")); + _("Parent device's JSON object data is not an array")); goto error; }
- if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) - goto error; + nchildren = virJSONValueArraySize(child_array); + + for (j = 0; j < nchildren; j++) { + g_autoptr(virNodeDeviceDef) child = NULL; + virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + + if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to parse child device")); + goto error; + } + + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) + goto error; + } } }
diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 8ba1d2da70..e246de4d87 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -360,6 +360,7 @@ mymain(void)
DO_TEST_LIST_DEFINED();
+ DO_TEST_PARSE_JSON("mdevctl-list-empty"); DO_TEST_PARSE_JSON("mdevctl-list-multiple");
DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
Thanks for catching this. I'd prefer to simplify this patch by returning early if there are no elements in the array. That reduces the diff to about 5 total lines changed, including the new test:
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b4dd57e5f4..f63a65e26a 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1120,6 +1120,9 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, goto error; }
+ if (virJSONValueArraySize(json_devicelist) == 0) + return 0;
This was kind of what I did first, too, but I thought I would require to also set not only the length of the array but also the array itself. That's why I ended up with this. if (virJSONValueArraySize(json_devicelist) == 0) { *devs = outdevs; return 0; } and since that matches exactly the already existing return I wrapped the if-statement around the existing code block... causing a horribly looking diff. But actually there are just two added lines: if (virJSONValueArraySize(json_devicelist) > 0) { {old code with 4 additional leading blanks for the indentation} }
+ /* mdevctl list --dumpjson produces an output that is an array that * contains only a single object which contains a property for each parent * device */ diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 8ba1d2da70..e246de4d87 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -360,6 +360,7 @@ mymain(void)
DO_TEST_LIST_DEFINED();
+ DO_TEST_PARSE_JSON("mdevctl-list-empty"); DO_TEST_PARSE_JSON("mdevctl-list-multiple");
DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
Boris Fiuczynski -
Jonathon Jongsma