devices: controller: Add get_attached_devices

Move the opencoded impl out of virt-manager details.py and into
virtinst, since this is entirely about XML comparison. Add tests for
it

Signed-off-by: Cole Robinson <crobinso@redhat.com>

tests/data/xmlparse/controller-attached-devices.xml

@@ -0,0 +1,191 @@
+<domain type='kvm'>
+  <name>f30</name>
+  <uuid>20b3a909-805e-4170-8698-4be84f194736</uuid>
+  <description>f30 for f30 testing</description>
+  <metadata>
+    <libosinfo:libosinfo xmlns:libosinfo="http://libosinfo.org/xmlns/libvirt/domain/1.0">
+      <libosinfo:os id="http://fedoraproject.org/fedora/30"/>
+    </libosinfo:libosinfo>
+  </metadata>
+  <memory unit='KiB'>8294400</memory>
+  <currentMemory unit='KiB'>8294400</currentMemory>
+  <vcpu placement='static'>5</vcpu>
+  <resource>
+    <partition>/machine</partition>
+  </resource>
+  <os>
+    <type arch='x86_64' machine='pc-q35-3.1'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <vmport state='off'/>
+  </features>
+  <cpu mode='custom' match='exact' check='full'>
+    <model fallback='forbid'>Skylake-Client-IBRS</model>
+    <vendor>Intel</vendor>
+    <feature policy='require' name='ss'/>
+    <feature policy='require' name='vmx'/>
+    <feature policy='require' name='hypervisor'/>
+    <feature policy='require' name='tsc_adjust'/>
+    <feature policy='require' name='clflushopt'/>
+    <feature policy='require' name='umip'/>
+    <feature policy='require' name='md-clear'/>
+    <feature policy='require' name='stibp'/>
+    <feature policy='require' name='arch-capabilities'/>
+    <feature policy='require' name='ssbd'/>
+    <feature policy='require' name='xsaves'/>
+    <feature policy='require' name='pdpe1gb'/>
+    <feature policy='require' name='skip-l1dfl-vmentry'/>
+  </cpu>
+  <clock offset='utc'>
+    <timer name='rtc' tickpolicy='catchup'/>
+    <timer name='pit' tickpolicy='delay'/>
+    <timer name='hpet' present='no'/>
+  </clock>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <pm>
+    <suspend-to-mem enabled='no'/>
+    <suspend-to-disk enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2'/>
+      <source file='/mnt/data/devel/images/f30.qcow2'/>
+      <backingStore/>
+      <target dev='vda' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw'/>
+      <target dev='sda' bus='sata'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='0' target='0' unit='3'/>
+    </disk>
+    <controller type='usb' index='0' model='qemu-xhci' ports='15'>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='pci' index='1' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='1' port='0x10'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='pci' index='2' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='2' port='0x11'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/>
+    </controller>
+    <controller type='pci' index='3' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='3' port='0x12'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x2'/>
+    </controller>
+    <controller type='pci' index='4' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='4' port='0x13'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x3'/>
+    </controller>
+    <controller type='pci' index='5' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='5' port='0x14'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x4'/>
+    </controller>
+    <controller type='pci' index='6' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='6' port='0x15'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x5'/>
+    </controller>
+    <controller type='pci' index='7' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='7' port='0x16'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x6'/>
+    </controller>
+    <controller type='pci' index='8' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='8' port='0x17'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x7'/>
+    </controller>
+    <controller type='pci' index='9' model='pcie-to-pci-bridge'>
+      <model name='pcie-pci-bridge'/>
+      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/>
+    </controller>
+    <interface type='network'>
+      <mac address='52:54:00:86:20:1c'/>
+      <source network='default'/>
+      <model type='virtio'/>
+      <link state='up'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </interface>
+    <serial type='pty'>
+      <target type='isa-serial' port='0'>
+        <model name='isa-serial'/>
+      </target>
+    </serial>
+    <console type='pty'>
+      <target type='virtio' port='0'/>
+    </console>
+    <channel type='unix'>
+      <target type='virtio' name='org.qemu.guest_agent.0'/>
+      <address type='virtio-serial' controller='0' bus='0' port='1'/>
+    </channel>
+    <channel type='spicevmc'>
+      <target type='virtio' name='com.redhat.spice.0'/>
+      <address type='virtio-serial' controller='0' bus='0' port='2'/>
+    </channel>
+    <channel type='spiceport'>
+      <source channel='org.spice-space.webdav.0'/>
+      <target type='virtio' name='org.spice-space.webdav.0'/>
+      <address type='virtio-serial' controller='0' bus='0' port='3'/>
+    </channel>
+    <input type='tablet' bus='usb'>
+      <address type='usb' bus='0' port='1'/>
+    </input>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='spice' autoport='yes'>
+      <listen type='address'/>
+      <image compression='off'/>
+    </graphics>
+    <sound model='ich9'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1b' function='0x0'/>
+    </sound>
+    <video>
+      <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </video>
+    <hostdev mode='subsystem' type='usb' managed='yes'>
+      <source>
+        <vendor id='0x1050'/>
+        <product id='0x0010'/>
+      </source>
+      <address type='usb' bus='0' port='4'/>
+    </hostdev>
+    <redirdev bus='usb' type='spicevmc'>
+      <address type='usb' bus='0' port='2'/>
+    </redirdev>
+    <redirdev bus='usb' type='spicevmc'>
+      <address type='usb' bus='0' port='3'/>
+    </redirdev>
+    <memballoon model='virtio'>
+      <stats period='5'/>
+      <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
+    </memballoon>
+    <rng model='virtio'>
+      <backend model='random'>/dev/urandom</backend>
+      <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/>
+    </rng>
+  </devices>
+  <seclabel type='dynamic' model='selinux' relabel='yes'/>
+</domain>
+

