From 0bd2f97bdaa3b06bfa4ce94f9a3cb2b5f34c4cae Mon Sep 17 00:00:00 2001
From: Cole Robinson <crobinso@redhat.com>
Date: Thu, 3 Oct 2013 08:40:24 -0400
Subject: [PATCH] VirtualDisk: Try to append targets, not just use first free
 (bz #905439)

This used to happen:

- create VM with cdrom, cdrom gets hdc
- customize before install
- add new cdrom, gets target hda or hdb (first free target)
- new cdrom now has priority in the boot order

Change the logic to try to append targets first, and if there isn't
any space left, use the first free one. This may cause other issues
but we'll just have to wait and see.
---
 tests/xmlconfig.py     |  8 ++++++++
 virtinst/devicedisk.py | 25 ++++++++++++++++++++-----
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/tests/xmlconfig.py b/tests/xmlconfig.py
index f8f2f715e..9861e56fa 100644
--- a/tests/xmlconfig.py
+++ b/tests/xmlconfig.py
@@ -1046,6 +1046,14 @@ class TestXMLConfig(unittest.TestCase):
         self.assertEquals("zz", VirtualDisk.num_to_target(27 * 26))
         self.assertEquals("aaa", VirtualDisk.num_to_target(27 * 26 + 1))
 
+        disk = virtinst.VirtualDisk(utils.get_conn())
+        disk.bus = "ide"
+
+        self.assertEquals("hda", disk.generate_target([]))
+        self.assertEquals("hdb", disk.generate_target(["hda"]))
+        self.assertEquals("hdc", disk.generate_target(["hdb", "sda"]))
+        self.assertEquals("hdb", disk.generate_target(["hda", "hdd"]))
+
     def testFedoraTreeinfo(self):
         i = utils.make_distro_installer(
                                 location="tests/cli-test-xml/fakefedoratree")
diff --git a/virtinst/devicedisk.py b/virtinst/devicedisk.py
index d99aa7041..662f52ede 100644
--- a/virtinst/devicedisk.py
+++ b/virtinst/devicedisk.py
@@ -827,13 +827,28 @@ class VirtualDisk(VirtualDevice):
         @rtype C{str}
         """
         prefix, maxnode = self.get_target_prefix()
+        skip_targets = [t for t in skip_targets if t and t.startswith(prefix)]
+        skip_targets.sort()
 
-        for i in range(1, maxnode + 1):
-            gen_t = prefix + self.num_to_target(i)
-            if gen_t not in skip_targets:
-                self.target = gen_t
-                return self.target
+        def get_target():
+            first_found = None
 
+            for i in range(1, maxnode + 1):
+                gen_t = prefix + self.num_to_target(i)
+                if gen_t in skip_targets:
+                    skip_targets.remove(gen_t)
+                    continue
+                if not skip_targets:
+                    return gen_t
+                elif not first_found:
+                    first_found = gen_t
+            if first_found:
+                return first_found
+
+        ret = get_target()
+        if ret:
+            self.target = ret
+            return ret
         raise ValueError(_("No more space for disks of type '%s'" % prefix))
 
 VirtualDisk.register_type()