[PATCH 0/3] Mdev test cleanup, completion and parent addr exposure
Clean up the testing json. Add a test for vfio-ccw mdev. Expose the parent address in the XML when dumping a mdev nodedev as the address is now part of the mdev nodedev name and users should not parse and manipulate the mdev nodedev name to retrieve it. Boris Fiuczynski (3): tests: correct formating in mdevctl test tests: adding vfio-ccw to nodedev tests nodedev: add parent_addr to mdev nodedev dumpxml docs/schemas/nodedev.rng | 5 ++ src/conf/node_device_conf.c | 2 + ...52_9b13_9b13_9b13_cc23009b1326-create.argv | 5 ++ ...52_9b13_9b13_9b13_cc23009b1326-create.json | 1 + ...52_9b13_9b13_9b13_cc23009b1326-define.argv | 5 ++ ...52_9b13_9b13_9b13_cc23009b1326-define.json | 1 + .../mdevctl-list-multiple.json | 56 +++++++++++-------- .../mdevctl-list-multiple.out.xml | 14 +++++ tests/nodedevmdevctltest.c | 31 +++++++++- ...v_cc000052_9b13_9b13_9b13_cc23009b1326.xml | 8 +++ 10 files changed, 104 insertions(+), 24 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.argv create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.json create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.argv create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.json create mode 100644 tests/nodedevschemadata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326.xml -- 2.33.1
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- .../mdevctl-list-multiple.json | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json b/tests/nodedevmdevctldata/mdevctl-list-multiple.json index ca1918d00a..98e46584a0 100644 --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.json +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json @@ -26,30 +26,31 @@ } ], "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" - } - ] - } + { + "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" + } + ] + } } ] } -- 2.33.1
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- ...52_9b13_9b13_9b13_cc23009b1326-create.argv | 5 +++ ...52_9b13_9b13_9b13_cc23009b1326-create.json | 1 + ...52_9b13_9b13_9b13_cc23009b1326-define.argv | 5 +++ ...52_9b13_9b13_9b13_cc23009b1326-define.json | 1 + .../mdevctl-list-multiple.json | 9 ++++++ .../mdevctl-list-multiple.out.xml | 9 ++++++ tests/nodedevmdevctltest.c | 31 ++++++++++++++++++- ...v_cc000052_9b13_9b13_9b13_cc23009b1326.xml | 8 +++++ 8 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.argv create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.json create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.argv create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.json create mode 100644 tests/nodedevschemadata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326.xml diff --git a/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.argv b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.argv new file mode 100644 index 0000000000..6d5c6e1cb8 --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.argv @@ -0,0 +1,5 @@ +mdevctl \ +start \ +--parent=0.0.0052 \ +--jsonfile=/dev/stdin \ +--uuid=cc000052-9b13-9b13-9b13-cc23009b1326 diff --git a/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.json b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.json new file mode 100644 index 0000000000..2dced5c695 --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.json @@ -0,0 +1 @@ +{"mdev_type":"vfio_ccw-io","start":"manual"} diff --git a/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.argv b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.argv new file mode 100644 index 0000000000..997ce08fb8 --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.argv @@ -0,0 +1,5 @@ +mdevctl \ +define \ +--parent=0.0.0052 \ +--jsonfile=/dev/stdin \ +--uuid=cc000052-9b13-9b13-9b13-cc23009b1326 diff --git a/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.json b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.json new file mode 100644 index 0000000000..2dced5c695 --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.json @@ -0,0 +1 @@ +{"mdev_type":"vfio_ccw-io","start":"manual"} diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json b/tests/nodedevmdevctldata/mdevctl-list-multiple.json index 98e46584a0..ee6641df6f 100644 --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.json +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json @@ -52,6 +52,15 @@ ] } } + ], + "0.0.0052": [ + { + "cc000052-9b13-9b13-9b13-cc23009b1326": { + "mdev_type": "vfio_ccw-io", + "start": "manual", + "attrs": [] + } + } ] } ] diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml index 4b558a1464..4df5f4f43c 100644 --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml @@ -41,3 +41,12 @@ <attr name='assign_control_domain' value='4'/> </capability> </device> +<device> + <name>mdev_cc000052_9b13_9b13_9b13_cc23009b1326_0_0_0052</name> + <parent>css_0_0_0052</parent> + <capability type='mdev'> + <type id='vfio_ccw-io'/> + <uuid>cc000052-9b13-9b13-9b13-cc23009b1326</uuid> + <iommuGroup number='0'/> + </capability> +</device> diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 92d3c75766..d660d4cc21 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -331,6 +331,32 @@ fakeMatrixDevice(void) return def; } + +/* Add a fake css device that can be used as a parent device for mediated + * devices. For our purposes, it only needs to have a name that matches the + * parent of the mdev, and it needs the proper name + */ +static virNodeDeviceDef * +fakeCSSDevice(void) +{ + virNodeDeviceDef *def = NULL; + virNodeDevCapCCW *css_dev; + + def = g_new0(virNodeDeviceDef, 1); + def->caps = g_new0(virNodeDevCapsDef, 1); + + def->name = g_strdup("css_0_0_0052"); + def->parent = g_strdup("computer"); + + def->caps->data.type = VIR_NODE_DEV_CAP_CSS_DEV; + css_dev = &def->caps->data.ccw_dev; + css_dev->cssid = 0; + css_dev->ssid = 0; + css_dev->devno = 82; + + return def; +} + static int addDevice(virNodeDeviceDef *def) { @@ -354,7 +380,8 @@ nodedevTestDriverAddTestDevices(void) { if (addDevice(fakeRootDevice()) < 0 || addDevice(fakePCIDevice()) < 0 || - addDevice(fakeMatrixDevice()) < 0) + addDevice(fakeMatrixDevice()) < 0 || + addDevice(fakeCSSDevice()) < 0) return -1; return 0; @@ -437,6 +464,7 @@ mymain(void) DO_TEST_CREATE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); DO_TEST_CREATE("mdev_fedc4916_1ca8_49ac_b176_871d16c13076"); DO_TEST_CREATE("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9"); + DO_TEST_CREATE("mdev_cc000052_9b13_9b13_9b13_cc23009b1326"); /* Test mdevctl stop command, pass an arbitrary uuid */ DO_TEST_STOP("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); @@ -449,6 +477,7 @@ mymain(void) DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); DO_TEST_DEFINE("mdev_fedc4916_1ca8_49ac_b176_871d16c13076"); DO_TEST_DEFINE("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9"); + DO_TEST_DEFINE("mdev_cc000052_9b13_9b13_9b13_cc23009b1326"); DO_TEST_UNDEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); diff --git a/tests/nodedevschemadata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326.xml b/tests/nodedevschemadata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326.xml new file mode 100644 index 0000000000..8a779464d8 --- /dev/null +++ b/tests/nodedevschemadata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326.xml @@ -0,0 +1,8 @@ +<device> + <name>mdev_cc000052_9b13_9b13_9b13_cc23009b1326</name> + <parent>css_0_0_0052</parent> + <capability type='mdev'> + <type id='vfio_ccw-io'/> + <uuid>cc000052-9b13-9b13-9b13-cc23009b1326</uuid> + </capability> +</device> -- 2.33.1
As the parent address is part of the mdev nodedev name lets expose the internally available parent address in the XML. Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/schemas/nodedev.rng | 5 +++++ src/conf/node_device_conf.c | 2 ++ tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index e4733f0804..0d7d1168b6 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -644,6 +644,11 @@ <ref name="UUID"/> </element> </optional> + <optional> + <element name="parent_addr"> + <data type="string"/> + </element> + </optional> <zeroOrMore> <element name="attr"> <attribute name="name"/> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 0bac0fde8d..61c8715037 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -604,6 +604,8 @@ virNodeDeviceCapMdevDefFormat(virBuffer *buf, virBufferEscapeString(buf, "<type id='%s'/>\n", data->mdev.type); virBufferEscapeString(buf, "<uuid>%s</uuid>\n", data->mdev.uuid); + virBufferEscapeString(buf, "<parent_addr>%s</parent_addr>\n", + data->mdev.parent_addr); virBufferAsprintf(buf, "<iommuGroup number='%u'/>\n", data->mdev.iommuGroupNumber); diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml index 4df5f4f43c..012c1a1596 100644 --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml @@ -4,6 +4,7 @@ <capability type='mdev'> <type id='i915-GVTg_V5_4'/> <uuid>200f228a-c80a-4d50-bfb7-f5a0e4e34045</uuid> + <parent_addr>0000:00:02.0</parent_addr> <iommuGroup number='0'/> </capability> </device> @@ -13,6 +14,7 @@ <capability type='mdev'> <type id='i915-GVTg_V5_4'/> <uuid>de807ffc-1923-4d5f-b6c9-b20ecebc6d4b</uuid> + <parent_addr>0000:00:02.0</parent_addr> <iommuGroup number='0'/> </capability> </device> @@ -22,6 +24,7 @@ <capability type='mdev'> <type id='i915-GVTg_V5_8'/> <uuid>435722ea-5f43-468a-874f-da34f1217f13</uuid> + <parent_addr>0000:00:02.0</parent_addr> <iommuGroup number='0'/> <attr name='testattr' value='42'/> </capability> @@ -32,6 +35,7 @@ <capability type='mdev'> <type id='vfio_ap-passthrough'/> <uuid>783e6dbb-ea0e-411f-94e2-717eaad438bf</uuid> + <parent_addr>matrix</parent_addr> <iommuGroup number='0'/> <attr name='assign_adapter' value='5'/> <attr name='assign_adapter' value='6'/> @@ -47,6 +51,7 @@ <capability type='mdev'> <type id='vfio_ccw-io'/> <uuid>cc000052-9b13-9b13-9b13-cc23009b1326</uuid> + <parent_addr>0.0.0052</parent_addr> <iommuGroup number='0'/> </capability> </device> -- 2.33.1
On 2/4/22 9:32 AM, Boris Fiuczynski wrote:
As the parent address is part of the mdev nodedev name lets expose the internally available parent address in the XML.
What is the issue that you're trying to solve here? The mdev xml already has a reference to the name of the parent device. So presumably you can look up the parent nodedev by name and then find its address, no? This seems like you're duplicating parent information in the child. Is it just for convenience? Jonathon
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/schemas/nodedev.rng | 5 +++++ src/conf/node_device_conf.c | 2 ++ tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml | 5 +++++ 3 files changed, 12 insertions(+)
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index e4733f0804..0d7d1168b6 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -644,6 +644,11 @@ <ref name="UUID"/> </element> </optional> + <optional> + <element name="parent_addr"> + <data type="string"/> + </element> + </optional> <zeroOrMore> <element name="attr"> <attribute name="name"/> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 0bac0fde8d..61c8715037 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -604,6 +604,8 @@ virNodeDeviceCapMdevDefFormat(virBuffer *buf,
virBufferEscapeString(buf, "<type id='%s'/>\n", data->mdev.type); virBufferEscapeString(buf, "<uuid>%s</uuid>\n", data->mdev.uuid); + virBufferEscapeString(buf, "<parent_addr>%s</parent_addr>\n", + data->mdev.parent_addr); virBufferAsprintf(buf, "<iommuGroup number='%u'/>\n", data->mdev.iommuGroupNumber);
diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml index 4df5f4f43c..012c1a1596 100644 --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml @@ -4,6 +4,7 @@ <capability type='mdev'> <type id='i915-GVTg_V5_4'/> <uuid>200f228a-c80a-4d50-bfb7-f5a0e4e34045</uuid> + <parent_addr>0000:00:02.0</parent_addr> <iommuGroup number='0'/> </capability> </device> @@ -13,6 +14,7 @@ <capability type='mdev'> <type id='i915-GVTg_V5_4'/> <uuid>de807ffc-1923-4d5f-b6c9-b20ecebc6d4b</uuid> + <parent_addr>0000:00:02.0</parent_addr> <iommuGroup number='0'/> </capability> </device> @@ -22,6 +24,7 @@ <capability type='mdev'> <type id='i915-GVTg_V5_8'/> <uuid>435722ea-5f43-468a-874f-da34f1217f13</uuid> + <parent_addr>0000:00:02.0</parent_addr> <iommuGroup number='0'/> <attr name='testattr' value='42'/> </capability> @@ -32,6 +35,7 @@ <capability type='mdev'> <type id='vfio_ap-passthrough'/> <uuid>783e6dbb-ea0e-411f-94e2-717eaad438bf</uuid> + <parent_addr>matrix</parent_addr> <iommuGroup number='0'/> <attr name='assign_adapter' value='5'/> <attr name='assign_adapter' value='6'/> @@ -47,6 +51,7 @@ <capability type='mdev'> <type id='vfio_ccw-io'/> <uuid>cc000052-9b13-9b13-9b13-cc23009b1326</uuid> + <parent_addr>0.0.0052</parent_addr> <iommuGroup number='0'/> </capability> </device>
On 2/4/22 6:10 PM, Jonathon Jongsma wrote:
On 2/4/22 9:32 AM, Boris Fiuczynski wrote:
As the parent address is part of the mdev nodedev name lets expose the internally available parent address in the XML.
What is the issue that you're trying to solve here? The mdev xml already has a reference to the name of the parent device. So presumably you can look up the parent nodedev by name and then find its address, no? This seems like you're duplicating parent information in the child. Is it just for convenience?
Jonathon
Actually it is not convenience. It is possible that an mdev definition exists and the parent device does not, e.g. the device driver has not been loaded or no longer is loaded and therefore the parent device might not/no longer exist or simply the parent device does not (for whatever reason) exist on the system at all. The reason to extend the mdev nodedev object names with the parent device address was to make it unique. At the same time nodedev object names are supposed to be treated as arbitrary strings. It is possible to retrieve the UUID but without the parent address or by looking at the nodedev object name and interpreting its last digits the information is not unique enough to find the related mdevctl definition. I guess that for correlation reasons it is stored internally on the nodedev mdev objects. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 2/4/22 16:32, Boris Fiuczynski wrote:
Clean up the testing json. Add a test for vfio-ccw mdev. Expose the parent address in the XML when dumping a mdev nodedev as the address is now part of the mdev nodedev name and users should not parse and manipulate the mdev nodedev name to retrieve it.
Boris Fiuczynski (3): tests: correct formating in mdevctl test tests: adding vfio-ccw to nodedev tests nodedev: add parent_addr to mdev nodedev dumpxml
docs/schemas/nodedev.rng | 5 ++ src/conf/node_device_conf.c | 2 + ...52_9b13_9b13_9b13_cc23009b1326-create.argv | 5 ++ ...52_9b13_9b13_9b13_cc23009b1326-create.json | 1 + ...52_9b13_9b13_9b13_cc23009b1326-define.argv | 5 ++ ...52_9b13_9b13_9b13_cc23009b1326-define.json | 1 + .../mdevctl-list-multiple.json | 56 +++++++++++-------- .../mdevctl-list-multiple.out.xml | 14 +++++ tests/nodedevmdevctltest.c | 31 +++++++++- ...v_cc000052_9b13_9b13_9b13_cc23009b1326.xml | 8 +++ 10 files changed, 104 insertions(+), 24 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.argv create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-create.json create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.argv create mode 100644 tests/nodedevmdevctldata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326-define.json create mode 100644 tests/nodedevschemadata/mdev_cc000052_9b13_9b13_9b13_cc23009b1326.xml
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Boris Fiuczynski -
Jonathon Jongsma -
Michal Prívozník