addhardware: Remove dead virtio-scsi collision code

The way we set controller_model earlier, means all the virtio-scsi
allocation code is essentially never set. That code does still fix
a valid case of when trying to add a scsi device when there isn't
any remaining slots open, but that should be rare enough that I'm
fine telling the user to edit manually set up a controller themselves
first.

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

tests/test_xmlconfig.py

@@ -113,11 +113,6 @@ class TestXMLMisc(unittest.TestCase):
         self.assertEqual("hdc", disk.generate_target(["hdb", "sda"]))
         self.assertEqual("hdb", disk.generate_target(["hda", "hdd"]))
 
-        disk.bus = "scsi"
-        self.assertEqual("sdb",
-            disk.generate_target(["sda", "sdg", "sdi"], 0))
-        self.assertEqual("sdh", disk.generate_target(["sda", "sdg"], 1))
-
     def testQuickTreeinfo(self):
         # Simple sanity test to make sure detect_distro works. test-urls
         # does much more exhaustive testing but it's only run occasionally

virtManager/addhardware.py

@@ -1483,51 +1483,17 @@ class vmmAddHardware(vmmGObjectUI):
         dev.set_defaults(self.vm.get_xmlobj())
         return dev
 
-    def _set_disk_controller(self, disk, controller_model, used_disks):
+    def _set_disk_controller(self, disk):
         # Add a SCSI controller with model virtio-scsi if needed
         disk.vmm_controller = None
-        if controller_model != "virtio-scsi":
-            return None
+        if not self.vm.xmlobj.can_default_virtioscsi():
+            return
 
-        # Get SCSI controllers
-        controllers = self.vm.xmlobj.devices.controller
-        ctrls_scsi = [x for x in controllers if
-                (x.type == DeviceController.TYPE_SCSI)]
-
-        # Create possible new controller
         controller = DeviceController(self.conn.get_backend())
         controller.type = "scsi"
-        controller.model = controller_model
-
-        # And set its index
+        controller.model = "virtio-scsi"
         controller.index = 0
-        if ctrls_scsi:
-            controller.index = max([x.index for x in ctrls_scsi]) + 1
-
-        # Take only virtio-scsi ones
-        ctrls_scsi = [x for x in ctrls_scsi
-                      if x.model == controller_model]
-
-        # Save occupied places per controller
-        occupied = {}
-        for d in used_disks:
-            if (d.get_target_prefix() == disk.get_target_prefix() and
-                d.bus == "scsi"):
-                num = DeviceDisk.target_to_num(d.target)
-                idx = num // 7
-                if idx not in occupied:
-                    occupied[idx] = []
-                if d.target not in occupied[idx]:
-                    occupied[idx].append(d.target)
-
-        for c in ctrls_scsi:
-            if c.index not in occupied or len(occupied[c.index]) < 7:
-                controller = c
-                break
-        else:
-            disk.vmm_controller = controller
-
-        return controller.index
+        disk.vmm_controller = controller
 
     def _build_storage(self):
         bus = uiutil.get_list_selection(
@@ -1541,13 +1507,6 @@ class vmmAddHardware(vmmGObjectUI):
         detect_zeroes = uiutil.get_list_selection(
             self.widget("storage-detect-zeroes"))
 
-        controller_model = None
-        if (bus == "scsi" and
-            self.vm.get_hv_type() in ["qemu", "kvm", "test"] and
-            not any([c.type == "scsi"
-                 for c in self.vm.xmlobj.devices.controller])):
-            controller_model = "virtio-scsi"
-
         disk = self.addstorage.build_device(self.vm.get_name(),
             collideguest=self.vm.xmlobj, device=device)
 
@@ -1567,10 +1526,8 @@ class vmmAddHardware(vmmGObjectUI):
             if d.target not in used:
                 used.append(d.target)
 
-        prefer_ctrl = self._set_disk_controller(
-            disk, controller_model, disks)
-
-        disk.generate_target(used, prefer_ctrl)
+        self._set_disk_controller(disk)
+        disk.generate_target(used)
         return disk
 
     def _build_network(self):

virtinst/devices/disk.py

@@ -840,16 +840,13 @@ class DeviceDisk(Device):
                     return _return(pref)
         return _return("sd")
 
-    def generate_target(self, skip_targets, pref_ctrl=None):
+    def generate_target(self, skip_targets):
         """
         Generate target device ('hda', 'sdb', etc..) for disk, excluding
-        any targets in 'skip_targets'.  If given the 'pref_ctrl'
-        parameter, it tries to select the target so that the disk is
-        mapped onto that controller.
+        any targets in 'skip_targets'.
         Sets self.target, and returns the generated value.
 
         :param skip_targets: list of targets to exclude
-        :param pref_ctrl: preferred controller to connect the disk to
         :returns: generated target
         """
         prefix, maxnode = self.get_target_prefix(skip_targets)
@@ -859,13 +856,7 @@ class DeviceDisk(Device):
         def get_target():
             first_found = None
 
-            ran = range(maxnode)
-            if pref_ctrl is not None:
-                # We assume narrow SCSI bus and libvirt assigning 7
-                # (1-7, 8-14, etc.) devices per controller
-                ran = range(pref_ctrl * 7, (pref_ctrl + 1) * 7)
-
-            for i in ran:
+            for i in range(maxnode):
                 gen_t = prefix + self.num_to_target(i + 1)
                 if gen_t in skip_targets:
                     skip_targets.remove(gen_t)
@@ -882,11 +873,6 @@ class DeviceDisk(Device):
             self.target = ret
             return ret
 
-        if pref_ctrl is not None:
-            # This basically means that we either chose full
-            # controller or didn't add any
-            raise ValueError(_("Controller number %d for disk of type %s has "
-                               "no empty slot to use" % (pref_ctrl, prefix)))
         raise ValueError(_("Only %s disks for bus '%s' are supported"
                            % (maxnode, self.bus)))
 

virtinst/guest.py

@@ -603,6 +603,14 @@ class Guest(XMLBuilder):
                 return True
         return False
 
+    def can_default_virtioscsi(self):
+        """
+        Return True if the guest supports virtio-scsi, and there's
+        no other scsi controllers attached to the guest
+        """
+        has_any_scsi = any([d.type == "scsi" for d in self.devices.controller])
+        return not has_any_scsi and self.supports_virtioscsi()
+
     def hyperv_supported(self):
         if not self.osinfo.is_windows():
             return False
@@ -953,13 +961,8 @@ class Guest(XMLBuilder):
             self.add_device(dev)
 
     def _add_implied_controllers(self):
-        has_any_scsi = False
-        for dev in self.devices.controller:
-            if dev.type == "scsi":
-                has_any_scsi = True
-
         # Add virtio-scsi controller if needed
-        if not has_any_scsi and self.supports_virtioscsi():
+        if self.can_default_virtioscsi():
             for dev in self.devices.disk:
                 if dev.bus == "scsi":
                     ctrl = DeviceController(self.conn)