src/node_device/node_device_driver.c | 41 ++++++++++--------- .../mdevctl-list-multiple.json | 4 +- 2 files changed, 23 insertions(+), 22 deletions(-)
Due to a rather unfortunate misunderstanding, we were parsing the list
of defined devices from mdevctl incorrectly. Since my primary
development machine only has a single device capable of mdevs, I
apparently neglected to test multiple parent devices and made some
assumptions based on reading the mdevctl code. These assumptions turned
out to be incorrect, so the parsing failed when devices from more than
one parent device were returned.
The details: mdevctl returns an array of objects representing the
defined devices. But instead of an array of multiple objects (with each
object representing a parent device), the array always contains only a
single object. That object has a separate property for each parent
device.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
src/node_device/node_device_driver.c | 41 ++++++++++---------
.../mdevctl-list-multiple.json | 4 +-
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 8a0a2c3847..cb2c3ceaa4 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1056,6 +1056,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
size_t noutdevs = 0;
size_t i;
size_t j;
+ virJSONValue *obj;
json_devicelist = virJSONValueFromString(jsonstring);
@@ -1065,31 +1066,33 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
goto error;
}
- n = virJSONValueArraySize(json_devicelist);
+ /* 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++) {
- virJSONValue *obj = virJSONValueArrayGet(json_devicelist, i);
const char *parent;
virJSONValue *child_array;
int nchildren;
- if (!virJSONValueIsObject(obj)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Parent device is not an object"));
- goto error;
- }
-
- /* mdevctl returns an array of objects. Each object is a parent device
- * object containing a single key-value pair which maps from the name
- * of the parent device to an array of child devices */
- if (virJSONValueObjectKeysNumber(obj) != 1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unexpected format for parent device object"));
- goto error;
- }
-
- parent = virJSONValueObjectGetKey(obj, 0);
- child_array = virJSONValueObjectGetValue(obj, 0);
+ /* 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 (!virJSONValueIsArray(child_array)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
index eefcd90c62..ca1918d00a 100644
--- a/tests/nodedevmdevctldata/mdevctl-list-multiple.json
+++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
@@ -24,9 +24,7 @@
]
}
}
- ]
- },
- {
+ ],
"matrix": [
{ "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
"mdev_type": "vfio_ap-passthrough",
--
2.31.1
On 6/10/21 8:15 PM, Jonathon Jongsma wrote: > Due to a rather unfortunate misunderstanding, we were parsing the list > of defined devices from mdevctl incorrectly. Since my primary > development machine only has a single device capable of mdevs, I > apparently neglected to test multiple parent devices and made some > assumptions based on reading the mdevctl code. These assumptions turned > out to be incorrect, so the parsing failed when devices from more than > one parent device were returned. > > The details: mdevctl returns an array of objects representing the > defined devices. But instead of an array of multiple objects (with each > object representing a parent device), the array always contains only a > single object. That object has a separate property for each parent > device. > > Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> > --- > src/node_device/node_device_driver.c | 41 ++++++++++--------- > .../mdevctl-list-multiple.json | 4 +- > 2 files changed, 23 insertions(+), 22 deletions(-) Reviewed-by: Michal Privoznik <mprivozn@redhat.com> My apologies for not reviewing earlier. Will push after the release. Michal
On 6/30/21 4:16 PM, Michal Prívozník wrote: > On 6/10/21 8:15 PM, Jonathon Jongsma wrote: >> Due to a rather unfortunate misunderstanding, we were parsing the list >> of defined devices from mdevctl incorrectly. Since my primary >> development machine only has a single device capable of mdevs, I >> apparently neglected to test multiple parent devices and made some >> assumptions based on reading the mdevctl code. These assumptions turned >> out to be incorrect, so the parsing failed when devices from more than >> one parent device were returned. >> >> The details: mdevctl returns an array of objects representing the >> defined devices. But instead of an array of multiple objects (with each >> object representing a parent device), the array always contains only a >> single object. That object has a separate property for each parent >> device. >> >> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> >> --- >> src/node_device/node_device_driver.c | 41 ++++++++++--------- >> .../mdevctl-list-multiple.json | 4 +- >> 2 files changed, 23 insertions(+), 22 deletions(-) > > Reviewed-by: Michal Privoznik <mprivozn@redhat.com> > > My apologies for not reviewing earlier. Will push after the release. Pushed now. Michal
ping
On Thu, Jun 10, 2021 at 1:18 PM Jonathon Jongsma <jjongsma@redhat.com> wrote:
>
> Due to a rather unfortunate misunderstanding, we were parsing the list
> of defined devices from mdevctl incorrectly. Since my primary
> development machine only has a single device capable of mdevs, I
> apparently neglected to test multiple parent devices and made some
> assumptions based on reading the mdevctl code. These assumptions turned
> out to be incorrect, so the parsing failed when devices from more than
> one parent device were returned.
>
> The details: mdevctl returns an array of objects representing the
> defined devices. But instead of an array of multiple objects (with each
> object representing a parent device), the array always contains only a
> single object. That object has a separate property for each parent
> device.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
> src/node_device/node_device_driver.c | 41 ++++++++++---------
> .../mdevctl-list-multiple.json | 4 +-
> 2 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 8a0a2c3847..cb2c3ceaa4 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1056,6 +1056,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
> size_t noutdevs = 0;
> size_t i;
> size_t j;
> + virJSONValue *obj;
>
> json_devicelist = virJSONValueFromString(jsonstring);
>
> @@ -1065,31 +1066,33 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
> goto error;
> }
>
> - n = virJSONValueArraySize(json_devicelist);
> + /* 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++) {
> - virJSONValue *obj = virJSONValueArrayGet(json_devicelist, i);
> const char *parent;
> virJSONValue *child_array;
> int nchildren;
>
> - if (!virJSONValueIsObject(obj)) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Parent device is not an object"));
> - goto error;
> - }
> -
> - /* mdevctl returns an array of objects. Each object is a parent device
> - * object containing a single key-value pair which maps from the name
> - * of the parent device to an array of child devices */
> - if (virJSONValueObjectKeysNumber(obj) != 1) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Unexpected format for parent device object"));
> - goto error;
> - }
> -
> - parent = virJSONValueObjectGetKey(obj, 0);
> - child_array = virJSONValueObjectGetValue(obj, 0);
> + /* 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 (!virJSONValueIsArray(child_array)) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> index eefcd90c62..ca1918d00a 100644
> --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> @@ -24,9 +24,7 @@
> ]
> }
> }
> - ]
> - },
> - {
> + ],
> "matrix": [
> { "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
> "mdev_type": "vfio_ap-passthrough",
> --
> 2.31.1
>
© 2016 - 2024 Red Hat, Inc.