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
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
© 2016 - 2024 Red Hat, Inc.