]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Fix race condition in ISCSIConnector _disconnect_volume_multipath_iscsi
authorPatrick East <patrick.east@purestorage.com>
Tue, 30 Sep 2014 18:47:42 +0000 (11:47 -0700)
committerThierry Carrez <thierry@openstack.org>
Wed, 8 Oct 2014 10:28:28 +0000 (12:28 +0200)
This is a similar issue as seen in
https://bugs.launchpad.net/cinder/+bug/1375382

The list of devices returned by driver.get_all_block_devices() in
_disconnect_volume_multipath_iscsi will potentially contain broken
symlinks as the SCSI devices have been deleted from calling
self._linuxscsi.remove_multipath_device(device_realpath) right before
_disconnect_volume_multipath_iscsi but the udev rule for the symlink
may not yet have completed.

Adding in a check to os.path.exists() will ensure that we will not
consider the broken symlinks as an “in use” device.

Change-Id: I79c9627e9b47127d3765fcec5b7e3bacef179630
Closes-Bug: #1375946
(cherry picked from commit 4541521de576297d9b7d4115b040ff54773d9d50)

cinder/brick/initiator/connector.py
cinder/tests/brick/test_brick_connector.py

index fb7277791270478bd780f06b435c93544e3eb5e0..a51735e7c271d1f2ba20a62a2724f87659d4e4ae 100644 (file)
@@ -351,12 +351,13 @@ class ISCSIConnector(InitiatorConnector):
         block_devices = self.driver.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 os.path.exists(dev):
+                if "/mapper/" in dev:
+                    devices.append(dev)
+                else:
+                    mpdev = self._get_multipath_device_name(dev)
+                    if mpdev:
+                        devices.append(mpdev)
 
         # Do a discovery to find all targets.
         # Targets for multiple paths for the same multipath device
index f2ff6caa11087cd7cd7d9022f1ba0b2bc0362c16..db87184a78d0d94e01d2f97626d2ec382bc84ea1 100644 (file)
@@ -302,6 +302,7 @@ class ISCSIConnectorTestCase(ConnectorTestCase):
                        lambda x: iqns.pop())
         self.stubs.Set(self.connector, '_disconnect_from_iscsi_portal',
                        fake_disconnect_from_iscsi_portal)
+        self.stubs.Set(os.path, 'exists', lambda x: True)
         fake_property = {'target_portal': portal,
                          'target_iqn': iqn1}
         self.connector._disconnect_volume_multipath_iscsi(fake_property,
@@ -326,6 +327,35 @@ class ISCSIConnectorTestCase(ConnectorTestCase):
                        lambda: [])
         self.stubs.Set(self.connector, '_disconnect_from_iscsi_portal',
                        fake_disconnect_from_iscsi_portal)
+        self.stubs.Set(os.path, 'exists', lambda x: True)
+        fake_property = {'target_portal': portal,
+                         'target_iqn': iqn}
+        self.connector._disconnect_volume_multipath_iscsi(fake_property,
+                                                          'fake/multipath')
+        # Target not in use by other mp devices, disconnect
+        self.assertEqual([fake_property], result)
+
+    def test_disconnect_volume_multipath_iscsi_with_invalid_symlink(self):
+        result = []
+
+        def fake_disconnect_from_iscsi_portal(properties):
+            result.append(properties)
+
+        portal = '10.0.0.1:3260'
+        name = 'volume-00000001'
+        iqn = 'iqn.2010-10.org.openstack:%s' % name
+        dev = ('ip-%s-iscsi-%s-lun-0' % (portal, iqn))
+        self.stubs.Set(self.connector,
+                       '_get_target_portals_from_iscsiadm_output',
+                       lambda x: [[portal, iqn]])
+        self.stubs.Set(self.connector, '_rescan_iscsi', lambda: None)
+        self.stubs.Set(self.connector, '_rescan_multipath', lambda: None)
+        self.stubs.Set(self.connector.driver, 'get_all_block_devices',
+                       lambda: [dev, '/dev/mapper/md-1'])
+        self.stubs.Set(self.connector, '_disconnect_from_iscsi_portal',
+                       fake_disconnect_from_iscsi_portal)
+        # Simulate a broken symlink by returning False for os.path.exists(dev)
+        self.stubs.Set(os.path, 'exists', lambda x: False)
         fake_property = {'target_portal': portal,
                          'target_iqn': iqn}
         self.connector._disconnect_volume_multipath_iscsi(fake_property,