tests/test_xmlparse.py

@@ -1537,3 +1537,27 @@ class XMLParseTest(unittest.TestCase):
             if not guest2.find_device(srcdev):
                 raise AssertionError("guest.find_device failed for dev=%s" %
                         srcdev)
+
+    def testControllerAttachedDevices(self):
+        """
+        Test DeviceController.get_attached_devices
+        """
+        xml = open(DATADIR + "controller-attached-devices.xml").read()
+        guest = virtinst.Guest(self.conn, xml)
+
+        # virtio-serial path
+        controller = [c for c in guest.devices.controller if
+                c.type == "virtio-serial"][0]
+        devs = controller.get_attached_devices(guest)
+        assert len(devs) == 4
+        assert devs[-1].DEVICE_TYPE == "console"
+
+        # disk path
+        controller = [c for c in guest.devices.controller if
+                c.type == "sata"][0]
+        devs = controller.get_attached_devices(guest)
+        assert len(devs) == 1
+        assert devs[-1].device == "cdrom"
+
+        # Little test for DeviceAddress.pretty_desc
+        assert devs[-1].address.pretty_desc() == "0:0:0:3"

virtManager/details/details.py

@@ -2381,27 +2381,23 @@ class vmmDetails(vmmGObjectUI):
         elif controller.type in ["scsi", "sata", "ide", "fdc"]:
             model = self.widget("controller-device-list").get_model()
             model.clear()
-            for disk in _calculate_disk_bus_index(self.vm.xmlobj.devices.disk):
+            disks = controller.get_attached_devices(self.vm.xmlobj)
-                if disk.address.compare_controller(controller, disk.bus):
+            for disk in disks:
-                    name = _label_for_device(disk)
+                name = _label_for_device(disk)
-                    infoStr = ("%s on %s" % (name, disk.address.pretty_desc()))
+                infoStr = ("%s on %s" % (name, disk.address.pretty_desc()))
-                    model.append([infoStr])
+                model.append([infoStr])
-                    self._disable_device_remove(
+                self._disable_device_remove(
-                        _("Cannot remove controller while devices are attached."))
+                    _("Cannot remove controller while devices are attached."))
-            uiutil.set_grid_row_visible(self.widget("device-list-label"), True)
+            uiutil.set_grid_row_visible(
-            uiutil.set_grid_row_visible(self.widget("controller-device-box"), True)
+                    self.widget("device-list-label"), True)
+            uiutil.set_grid_row_visible(
+                    self.widget("controller-device-box"), True)
+
         elif controller.type == "virtio-serial":
-            for dev in self.vm.xmlobj.devices.channel:
+            devs = controller.get_attached_devices(self.vm.xmlobj)
-                if dev.address.compare_controller(controller, dev.address.type):
+            if devs:
-                    self._disable_device_remove(
+                self._disable_device_remove(
-                        _("Cannot remove controller while devices are attached."))
+                    _("Cannot remove controller while devices are attached."))
-                    break
-            for dev in self.vm.xmlobj.devices.console:
-                # virtio console is implied to be on virtio-serial index=0
-                if controller.index == 0 and dev.target_type == "virtio":
-                    self._disable_device_remove(
-                        _("Cannot remove controller while devices are attached."))
-                    break
 
         type_label = vmmAddHardware.controller_pretty_desc(controller)
         self.widget("controller-type").set_text(type_label)

virtinst/devices/controller.py

@@ -77,3 +77,36 @@ class DeviceController(Device):
     driver_queues = XMLProperty("./driver/@queues", is_int=True)
 
     master_startport = XMLProperty("./master/@startport", is_int=True)
+
+
+    def _get_attached_disk_devices(self, guest):
+        ret = []
+        for disk in guest.devices.disk:
+            if (self.type == disk.bus and
+                self.index == disk.address.controller):
+                ret.append(disk)
+        return ret
+
+    def _get_attached_virtioserial_devices(self, guest):
+        ret = []
+        for dev in guest.devices.channel:
+            if (self.type == dev.address.type and
+                self.index == dev.address.controller):
+                ret.append(dev)
+        for dev in guest.devices.console:
+            # virtio console is implied to be on virtio-serial index=0
+            if self.index == 0 and dev.target_type == "virtio":
+                ret.append(dev)
+        return ret
+
+    def get_attached_devices(self, guest):
+        """
+        Return all the Device objects from the passed Guest that are attached
+        to this controller
+        """
+        ret = []
+        if self.type == "virtio-serial":
+            ret = self._get_attached_virtioserial_devices(guest)
+        elif self.type in ["scsi", "sata", "ide", "fdc"]:
+            ret = self._get_attached_disk_devices(guest)
+        return ret

virtinst/devices/device.py

@@ -65,12 +65,6 @@ class DeviceAddress(XMLBuilder):
                             (self.controller, self.bus, self.target, self.unit))
         return pretty_desc
 
-    def compare_controller(self, controller, dev_bus):
-        if (controller.type == dev_bus and
-            controller.index == self.controller):
-            return True
-        return False
-
 
     type = XMLProperty("./@type")
     # type=pci