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@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