Re: [libvirt PATCH 1/2] nodedev: fix parent device of inactive mdevs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux