[PATCH virt-manager v2 0/4] Enable MDEV support

[Shalini Chellathurai Saroja]

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.
> Thanks,
> Cole
>
-- 
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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20210602/ccc34d51/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gckelmnagggoljnk.png
Type: image/png
Size: 65413 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20210602/ccc34d51/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dojdhkbeiffmifad.png
Type: image/png
Size: 66098 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20210602/ccc34d51/attachment-0001.png>