[PATCH v3] nodedev: fix internal error when no defined mdevs exist

Boris Fiuczynski posted 1 patch 2 years, 9 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210721113000.12799-1-fiuczy@linux.ibm.com
src/node_device/node_device_driver.c                | 6 ++++++
tests/nodedevmdevctldata/mdevctl-list-empty.json    | 1 +
tests/nodedevmdevctldata/mdevctl-list-empty.out.xml | 0
tests/nodedevmdevctltest.c                          | 1 +
4 files changed, 8 insertions(+)
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
[PATCH v3] nodedev: fix internal error when no defined mdevs exist
Posted by Boris Fiuczynski 2 years, 9 months ago
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 causes 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>
Reviewed-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/node_device/node_device_driver.c                | 6 ++++++
 tests/nodedevmdevctldata/mdevctl-list-empty.json    | 1 +
 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml | 0
 tests/nodedevmdevctltest.c                          | 1 +
 4 files changed, 8 insertions(+)
 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 17a7aea2bb..6f8968f1f0 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1139,6 +1139,12 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
         goto error;
     }
 
+    if (virJSONValueArraySize(json_devicelist) == 0) {
+        VIR_DEBUG("mdevctl has no defined mediated devices");
+        *devs = NULL;
+        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");
-- 
2.31.1

Re: [PATCH v3] nodedev: fix internal error when no defined mdevs exist
Posted by Jonathon Jongsma 2 years, 9 months ago
On Wed, 21 Jul 2021 13:30:00 +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 causes 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>
> Reviewed-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/node_device/node_device_driver.c                | 6 ++++++
>  tests/nodedevmdevctldata/mdevctl-list-empty.json    | 1 +
>  tests/nodedevmdevctldata/mdevctl-list-empty.out.xml | 0
>  tests/nodedevmdevctltest.c                          | 1 +
>  4 files changed, 8 insertions(+)
>  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 17a7aea2bb..6f8968f1f0
> 100644 --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1139,6 +1139,12 @@ nodeDeviceParseMdevctlJSON(const char
> *jsonstring, goto error;
>      }
>  
> +    if (virJSONValueArraySize(json_devicelist) == 0) {
> +        VIR_DEBUG("mdevctl has no defined mediated devices");
> +        *devs = NULL;
> +        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");



Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>

I've pushed it upstream.

Thanks,
Jonathon