On 6/2/21 9:34 AM, Shalini Chellathurai Saroja wrote:
>
> On 6/1/21 11:13 PM, Cole Robinson wrote:
>> On 5/31/21 3:54 PM, Shalini Chellathurai Saroja wrote:
>>> Enable support for mediated devices in virt-xml tool and virt-manager
>>> GUI tool.
>>>
>> Thanks! I pushed 1-3 with a small tweak to patch #1 to keep code
>> coverage at 100%.
>
> Good morning Cole,
>
> Thank you for reviewing and pushing the virt-xml patch.
>
>> In order to get the parent naming stuff in addhardware.py to trigger I
>> needed to change things a bit:
>>
>> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
>> index 2df84bf5..13b899c3 100644
>> --- a/virtManager/addhardware.py
>> +++ b/virtManager/addhardware.py
>> @@ -783,9 +783,10 @@ class vmmAddHardware(vmmGObjectUI):
>> prettyname += " (%s)" % subdev.pretty_name()
>>
>> if devtype == "mdev":
>> - for subdev in self.conn.filter_nodedevs("mdev"):
>> - if dev.xmlobj.name == subdev.xmlobj.parent:
>> - prettyname += " (%s)" % subdev.pretty_name()
>> + for parentdev in self.conn.list_nodedevs():
>> + if dev.xmlobj.parent == parentdev.xmlobj.name:
>> + prettyname = "%s %s" % (
>> + parentdev.pretty_name(), prettyname)
>>
>> model.append([dev.xmlobj, prettyname])
>>
>>
>> Was it working in your code before that, like on a real system with mdev
>> devices? Or was the Add Hardware->MDEV only listing devices with name
>> `mdev_$UUID` ?
> MDEV devices are listed as mdev_$UUID in my system with MDEV devices.
> I have provided a screenshot with my code below.
>
>> The point of this code is to give more descriptive names to what we list
>> in the UI, if we can help it. The names for 'mdev' devices are not
>> descriptive.
> Yes, your code provides a more descriptive name and helps to
> identify the device type (like CCW/PCI/AP device) . We can use it.
> I have provided a screenshot with your code below.
>
>> Their type id=XXX values are a bit better:
>> vfio_ap-passthrough, nvidia-11, etc. Maybe we can just use that? The
>> parent devices have a bit more info under <capability><type><name> but
>> it's not clear if that's always available. Maybe we can get away with
>> just using the <type id=XXX> value in the mdev pretty_name() definition
> More than one MDEV device can have the same type id as shown below.
> May be, we can use type id in combination with mdev_$uuid.
> I feel like your suggested code is better, as we could identify the parent
> device and MDEV device type with this code.
>
> # virsh nodedev-dumpxml mdev_8e782fea_e5f4_45fa_a0f9_024cf66e5009
> <device>
> <name>mdev_8e782fea_e5f4_45fa_a0f9_024cf66e5009</name>
>
> <path>/sys/devices/css0/0.0.0024/8e782fea-e5f4-45fa-a0f9-024cf66e5009</path>
> <parent>css_0_0_0024</parent>
> <driver>
> <name>vfio_mdev</name>
> </driver>
> <capability type='mdev'>
> *<type id='vfio_ccw-io'/>*
> <uuid>8e782fea-e5f4-45fa-a0f9-024cf66e5009</uuid>
> <iommuGroup number='1'/>
> </capability>
> </device>
>
> # virsh nodedev-dumpxml mdev_92cc4b96_95d8_47ce_923c_c688d061bc41
> <device>
> <name>mdev_92cc4b96_95d8_47ce_923c_c688d061bc41</name>
>
> <path>/sys/devices/css0/0.0.0023/92cc4b96-95d8-47ce-923c-c688d061bc41</path>
> <parent>css_0_0_0023</parent>
> <driver>
> <name>vfio_mdev</name>
> </driver>
> <capability type='mdev'>
> *<type id='vfio_ccw-io'/>*
> <uuid>92cc4b96-95d8-47ce-923c-c688d061bc41</uuid>
> <iommuGroup number='2'/>
> </capability>
> </device>
>
> Similarly, <capability><type><name> info in MDEV parent device
> can also be same for more than one MDEV device.
>
> # virsh nodedev-dumpxml css_0_0_0023
> <device>
> <name>css_0_0_0023</name>
> <path>/sys/devices/css0/0.0.0023</path>
> <parent>computer</parent>
> <driver>
> <name>vfio_ccw</name>
> </driver>
> <capability type='css'>
> <cssid>0x0</cssid>
> <ssid>0x0</ssid>
> <devno>0x0023</devno>
> <capability type='mdev_types'>
> <type id='vfio_ccw-io'>
> *<name>I/O subchannel (Non-QDIO)</name>*
> <deviceAPI>vfio-ccw</deviceAPI>
> <availableInstances>0</availableInstances>
> </type>
> </capability>
> </capability>
> </device>
>
> # virsh nodedev-dumpxml css_0_0_0024
> <device>
> <name>css_0_0_0024</name>
> <path>/sys/devices/css0/0.0.0024</path>
> <parent>computer</parent>
> <driver>
> <name>vfio_ccw</name>
> </driver>
> <capability type='css'>
> <cssid>0x0</cssid>
> <ssid>0x0</ssid>
> <devno>0x0024</devno>
> <capability type='mdev_types'>
> <type id='vfio_ccw-io'>
> *<name>I/O subchannel (Non-QDIO)</name>*
> <deviceAPI>vfio-ccw</deviceAPI>
> <availableInstances>0</availableInstances>
> </type>
> </capability>
> </capability>
> </device>
>
> Are you ok with using your suggested code (parent name mdev_$uuid)
> for prettyname? Please let me know your feedback.
Yes if you think that's looks good on a real machine, then that works
for me too.
Thanks,
Cole
