Detach volume fails when using multipath iscsi

Enabled multipath for libvirt iscsi volume driver. Attach volume worked,
however, detach volume failed. It failed because it can't find multipath
device "/dev/mapper/xxxxxxxx" from connection_info and block device
mapping table.

The fix is to retrieve multipath device info dynamically at detach time.

Fixes: bug #1178893

Change-Id: I53a3888daef93b57bf7f57438f74cee05e794b52

2 files changed, 71 insertions, 23 deletions

diff --git a/nova/tests/test_libvirt_volume.py b/nova/tests/test_libvirt_volume.py
index 1186ed268..6d2dc9dce 100644
--- a/nova/tests/test_libvirt_volume.py
+++ b/nova/tests/test_libvirt_volume.py
@@ -389,6 +389,36 @@ class LibvirtVolumeTestCase(test.TestCase):
         self.assertEqual(tree.find('./source').get('dev'), mpdev_filepath)
         libvirt_driver.disconnect_volume(connection_info, 'vde')
 
+    def test_libvirt_kvm_volume_with_multipath_getmpdev(self):
+        self.flags(libvirt_iscsi_use_multipath=True)
+        self.stubs.Set(os.path, 'exists', lambda x: True)
+        libvirt_driver = volume.LibvirtISCSIVolumeDriver(self.fake_conn)
+        name0 = 'volume-00000000'
+        location0 = '10.0.2.15:3260'
+        iqn0 = 'iqn.2010-10.org.openstack:%s' % name0
+        vol0 = {'id': 0, 'name': name0}
+        dev0 = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-0' % (location0, iqn0)
+        name = 'volume-00000001'
+        location = '10.0.2.15:3260'
+        iqn = 'iqn.2010-10.org.openstack:%s' % name
+        vol = {'id': 1, 'name': name}
+        dev = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location, iqn)
+        devs = [dev0, dev]
+        self.stubs.Set(self.fake_conn, 'get_all_block_devices', lambda: devs)
+        connection_info = self.iscsi_connection(vol, location, iqn)
+        mpdev_filepath = '/dev/mapper/foo'
+        disk_info = {
+            "bus": "virtio",
+            "dev": "vde",
+            "type": "disk",
+            }
+        target_portals = ['fake_portal1', 'fake_portal2']
+        libvirt_driver._get_multipath_device_name = lambda x: mpdev_filepath
+        conf = libvirt_driver.connect_volume(connection_info, disk_info)
+        tree = conf.format_dom()
+        self.assertEqual(tree.find('./source').get('dev'), mpdev_filepath)
+        libvirt_driver.disconnect_volume(connection_info, 'vde')
+
     def test_libvirt_nfs_driver(self):
         # NOTE(vish) exists is to make driver assume connecting worked
         mnt_base = '/mnt'
diff --git a/nova/virt/libvirt/volume.py b/nova/virt/libvirt/volume.py
index 5f8d4bd72..1415da9bf 100644
--- a/nova/virt/libvirt/volume.py
+++ b/nova/virt/libvirt/volume.py
@@ -269,32 +269,20 @@ class LibvirtISCSIVolumeDriver(LibvirtBaseVolumeDriver):
     @lockutils.synchronized('connect_volume', 'nova-')
     def disconnect_volume(self, connection_info, disk_dev):
         """Detach the volume from instance_name."""
+        iscsi_properties = connection_info['data']
+        multipath_device = None
+        if CONF.libvirt_iscsi_use_multipath:
+            host_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-%s" %
+                           (iscsi_properties['target_portal'],
+                            iscsi_properties['target_iqn'],
+                            iscsi_properties.get('target_lun', 0)))
+            multipath_device = self._get_multipath_device_name(host_device)
+
         super(LibvirtISCSIVolumeDriver,
               self).disconnect_volume(connection_info, disk_dev)
-        iscsi_properties = connection_info['data']
 
-        if CONF.libvirt_iscsi_use_multipath and \
-           "mapper" in connection_info['data']['device_path']:
-            self._rescan_iscsi()
-            self._rescan_multipath()
-            devices = [dev for dev in self.connection.get_all_block_devices()
-                       if "/mapper/" in dev]
-            if not devices:
-                #disconnect if no other multipath devices
-                self._disconnect_mpath(iscsi_properties)
-                return
-
-            other_iqns = [self._get_multipath_iqn(device)
-                          for device in devices]
-
-            if iscsi_properties['target_iqn'] not in other_iqns:
-                #disconnect if no other multipath devices with same iqn
-                self._disconnect_mpath(iscsi_properties)
-                return
-
-            #else do not disconnect iscsi portals,
-            #as they are used for other luns
-            return
+        if CONF.libvirt_iscsi_use_multipath and multipath_device:
+            return self._disconnect_volume_multipath_iscsi(iscsi_properties)
 
         # NOTE(vish): Only disconnect from the target if no luns from the
         #             target are in use.
@@ -306,6 +294,36 @@ class LibvirtISCSIVolumeDriver(LibvirtBaseVolumeDriver):
         if not devices:
             self._disconnect_from_iscsi_portal(iscsi_properties)
 
+    def _disconnect_volume_multipath_iscsi(self, iscsi_properties):
+        self._rescan_iscsi()
+        self._rescan_multipath()
+        block_devices = self.connection.get_all_block_devices()
+        devices = []
+        for dev in block_devices:
+            if "/mapper/" in dev:
+                devices.append(dev)
+            else:
+                mpdev = self._get_multipath_device_name(dev)
+                if mpdev:
+                    devices.append(mpdev)
+
+        if not devices:
+            # disconnect if no other multipath devices
+            self._disconnect_mpath(iscsi_properties)
+            return
+
+        other_iqns = [self._get_multipath_iqn(device)
+                      for device in devices]
+
+        if iscsi_properties['target_iqn'] not in other_iqns:
+            # disconnect if no other multipath devices with same iqn
+            self._disconnect_mpath(iscsi_properties)
+            return
+
+        # else do not disconnect iscsi portals,
+        # as they are used for other luns
+        return
+
     def _connect_to_iscsi_portal(self, iscsi_properties):
         # NOTE(vish): If we are on the same host as nova volume, the
         #             discovery makes the target so we don't need to