From 86328d11468af66d95587b53ce28f65ed92c46d7 Mon Sep 17 00:00:00 2001
From: Kevin Zhao <kevin.zhao@linaro.org>
Date: Thu, 12 Mar 2020 11:31:33 +0800
Subject: [PATCH] fix scsi disk unit number of the attaching volume when cdrom
 bus is scsi

From Image Meta Properties: hw_cdrom_bus=scsi, and use virtio-scsi mode,
it will also need a disk address for it. So we need to calculate the
disk address when call the function to get the next unit of scsi controller.

Closes-Bug: #1867075
Change-Id: Ifd8b249de3e8f96fa13db252f0abe2b1bd950de0
Signed-off-by: Kevin Zhao <kevin.zhao@linaro.org>
(cherry picked from commit c8d6767cf8baaf3cc81496c83db10c8ae72fce06)
(cherry picked from commit 11b2b7f0b3a8c09216cd8ebfea8b4cd059605290)
---
 nova/tests/unit/virt/libvirt/test_driver.py | 46 +++++++++++++++++++++
 nova/virt/libvirt/driver.py                 | 10 ++---
 2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 1b0fb66c0581..304ac6e8b1ca 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -4893,6 +4893,52 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         guest = libvirt_guest.Guest(FakeVirtDomain())
         return drvr._get_serial_ports_from_guest(guest, mode=mode)
 
+    def test_get_scsi_controller_next_unit_from_guest(self):
+        xml = """
+    <domain type='kvm'>
+      <devices>
+        <disk type='file' device='disk'>
+          <target dev='sda' bus='scsi'/>
+          <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+        </disk>
+        <disk type='file' device='disk'>
+          <target dev='vda' bus='virtio'/>
+        </disk>
+        <disk type='file' device='cdrom'>
+          <target dev='sdc' bus='scsi'/>
+          <address type='drive' controller='0' bus='0' target='0' unit='1'/>
+        </disk>
+      </devices>
+    </domain>
+            """
+        self._test_get_scsi_controller_next_unit_from_guest(xml, 2)
+
+    def test_get_scsi_controller_next_unit_from_guest_no_scsi(self):
+        xml = """
+    <domain type='kvm'>
+      <devices>
+        <disk type='file' device='disk'>
+          <target dev='vda' bus='virtio'/>
+        </disk>
+        <disk type='file' device='disk'>
+          <target dev='vdb' bus='virtio'/>
+        </disk>
+      </devices>
+    </domain>
+            """
+        self._test_get_scsi_controller_next_unit_from_guest(xml, 0)
+
+    @mock.patch.object(libvirt_guest.Guest, "get_xml_desc")
+    def _test_get_scsi_controller_next_unit_from_guest(self, xml,
+                                                       expect_num,
+                                                       mock_get_xml_desc):
+        mock_get_xml_desc.return_value = xml
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        guest = libvirt_guest.Guest(FakeVirtDomain())
+        i = drvr._get_scsi_controller_next_unit(guest)
+        self.assertEqual(expect_num, i)
+
     def test_get_guest_config_with_type_xen(self):
         self.flags(enabled=True, group='vnc')
         self.flags(virt_type='xen',
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 741a6cb12ec8..aa5cb89f088f 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1197,16 +1197,16 @@ class LibvirtDriver(driver.ComputeDriver):
         for source in tcp_devices:
             yield (source.get("host"), int(source.get("service")))
 
-    def _get_scsi_controller_max_unit(self, guest):
+    def _get_scsi_controller_next_unit(self, guest):
         """Returns the max disk unit used by scsi controller"""
         xml = guest.get_xml_desc()
         tree = etree.fromstring(xml)
-        addrs = "./devices/disk[@device='disk']/address[@type='drive']"
+        addrs = "./devices/disk[target/@bus='scsi']/address[@type='drive']"
 
         ret = []
-        for obj in tree.findall(addrs):
+        for obj in tree.xpath(addrs):
             ret.append(int(obj.get('unit', 0)))
-        return max(ret)
+        return max(ret) + 1 if ret else 0
 
     @staticmethod
     def _get_rbd_driver():
@@ -1528,7 +1528,7 @@ class LibvirtDriver(driver.ComputeDriver):
         disk_info = blockinfo.get_info_from_bdm(
             instance, CONF.libvirt.virt_type, instance.image_meta, bdm)
         if disk_info['bus'] == 'scsi':
-            disk_info['unit'] = self._get_scsi_controller_max_unit(guest) + 1
+            disk_info['unit'] = self._get_scsi_controller_next_unit(guest)
 
         conf = self._get_volume_config(connection_info, disk_info)