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>

nova/tests/unit/virt/libvirt/test_driver.py

@@ -5504,6 +5504,52 @@ def _test_get_serial_ports_from_guest(self, mode, mock_get_xml_desc,
         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',

nova/virt/libvirt/driver.py

@@ -1402,16 +1402,16 @@ def _get_serial_ports_from_guest(self, guest, mode=None):
         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
 
     def _cleanup_rbd(self, instance):
         # NOTE(nic): On revert_resize, the cleanup steps for the root
@@ -1725,7 +1725,7 @@ def attach_volume(self, context, connection_info, instance, mountpoint,
         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)