This adds some internal API to query for persistent mediated devices
that are defined by mdevctl. Following commits will make use of this
information. This just provides the infrastructure and tests for this
feature. One test verifies that we are executing mdevctl with the proper
arguments, and the other test verifies that we can parse the returned
JSON and convert it to our own internal node device representations.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
src/node_device/node_device_driver.c | 150 ++++++++++++++++++
src/node_device/node_device_driver.h | 7 +
.../mdevctl-list-defined.argv | 1 +
.../mdevctl-list-multiple.json | 59 +++++++
.../mdevctl-list-multiple.out.xml | 39 +++++
tests/nodedevmdevctltest.c | 95 ++++++++++-
6 files changed, 349 insertions(+), 2 deletions(-)
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 6143459618..bbd373e32e 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -853,6 +853,156 @@ virMdevctlStop(virNodeDeviceDefPtr def)
}
+virCommandPtr
+nodeDeviceGetMdevctlListCommand(bool defined,
+ char **output)
+{
+ virCommandPtr cmd = virCommandNewArgList(MDEVCTL,
+ "list",
+ "--dumpjson",
+ NULL);
+
+ if (defined)
+ virCommandAddArg(cmd, "--defined");
+
+ virCommandSetOutputBuffer(cmd, output);
+
+ return cmd;
+}
+
+
+static void mdevGenerateDeviceName(virNodeDeviceDefPtr dev)
+{
+ nodeDeviceGenerateName(dev, "mdev", dev->caps->data.mdev.uuid, NULL);
+}
+
+
+static virNodeDeviceDefPtr
+nodeDeviceParseMdevctlChildDevice(const char *parent,
+ virJSONValuePtr json)
+{
+ virNodeDevCapMdevPtr mdev;
+ const char *uuid;
+ virJSONValuePtr props, attrs;
+ g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1);
+
+ /* the child object should have a single key equal to its uuid.
+ * The value is an object describing the properties of the mdev */
+ if (virJSONValueObjectKeysNumber(json) != 1)
+ return NULL;
+
+ uuid = virJSONValueObjectGetKey(json, 0);
+ props = virJSONValueObjectGetValue(json, 0);
+
+ child->parent = g_strdup(parent);
+ child->caps = g_new0(virNodeDevCapsDef, 1);
+ child->caps->data.type = VIR_NODE_DEV_CAP_MDEV;
+
+ mdev = &child->caps->data.mdev;
+ mdev->uuid = g_strdup(uuid);
+ mdev->type =
+ g_strdup(virJSONValueObjectGetString(props, "mdev_type"));
+
+ attrs = virJSONValueObjectGet(props, "attrs");
+
+ if (attrs && virJSONValueIsArray(attrs)) {
+ size_t i;
+ int nattrs = virJSONValueArraySize(attrs);
+
+ mdev->attributes = g_new0(virMediatedDeviceAttrPtr, nattrs);
+ mdev->nattributes = nattrs;
+
+ for (i = 0; i < nattrs; i++) {
+ virJSONValuePtr attr = virJSONValueArrayGet(attrs, i);
+ virMediatedDeviceAttrPtr attribute;
+ virJSONValuePtr value;
+
+ if (!virJSONValueIsObject(attr) ||
+ virJSONValueObjectKeysNumber(attr) != 1)
+ return NULL;
+
+ attribute = g_new0(virMediatedDeviceAttr, 1);
+ attribute->name = g_strdup(virJSONValueObjectGetKey(attr, 0));
+ value = virJSONValueObjectGetValue(attr, 0);
+ attribute->value = g_strdup(virJSONValueGetString(value));
+ mdev->attributes[i] = attribute;
+ }
+ }
+ mdevGenerateDeviceName(child);
+
+ return g_steal_pointer(&child);
+}
+
+
+int
+nodeDeviceParseMdevctlJSON(const char *jsonstring,
+ virNodeDeviceDefPtr **devs)
+{
+ int n;
+ g_autoptr(virJSONValue) json_devicelist = NULL;
+ virNodeDeviceDefPtr *outdevs = NULL;
+ size_t noutdevs = 0;
+ size_t i, j;
+
+ json_devicelist = virJSONValueFromString(jsonstring);
+
+ if (!json_devicelist)
+ goto parsefailure;
+
+ if (!virJSONValueIsArray(json_devicelist))
+ goto parsefailure;
+
+ n = virJSONValueArraySize(json_devicelist);
+
+ for (i = 0; i < n; i++) {
+ virJSONValuePtr obj = virJSONValueArrayGet(json_devicelist, i);
+ const char *parent;
+ virJSONValuePtr child_array;
+ int nchildren;
+
+ if (!virJSONValueIsObject(obj))
+ goto parsefailure;
+
+ /* 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)
+ goto parsefailure;
+
+ parent = virJSONValueObjectGetKey(obj, 0);
+ child_array = virJSONValueObjectGetValue(obj, 0);
+
+ if (!virJSONValueIsArray(child_array))
+ goto parsefailure;
+
+ nchildren = virJSONValueArraySize(child_array);
+
+ for (j = 0; j < nchildren; j++) {
+ g_autoptr(virNodeDeviceDef) child = NULL;
+ virJSONValuePtr child_obj = virJSONValueArrayGet(child_array, j);
+
+ if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj)))
+ goto parsefailure;
+
+ if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0)
+ goto parsefailure;
+ }
+ }
+
+ *devs = outdevs;
+ return noutdevs;
+
+ parsefailure:
+ for (i = 0; i < noutdevs; i++)
+ virNodeDeviceDefFree(outdevs[i]);
+ VIR_FREE(outdevs);
+
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unable to parse JSON response"));
+ return -1;
+}
+
+
int
nodeDeviceDestroy(virNodeDevicePtr device)
{
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index 02baf56dab..80ac7c5320 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -119,6 +119,13 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
virCommandPtr
nodeDeviceGetMdevctlStopCommand(const char *uuid);
+virCommandPtr
+nodeDeviceGetMdevctlListCommand(bool defined, char **output);
+
+int
+nodeDeviceParseMdevctlJSON(const char *jsonstring,
+ virNodeDeviceDefPtr **devs);
+
void
nodeDeviceGenerateName(virNodeDeviceDefPtr def,
const char *subsystem,
diff --git a/tests/nodedevmdevctldata/mdevctl-list-defined.argv b/tests/nodedevmdevctldata/mdevctl-list-defined.argv
new file mode 100644
index 0000000000..72b5906e9e
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdevctl-list-defined.argv
@@ -0,0 +1 @@
+$MDEVCTL_BINARY$ list --dumpjson --defined
diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
new file mode 100644
index 0000000000..eefcd90c62
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
@@ -0,0 +1,59 @@
+[
+ {
+ "0000:00:02.0": [
+ {
+ "200f228a-c80a-4d50-bfb7-f5a0e4e34045": {
+ "mdev_type": "i915-GVTg_V5_4",
+ "start": "manual"
+ }
+ },
+ {
+ "de807ffc-1923-4d5f-b6c9-b20ecebc6d4b": {
+ "mdev_type": "i915-GVTg_V5_4",
+ "start": "auto"
+ }
+ },
+ {
+ "435722ea-5f43-468a-874f-da34f1217f13": {
+ "mdev_type": "i915-GVTg_V5_8",
+ "start": "manual",
+ "attrs": [
+ {
+ "testattr": "42"
+ }
+ ]
+ }
+ }
+ ]
+ },
+ {
+ "matrix": [
+ { "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
+ "mdev_type": "vfio_ap-passthrough",
+ "start": "manual",
+ "attrs": [
+ {
+ "assign_adapter": "5"
+ },
+ {
+ "assign_adapter": "6"
+ },
+ {
+ "assign_domain": "0xab"
+ },
+ {
+ "assign_control_domain": "0xab"
+ },
+ {
+ "assign_domain": "4"
+ },
+ {
+ "assign_control_domain": "4"
+ }
+ ]
+ }
+ }
+ ]
+ }
+]
+
diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
new file mode 100644
index 0000000000..543ad916b7
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
@@ -0,0 +1,39 @@
+<device>
+ <name>mdev_200f228a_c80a_4d50_bfb7_f5a0e4e34045</name>
+ <parent>0000:00:02.0</parent>
+ <capability type='mdev'>
+ <type id='i915-GVTg_V5_4'/>
+ <iommuGroup number='0'/>
+ </capability>
+</device>
+<device>
+ <name>mdev_de807ffc_1923_4d5f_b6c9_b20ecebc6d4b</name>
+ <parent>0000:00:02.0</parent>
+ <capability type='mdev'>
+ <type id='i915-GVTg_V5_4'/>
+ <iommuGroup number='0'/>
+ </capability>
+</device>
+<device>
+ <name>mdev_435722ea_5f43_468a_874f_da34f1217f13</name>
+ <parent>0000:00:02.0</parent>
+ <capability type='mdev'>
+ <type id='i915-GVTg_V5_8'/>
+ <iommuGroup number='0'/>
+ <attr name='testattr' value='42'/>
+ </capability>
+</device>
+<device>
+ <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name>
+ <parent>matrix</parent>
+ <capability type='mdev'>
+ <type id='vfio_ap-passthrough'/>
+ <iommuGroup number='0'/>
+ <attr name='assign_adapter' value='5'/>
+ <attr name='assign_adapter' value='6'/>
+ <attr name='assign_domain' value='0xab'/>
+ <attr name='assign_control_domain' value='0xab'/>
+ <attr name='assign_domain' value='4'/>
+ <attr name='assign_control_domain' value='4'/>
+ </capability>
+</device>
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 1d3cb00400..8bc464d59f 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -143,6 +143,87 @@ testMdevctlStop(const void *data)
return ret;
}
+static int
+testMdevctlListDefined(const void *data G_GNUC_UNUSED)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ const char *actualCmdline = NULL;
+ int ret = -1;
+ g_autoptr(virCommand) cmd = NULL;
+ g_autofree char *output = NULL;
+ g_autofree char *cmdlinefile =
+ g_strdup_printf("%s/nodedevmdevctldata/mdevctl-list-defined.argv",
+ abs_srcdir);
+
+ cmd = nodeDeviceGetMdevctlListCommand(true, &output);
+
+ if (!cmd)
+ goto cleanup;
+
+ virCommandSetDryRun(&buf, NULL, NULL);
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ if (!(actualCmdline = virBufferCurrentContent(&buf)))
+ goto cleanup;
+
+ if (nodedevCompareToFile(actualCmdline, cmdlinefile) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&buf);
+ virCommandSetDryRun(NULL, NULL, NULL);
+ return ret;
+}
+
+static int
+testMdevctlParse(const void *data)
+{
+ g_autofree char *buf = NULL;
+ const char *filename = data;
+ g_autofree char *jsonfile = g_strdup_printf("%s/nodedevmdevctldata/%s.json",
+ abs_srcdir, filename);
+ g_autofree char *xmloutfile = g_strdup_printf("%s/nodedevmdevctldata/%s.out.xml",
+ abs_srcdir, filename);
+ int ret = -1;
+ int nmdevs = 0;
+ virNodeDeviceDefPtr *mdevs = NULL;
+ virBuffer xmloutbuf = VIR_BUFFER_INITIALIZER;
+ size_t i;
+
+ if (virFileReadAll(jsonfile, 1024*1024, &buf) < 0) {
+ VIR_TEST_DEBUG("Unable to read file %s", jsonfile);
+ return -1;
+ }
+
+ if ((nmdevs = nodeDeviceParseMdevctlJSON(buf, &mdevs)) < 0) {
+ VIR_TEST_DEBUG("Unable to parse json for %s", filename);
+ return -1;
+ }
+
+ for (i = 0; i < nmdevs; i++) {
+ g_autofree char *devxml = virNodeDeviceDefFormat(mdevs[i]);
+ if (!devxml)
+ goto cleanup;
+ virBufferAddStr(&xmloutbuf, devxml);
+ }
+
+ if (nodedevCompareToFile(virBufferCurrentContent(&xmloutbuf), xmloutfile) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&xmloutbuf);
+ for (i = 0; i < nmdevs; i++)
+ virNodeDeviceDefFree(mdevs[i]);
+ g_free(mdevs);
+
+ return ret;
+}
+
static void
nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv)
{
@@ -263,13 +344,13 @@ mymain(void)
}
#define DO_TEST_FULL(desc, func, info) \
- if (virTestRun(desc, func, &info) < 0) \
+ if (virTestRun(desc, func, info) < 0) \
ret = -1;
#define DO_TEST_START_FULL(virt_type, create, filename) \
do { \
struct startTestInfo info = { virt_type, create, filename }; \
- DO_TEST_FULL("mdevctl start " filename, testMdevctlStartHelper, info); \
+ DO_TEST_FULL("mdevctl start " filename, testMdevctlStartHelper, &info); \
} \
while (0)
@@ -279,6 +360,12 @@ mymain(void)
#define DO_TEST_STOP(uuid) \
DO_TEST_FULL("mdevctl stop " uuid, testMdevctlStop, uuid)
+#define DO_TEST_LIST_DEFINED() \
+ DO_TEST_FULL("mdevctl list --defined", testMdevctlListDefined, NULL)
+
+#define DO_TEST_PARSE_JSON(filename) \
+ DO_TEST_FULL("parse mdevctl json " filename, testMdevctlParse, filename)
+
/* Test mdevctl start commands */
DO_TEST_START("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");
DO_TEST_START("mdev_fedc4916_1ca8_49ac_b176_871d16c13076");
@@ -287,6 +374,10 @@ mymain(void)
/* Test mdevctl stop command, pass an arbitrary uuid */
DO_TEST_STOP("e2451f73-c95b-4124-b900-e008af37c576");
+ DO_TEST_LIST_DEFINED();
+
+ DO_TEST_PARSE_JSON("mdevctl-list-multiple");
+
done:
nodedevTestDriverFree(driver);
--
2.26.2
On Thu, Dec 24, 2020 at 08:14:29AM -0600, Jonathon Jongsma wrote:
> This adds some internal API to query for persistent mediated devices
> that are defined by mdevctl. Following commits will make use of this
> information. This just provides the infrastructure and tests for this
> feature. One test verifies that we are executing mdevctl with the proper
> arguments, and the other test verifies that we can parse the returned
> JSON and convert it to our own internal node device representations.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
> src/node_device/node_device_driver.c | 150 ++++++++++++++++++
> src/node_device/node_device_driver.h | 7 +
> .../mdevctl-list-defined.argv | 1 +
> .../mdevctl-list-multiple.json | 59 +++++++
> .../mdevctl-list-multiple.out.xml | 39 +++++
> tests/nodedevmdevctltest.c | 95 ++++++++++-
> 6 files changed, 349 insertions(+), 2 deletions(-)
> create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv
> create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json
> create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
>
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 6143459618..bbd373e32e 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -853,6 +853,156 @@ virMdevctlStop(virNodeDeviceDefPtr def)
> }
>
>
> +virCommandPtr
> +nodeDeviceGetMdevctlListCommand(bool defined,
> + char **output)
> +{
> + virCommandPtr cmd = virCommandNewArgList(MDEVCTL,
> + "list",
> + "--dumpjson",
> + NULL);
> +
> + if (defined)
> + virCommandAddArg(cmd, "--defined");
> +
> + virCommandSetOutputBuffer(cmd, output);
> +
> + return cmd;
> +}
> +
At this point I think we could split this even further and add the parser code
first and then introduce listing support.
...
> +int
> +nodeDeviceParseMdevctlJSON(const char *jsonstring,
> + virNodeDeviceDefPtr **devs)
> +{
> + int n;
> + g_autoptr(virJSONValue) json_devicelist = NULL;
> + virNodeDeviceDefPtr *outdevs = NULL;
> + size_t noutdevs = 0;
> + size_t i, j;
One declaration per line please (I've seen another one, but not sure if it
was in this patch or some later one).
> +
> + json_devicelist = virJSONValueFromString(jsonstring);
> +
> + if (!json_devicelist)
> + goto parsefailure;
> +
> + if (!virJSONValueIsArray(json_devicelist))
> + goto parsefailure;
> +
> + n = virJSONValueArraySize(json_devicelist);
> +
> + for (i = 0; i < n; i++) {
> + virJSONValuePtr obj = virJSONValueArrayGet(json_devicelist, i);
> + const char *parent;
> + virJSONValuePtr child_array;
> + int nchildren;
> +
> + if (!virJSONValueIsObject(obj))
> + goto parsefailure;
> +
> + /* 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)
> + goto parsefailure;
> +
> + parent = virJSONValueObjectGetKey(obj, 0);
> + child_array = virJSONValueObjectGetValue(obj, 0);
> +
> + if (!virJSONValueIsArray(child_array))
> + goto parsefailure;
> +
> + nchildren = virJSONValueArraySize(child_array);
> +
> + for (j = 0; j < nchildren; j++) {
> + g_autoptr(virNodeDeviceDef) child = NULL;
> + virJSONValuePtr child_obj = virJSONValueArrayGet(child_array, j);
> +
> + if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj)))
> + goto parsefailure;
> +
> + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0)
> + goto parsefailure;
> + }
> + }
> +
> + *devs = outdevs;
> + return noutdevs;
> +
> + parsefailure:
As pointed out in v2:
s/parsefailure/error
> + for (i = 0; i < noutdevs; i++)
> + virNodeDeviceDefFree(outdevs[i]);
> + VIR_FREE(outdevs);
> +
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("unable to parse JSON response"));
> + return -1;
> +}
> +
> +
...
> --- /dev/null
> +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> @@ -0,0 +1,59 @@
> +[
> + {
> + "0000:00:02.0": [
> + {
> + "200f228a-c80a-4d50-bfb7-f5a0e4e34045": {
> + "mdev_type": "i915-GVTg_V5_4",
> + "start": "manual"
> + }
> + },
> + {
> + "de807ffc-1923-4d5f-b6c9-b20ecebc6d4b": {
> + "mdev_type": "i915-GVTg_V5_4",
> + "start": "auto"
> + }
> + },
> + {
> + "435722ea-5f43-468a-874f-da34f1217f13": {
> + "mdev_type": "i915-GVTg_V5_8",
> + "start": "manual",
> + "attrs": [
> + {
> + "testattr": "42"
> + }
> + ]
> + }
> + }
> + ]
> + },
> + {
> + "matrix": [
> + { "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
> + "mdev_type": "vfio_ap-passthrough",
> + "start": "manual",
> + "attrs": [
> + {
> + "assign_adapter": "5"
> + },
> + {
> + "assign_adapter": "6"
> + },
Why are there 2 of each? Does that indicate to mdevctl that when creating an
mdev on the matrix device it can pick any of the adapters for the mdev?
> + {
> + "assign_domain": "0xab"
> + },
> + {
> + "assign_control_domain": "0xab"
> + },
> + {
> + "assign_domain": "4"
> + },
> + {
> + "assign_control_domain": "4"
Are ^these the real life attribute names mdevctl uses for vfio_ap-passthrough?
...
> +<device>
> + <name>mdev_435722ea_5f43_468a_874f_da34f1217f13</name>
> + <parent>0000:00:02.0</parent>
> + <capability type='mdev'>
> + <type id='i915-GVTg_V5_8'/>
> + <iommuGroup number='0'/>
> + <attr name='testattr' value='42'/>
> + </capability>
> +</device>
> +<device>
> + <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name>
> + <parent>matrix</parent>
> + <capability type='mdev'>
> + <type id='vfio_ap-passthrough'/>
> + <iommuGroup number='0'/>
> + <attr name='assign_adapter' value='5'/>
> + <attr name='assign_adapter' value='6'/>
> + <attr name='assign_domain' value='0xab'/>
> + <attr name='assign_control_domain' value='0xab'/>
> + <attr name='assign_domain' value='4'/>
> + <attr name='assign_control_domain' value='4'/>
Bjoern was right in his response to v2 when he said this adds quite some noise
for the end user and is not consistent at all with how we represent the AP
scheme. On the other hand, at the time of posting the v2 we already
had introduced and documented the <attr> element, so I still think what you
have here is the right approach because should we have gone with vendor
specific elements the end users might get the impression that libvirt owns the
attributes or IOW is somehow responsible for whether the mdev is going to be
created successfully with these attributes or not - whilst we're just the
middle man and we're merely passing these attributes to mdevctl and we also
document that it's a generic mechanism to allow vendor-specific attributes
to be specified along with the mdev.
However, the end users know nothing about what the names of the attributes are
and what mdevctl accepts, so maybe we could, for XML purposes, name the
attributes the same way as we name the AP elements, i.e. ap-adapter, ap-control
(yeah, it should have been "ap_xyz" I overlooked this during review, so that
is on me). The fact we're using mdevctl underneath is completely transparent
to the end users, so we should strive for at least some kind of consistency
so anyone can tell how the individual node devices relate to each other.
Code-wise I don't see any outstanding problems anymore, so once we come to an
agreement on what the XML needs to look like to make most (ideally all) parties
happy, you have my:
Reviewed-by: Erik Skultety <eskultet@redhat.com>
On Thu, Dec 24, 2020 at 08:14:29AM -0600, Jonathon Jongsma wrote:
> This adds some internal API to query for persistent mediated devices
> that are defined by mdevctl. Following commits will make use of this
> information. This just provides the infrastructure and tests for this
> feature. One test verifies that we are executing mdevctl with the proper
> arguments, and the other test verifies that we can parse the returned
> JSON and convert it to our own internal node device representations.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
> src/node_device/node_device_driver.c | 150 ++++++++++++++++++
> src/node_device/node_device_driver.h | 7 +
> .../mdevctl-list-defined.argv | 1 +
> .../mdevctl-list-multiple.json | 59 +++++++
> .../mdevctl-list-multiple.out.xml | 39 +++++
> tests/nodedevmdevctltest.c | 95 ++++++++++-
> 6 files changed, 349 insertions(+), 2 deletions(-)
> create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv
> create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json
> create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
>
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 6143459618..bbd373e32e 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -853,6 +853,156 @@ virMdevctlStop(virNodeDeviceDefPtr def)
> }
> +int
> +nodeDeviceParseMdevctlJSON(const char *jsonstring,
> + virNodeDeviceDefPtr **devs)
> +{
> + int n;
> + g_autoptr(virJSONValue) json_devicelist = NULL;
> + virNodeDeviceDefPtr *outdevs = NULL;
> + size_t noutdevs = 0;
> + size_t i, j;
> +
> + json_devicelist = virJSONValueFromString(jsonstring);
> +
> + if (!json_devicelist)
> + goto parsefailure;
> +
> + if (!virJSONValueIsArray(json_devicelist))
> + goto parsefailure;
> +
> + n = virJSONValueArraySize(json_devicelist);
> +
> + for (i = 0; i < n; i++) {
> + virJSONValuePtr obj = virJSONValueArrayGet(json_devicelist, i);
> + const char *parent;
> + virJSONValuePtr child_array;
> + int nchildren;
> +
> + if (!virJSONValueIsObject(obj))
> + goto parsefailure;
> +
> + /* 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)
> + goto parsefailure;
> +
> + parent = virJSONValueObjectGetKey(obj, 0);
> + child_array = virJSONValueObjectGetValue(obj, 0);
> +
> + if (!virJSONValueIsArray(child_array))
> + goto parsefailure;
> +
> + nchildren = virJSONValueArraySize(child_array);
> +
> + for (j = 0; j < nchildren; j++) {
> + g_autoptr(virNodeDeviceDef) child = NULL;
> + virJSONValuePtr child_obj = virJSONValueArrayGet(child_array, j);
> +
> + if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj)))
> + goto parsefailure;
> +
> + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0)
> + goto parsefailure;
> + }
> + }
> +
> + *devs = outdevs;
> + return noutdevs;
> +
> + parsefailure:
> + for (i = 0; i < noutdevs; i++)
> + virNodeDeviceDefFree(outdevs[i]);
> + VIR_FREE(outdevs);
> +
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("unable to parse JSON response"));
This error message is horrible for debugging as it tells us nothing
about what part of parsing failed. Please report error messages at
the original site of the error
> + return -1;
> +}
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2026 Red Hat, Inc.