4:29 p.m.
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(a)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>
--
2.31.1
4:29 p.m.
New subject: [libvirt PATCH 2/2] virsh: allow nodedev-list --all --tree
Allow the tree view with --all so that we can see all inactive mdevs in
a tree structure nested under their parent devices.
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
tools/virsh-nodedev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 8c2086b71f..5b1afe4601 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -428,8 +428,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
return false;
}
- if (tree && (cap_str || inactive || all)) {
- vshError(ctl, "%s", _("Option --tree is incompatible with other
options"));
+ if (tree && (cap_str || inactive)) {
+ vshError(ctl, "%s", _("Option --tree is incompatible with --cap
and --inactive"));
return false;
}
--
2.31.1
9:45 a.m.
New subject: [libvirt PATCH 2/2] virsh: allow nodedev-list --all --tree
On 7/7/21 11:29 PM, Jonathon Jongsma wrote:
Allow the tree view with --all so that we can see all inactive mdevs
in
a tree structure nested under their parent devices.
Signed-off-by: Jonathon Jongsma<jjongsma(a)redhat.com>
Reviewed-by: Shalini
Chellathurai Saroja <shalini(a)linux.ibm.com>
--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
4:33 a.m.
New subject: [libvirt PATCH 2/2] virsh: allow nodedev-list --all --tree
On 7/7/21 11:29 PM, Jonathon Jongsma wrote:
Allow the tree view with --all so that we can see all inactive mdevs
in
a tree structure nested under their parent devices.
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
tools/virsh-nodedev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
5:18 a.m.
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(a)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>
Hello Jonathon,
While testing the patches, I noticed that a mdev is listed with virsh
nodedev-list,
even after the mdev device is undefined and is not available in mdevctl.
I have provided
below the test results for your reference.
$ lsmod | grep vfio_ap
vfio_ap 28672 0
kvm 454656 1 vfio_ap
mdev 24576 3 vfio_ccw,vfio_mdev,vfio_ap
vfio 36864 5
vfio_ccw,vfio_mdev,vfio_iommu_type1,vfio_pci,vfio_ap
$ ./tools/virsh nodedev-list --all | grep ap_matrix
ap_matrix
$ ./tools/virsh nodedev-list --cap mdev --inactive
$ ./tools/virsh nodedev-define ../../mdev-inactive.xml
Node device 'mdev_9eb4280d_2752_429a_b91b_813b4a5f4598' defined from
'../../mdev-inactive.xml'
$ ./tools/virsh nodedev-dumpxml mdev_9eb4280d_2752_429a_b91b_813b4a5f4598
<device>
<name>mdev_9eb4280d_2752_429a_b91b_813b4a5f4598</name>
<parent>css_0_0_0024</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<uuid>9eb4280d-2752-429a-b91b-813b4a5f4598</uuid>
<iommuGroup number='0'/>
</capability>
</device>
$ ./tools/virsh nodedev-list --cap mdev --inactive
mdev_9eb4280d_2752_429a_b91b_813b4a5f4598
$ ./tools/virsh *nodedev-undefine* mdev_9eb4280d_2752_429a_b91b_813b4a5f4598
Undefined node device 'mdev_9eb4280d_2752_429a_b91b_813b4a5f4598'
$ ./tools/virsh nodedev-list --cap mdev --inactive
mdev_9eb4280d_2752_429a_b91b_813b4a5f4598
$ mdevctl list --defined
--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
6:11 a.m.
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(a)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
9:48 a.m.
On Fri, Jul 9, 2021 at 6:11 AM Boris Fiuczynski <fiuczy(a)linux.ibm.com> wrote:
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.
This looks like the bug parsing an empty list that you just submitted
a patch for.
# 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.
Ah, this bug only manifests for inactive devices that were loaded from
mdevctl without first being processed by libvirt some other way. So,
in your example, the bug is not present because libvirt parsed the
appropriate parent id from your XML input. Here are a couple simple
ways to reproduce the bug:
- define a device outside of libvirt (using mdevctl directly) and
then dump the xml in libvirt.
- define the device with nodedev-define, then restart the libvirt
daemon so that the cached value from the XML input is forgotten. Then
dump xml for the defined device.
Jonathon
10:26 a.m.
On 7/9/21 4:48 PM, Jonathon Jongsma wrote:
On Fri, Jul 9, 2021 at 6:11 AM Boris Fiuczynski
<fiuczy(a)linux.ibm.com> wrote:
>
> 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.
This looks like the bug parsing an empty list that you just submitted
a patch for.
Yes, but I might have broken another path with that fix...
nodedev-create followed by nodedev-dumpxml
> # 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.
Ah, this bug only manifests for inactive devices that were loaded from
mdevctl without first being processed by libvirt some other way. So,
in your example, the bug is not present because libvirt parsed the
appropriate parent id from your XML input. Here are a couple simple
ways to reproduce the bug:
- define a device outside of libvirt (using mdevctl directly) and
then dump the xml in libvirt.
- define the device with nodedev-define, then restart the libvirt
daemon so that the cached value from the XML input is forgotten. Then
dump xml for the defined device.
Shalini or I will give that a spin
Jonathon
--
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
8:46 a.m.
> Ah, this bug only manifests for inactive devices that were loaded
from
> mdevctl without first being processed by libvirt some other way. So,
> in your example, the bug is not present because libvirt parsed the
> appropriate parent id from your XML input. Here are a couple simple
> ways to reproduce the bug:
> - define a device outside of libvirt (using mdevctl directly) and
> then dump the xml in libvirt.
> - define the device with nodedev-define, then restart the libvirt
> daemon so that the cached value from the XML input is forgotten. Then
> dump xml for the defined device.
Shalini or I will give that a spin
>
> Jonathon
>
Hello Jonathon,
I tested the patches with a mdev device of type vfio_ccw, these patches
fix these issues. Thank you.
--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
9:43 a.m.
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(a)redhat.com>
Hello Jonathon,
I also tested the patch with vfio_ap devices. It works fine. Thank you.
Reviewed-by: Shalini Chellathurai Saroja <shalini(a)linux.ibm.com>
--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
4:33 a.m.
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(a)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);
I wonder why syntax-check did not catch this. We prefer
virFileCanonicalizePath().
+ virNodeDeviceObj *parentobj = NULL;
+ if ((parentobj = virNodeDeviceObjListFindBySysfsPath(driver->devs,
And we also like an empty line between these two ^^ to break the block
into two.
+
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;
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1224
days inactive
1236
days old
10 comments
4 participants
participants (4)
-
Boris Fiuczynski -
Jonathon Jongsma -
Michal Prívozník -
Shalini Chellathurai Saroja