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

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

 




On 6/2/21 7:45 PM, Cole Robinson wrote:
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.
ok, then I will update the code and send v3, thank you for the review.

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





[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux