[virt-tools-list] [virt-manager PATCH RFC] Remove the difference between VirtIO-SCSI and pure SCSI disks

[Martin Kletzander]

These disk types are differentiated by the *model* of the controller
they are connected to.  I added a patch that ensures the controller
which these disk are connected to has model='virtio-scsi', but that is
breaking libvirt's address generation; having <address
type='whatever'/> specified makes all unspecified values to fallback
to zeroes.  Libvirt's address generation is very deterministic (values
are taken from the disk's target), but will be reliable for an unknown
period and it's dependant on the order of the controllers.  Other
thing we can do is not to differentiate between those two and let the
user decide since the controller model can now be changed using in UI.

There's bunch of differences between these two approaches, but mainly,
from code POV I think it is not desirable to show all the disk types
to the user.  That would mean lots of changes which might not last
long.  Also we might not be able to guarantee the disk won't be VirtIO
if the user doesn't select it, e.g. it's the only scsi controller
available in qemu.  The machine may not start if virtio-scsi
controller is not available in such qemu OTOH.  My suggestion would be
to get rid of the differences.  And to see how "huge" the patch would
be in that case, have a look yourself below (I hope it's correct, it
works for me at least).

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
 virtManager/addhardware.py | 39 ++++-----------------------------------
 1 file changed, 4 insertions(+), 35 deletions(-)

diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
index 8719829..39d0f68 100644
--- a/virtManager/addhardware.py
+++ b/virtManager/addhardware.py
@@ -822,16 +822,13 @@ class vmmAddHardware(vmmGObjectUI):
         if self.vm.is_hvm():
             model.append(["ide", "IDE"])
             model.append(["fdc", "Floppy"])
-
-            if self.vm.stable_defaults():
-                model.append(["scsi", "SCSI"])
-                model.append(["usb", "USB"])
+            model.append(["scsi", "SCSI"])
+            model.append(["usb", "USB"])

         if self.vm.get_hv_type() in ["qemu", "kvm", "test"]:
             model.append(["sata", "SATA"])
             model.append(["sd", "SD"])
             model.append(["virtio", "Virtio"])
-            model.append(["virtio-scsi", "Virtio SCSI"])

         if self.conn.is_xen() or self.conn.is_test_conn():
             model.append(["xen", "Xen"])
@@ -849,10 +846,10 @@ class vmmAddHardware(vmmGObjectUI):
         model.clear()

         disk_buses = ["ide", "sata", "scsi", "sd",
-                      "usb", "virtio", "virtio-scsi", "xen"]
+                      "usb", "virtio", "xen"]
         floppy_buses = ["fdc"]
         cdrom_buses = ["ide", "scsi"]
-        lun_buses = ["virtio-scsi"]
+        lun_buses = ["scsi"]

         if bus in disk_buses:
             model.append([virtinst.VirtualDisk.DEVICE_DISK,
@@ -1654,10 +1651,6 @@ class vmmAddHardware(vmmGObjectUI):
         controller_model = None
         conn = self.conn.get_backend()

-        if bus == "virtio-scsi":
-            bus = "scsi"
-            controller_model = "virtio-scsi"
-
         # Make sure default pool is running
         if self.is_default_storage():
             ret = sharedui.check_default_pool_active(self.err, self.conn)
@@ -1724,31 +1717,7 @@ class vmmAddHardware(vmmGObjectUI):

         sharedui.check_path_search_for_qemu(self.err, self.conn, disk.path)

-        # Add a SCSI controller with model virtio-scsi if needed
         disk.vmm_controller = None
-        if (controller_model == "virtio-scsi") and (bus == "scsi"):
-            controllers = self.vm.get_controller_devices()
-            ctrls_scsi = [x for x in controllers if
-                    (x.type == VirtualController.TYPE_SCSI)]
-            if len(ctrls_scsi) > 0:
-                index_new = max([x.index for x in ctrls_scsi]) + 1
-            else:
-                index_new = 0
-            controller = VirtualController(conn)
-            controller.type = "scsi"
-            controller.model = controller_model
-            disk.vmm_controller = controller
-            for d in controllers:
-                if controller.type == d.type:
-                    controller.index = index_new
-                if controller_model == d.model:
-                    disk.vmm_controller = None
-                    controller = d
-                    break
-
-            disk.address.type = disk.address.ADDRESS_TYPE_DRIVE
-            disk.address.controller = controller.index
-
         self._dev = disk
         return True

-- 
1.8.5.3