[libvirt PATCH 1/2] nodedev: fix parent device of inactive mdevs
Boris Fiuczynski
fiuczy at linux.ibm.com
Fri Jul 9 11:11:21 UTC 2021
Some observations without these patches
# mdevctl list -d
e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0033 vfio_ccw-io manual (active)
# virsh nodedev-list --cap mdev
mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
# virsh nodedev-list --inactive --cap mdev
# virsh nodedev-dumpxml mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<path>/sys/devices/css0/0.0.0033/e60cef97-3f6b-485e-ac46-0520f9f66ac2</path>
<parent>css_0_0_0033</parent>
<driver>
<name>vfio_mdev</name>
</driver>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
# virsh nodedev-destroy mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
Destroyed node device 'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2'
# virsh nodedev-list --inactive --cap mdev
mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
QUESTION: My mdev is defined and active. I know this from looking at
mdevctl.
As the option inactive seems not to match the mdevctl option defined how
can I find out that I can e.g. use nodedev-undefine without
stopping/destroying it first?
Do we need another option like defined on nodedev-list?
Anyway using nodedev-dumpxml I get the parent correctly.
# virsh nodedev-dumpxml mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<parent>css_0_0_0033</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
And now the wrap up to start over again...
# virsh nodedev-undefine mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
Undefined node device 'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2'
# virsh nodedev-list --inactive --cap mdev
mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
# virsh nodedev-list --all --cap mdev
mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
# virsh nodedev-list --cap mdev
# mdevctl list -d
# mdevctl list
# virsh nodedev-dumpxml mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<parent>css_0_0_0033</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
# virsh nodedev-start mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
error: Failed to start device mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
error: internal error: Unable to create mediated device: Config for
e60cef97-3f6b-485e-ac46-0520f9f66ac2 does not exist, define it first?
# virsh nodedev-undefine mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
Undefined node device 'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2'
# virsh nodedev-list --all --cap mdev
mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
That is definitely a bug.
But in all dumped XMLs the parent seems to be provided correctly.
# cat mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2.xml
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<path>/sys/devices/css0/0.0.0033/e60cef97-3f6b-485e-ac46-0520f9f66ac2</path>
<parent>css_0_0_0033</parent>
<driver>
<name>vfio_mdev</name>
</driver>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
# virsh nodedev-define mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2.xml
Node device 'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2' defined from
'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2.xml'
# virsh nodedev-list --all --cap mdev
mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
# mdevctl list -d
e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0033 vfio_ccw-io manual
# virsh nodedev-dumpxml mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<parent>css_0_0_0033</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
Rerunning it with the dumpxml from the defined only mdev.
# virsh nodedev-undefine mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
Undefined node device 'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2'
# mdevctl list -d
# virsh nodedev-dumpxml mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<parent>css_0_0_0033</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
# cat mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2_defined.xml
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<parent>css_0_0_0033</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
# virsh nodedev-define mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2_defined.xml
Node device 'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2' defined from
'mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2_defined.xml'
# mdevctl list -d
e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0033 vfio_ccw-io manual
# virsh nodedev-dumpxml mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2
<device>
<name>mdev_e60cef97_3f6b_485e_ac46_0520f9f66ac2</name>
<parent>css_0_0_0033</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>e60cef97-3f6b-485e-ac46-0520f9f66ac2</uuid>
<iommuGroup number='1'/>
</capability>
</device>
So either I misunderstood the problem you are trying to resolve or it
exists on PCI only.
On 7/7/21 11:29 PM, Jonathon Jongsma wrote:
> Inactive mdevs were simply formatting their parent name as the value
> received from mdevctl rather than looking up the libvirt nodedev name of
> the parent device. This resulted in a parent value of e.g.
> '0000:5b:00.0' instead of 'pci_0000_5b_00_0'. This prevented defining a
> new mdev device from the output of nodedev-dumpxml.
>
> Unfortunately, it's not simple to fix this comprehensively due to the
> fact that mdevctl supports defining (inactive) mdevs for parent devices
> that do not actually exist on the host (yet). So for those persistent
> mdev definitions that do not have a valid parent in the device list, the
> parent device will be set to the root "computer" device.
>
> Unfortunately, because the value of the 'parent' field now depends on
> the configuration of the host, the mdevctl parsing test will output
> 'computer' for all test devices. Fixing this would require a more
> extensive mock test environment.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1979761
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
>
> fixup
> ---
> src/node_device/node_device_driver.c | 20 ++++++++++++++++++-
> .../mdevctl-list-multiple.out.xml | 8 ++++----
> 2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index b4dd57e5f4..26b0c2f032 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1051,6 +1051,7 @@ nodeDeviceParseMdevctlChildDevice(const char *parent,
> virJSONValue *props;
> virJSONValue *attrs;
> g_autoptr(virNodeDeviceDef) child = g_new0(virNodeDeviceDef, 1);
> + g_autofree char *parent_sysfs_path = NULL;
>
> /* the child object should have a single key equal to its uuid.
> * The value is an object describing the properties of the mdev */
> @@ -1060,7 +1061,24 @@ nodeDeviceParseMdevctlChildDevice(const char *parent,
> uuid = virJSONValueObjectGetKey(json, 0);
> props = virJSONValueObjectGetValue(json, 0);
>
> - child->parent = g_strdup(parent);
> + /* Look up id of parent device. mdevctl supports defining mdevs for parent
> + * devices that are not present on the system (to support starting mdevs on
> + * hotplug, etc) so the parent may not actually exist. */
> + parent_sysfs_path = g_strdup_printf("/sys/class/mdev_bus/%s", parent);
> + if (virFileExists(parent_sysfs_path)) {
> + g_autofree char *canon_syspath = realpath(parent_sysfs_path, NULL);
> + virNodeDeviceObj *parentobj = NULL;
> + if ((parentobj = virNodeDeviceObjListFindBySysfsPath(driver->devs,
> + canon_syspath))) {
> + virNodeDeviceDef *parentdef = virNodeDeviceObjGetDef(parentobj);
> + child->parent = g_strdup(parentdef->name);
> + virNodeDeviceObjEndAPI(&parentobj);
> +
> + child->parent_sysfs_path = g_steal_pointer(&canon_syspath);
> + }
> + }
> + if (!child->parent)
> + child->parent = g_strdup("computer");
> child->caps = g_new0(virNodeDevCapsDef, 1);
> child->caps->data.type = VIR_NODE_DEV_CAP_MDEV;
>
> diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
> index cf7e966256..eead6f2a40 100644
> --- a/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
> +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
> @@ -1,6 +1,6 @@
> <device>
> <name>mdev_200f228a_c80a_4d50_bfb7_f5a0e4e34045</name>
> - <parent>0000:00:02.0</parent>
> + <parent>computer</parent>
> <capability type='mdev'>
> <type id='i915-GVTg_V5_4'/>
> <uuid>200f228a-c80a-4d50-bfb7-f5a0e4e34045</uuid>
> @@ -9,7 +9,7 @@
> </device>
> <device>
> <name>mdev_de807ffc_1923_4d5f_b6c9_b20ecebc6d4b</name>
> - <parent>0000:00:02.0</parent>
> + <parent>computer</parent>
> <capability type='mdev'>
> <type id='i915-GVTg_V5_4'/>
> <uuid>de807ffc-1923-4d5f-b6c9-b20ecebc6d4b</uuid>
> @@ -18,7 +18,7 @@
> </device>
> <device>
> <name>mdev_435722ea_5f43_468a_874f_da34f1217f13</name>
> - <parent>0000:00:02.0</parent>
> + <parent>computer</parent>
> <capability type='mdev'>
> <type id='i915-GVTg_V5_8'/>
> <uuid>435722ea-5f43-468a-874f-da34f1217f13</uuid>
> @@ -28,7 +28,7 @@
> </device>
> <device>
> <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name>
> - <parent>matrix</parent>
> + <parent>computer</parent>
> <capability type='mdev'>
> <type id='vfio_ap-passthrough'/>
> <uuid>783e6dbb-ea0e-411f-94e2-717eaad438bf</uuid>
>
--
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
More information about the libvir-list
mailing list