Remove race condition from lvextend

Currently it's possible for extend_volume in lvm to return
from the deactivate_lv call and try to extend the volume before
the lv has actually been deactivated. This patch adds logic to
make sure that the lv is deactivated before returning from
deactivate_lv.

Change-Id: I5c3671043df6e7474acdfcce342d655ac215a461
Closes-bug: #1495560

os_brick/exception.py

@@ -94,6 +94,10 @@ class NoFibreChannelVolumeDeviceFound(BrickException):
     message = _("Unable to find a Fibre Channel volume device.")
 
 
+class VolumeNotDeactivated(BrickException):
+    message = _('Volume %(name)s was not deactivated in time.')
+
+
 class VolumeDeviceNotFound(BrickException):
     message = _("Volume device not found at %(device)s.")
 

os_brick/local_dev/lvm.py

@@ -616,6 +616,20 @@ class LVM(executor.Executor):
             return name
         return '_' + name
 
+    def _lv_is_active(self, name):
+        cmd = LVM.LVM_CMD_PREFIX + ['lvdisplay', '--noheading', '-C', '-o',
+                                    'Attr', '%s/%s' % (self.vg_name, name)]
+        out, _err = self._execute(*cmd,
+                                  root_helper=self._root_helper,
+                                  run_as_root=True)
+        if out:
+            out = out.strip()
+            # An example output might be '-wi-a----'; the 4th index specifies
+            # the status of the volume. 'a' for active, '-' for inactive.
+            if (out[4] == 'a'):
+                return True
+        return False
+
     def deactivate_lv(self, name):
         lv_path = self.vg_name + '/' + self._mangle_lv_name(name)
         cmd = ['lvchange', '-a', 'n']
@@ -631,6 +645,21 @@ class LVM(executor.Executor):
             LOG.error(_LE('StdErr  :%s'), err.stderr)
             raise
 
+        # Wait until lv is deactivated to return in
+        # order to prevent a race condition.
+        self._wait_for_volume_deactivation(name)
+
+    @utils.retry(exceptions=exception.VolumeNotDeactivated, retries=3,
+                 backoff_rate=1)
+    def _wait_for_volume_deactivation(self, name):
+        LOG.debug("Checking to see if volume %s has been deactivated.",
+                  name)
+        if self._lv_is_active(name):
+            LOG.debug("Volume %s is still active.", name)
+            raise exception.VolumeNotDeactivated(name=name)
+        else:
+            LOG.debug("Volume %s has been deactivated.", name)
+
     def activate_lv(self, name, is_snapshot=False, permanent=False):
         """Ensure that logical volume/snapshot logical volume is activated.
 

os_brick/tests/local_dev/test_brick_lvm.py

@@ -364,3 +364,29 @@ class BrickLvmTestCase(base.TestCase):
         self.vg.vg_name = "test-volumes"
         self.vg.extend_volume("test", "2G")
         self.assertFalse(self.vg.deactivate_lv.called)
+
+    def test_lv_deactivate(self):
+        with mock.patch.object(self.vg, '_execute'):
+            is_active_mock = mock.Mock()
+            is_active_mock.return_value = False
+            self.vg._lv_is_active = is_active_mock
+            self.vg.create_volume('test', '1G')
+            self.vg.deactivate_lv('test')
+
+    def test_lv_deactivate_timeout(self):
+        with mock.patch.object(self.vg, '_execute'):
+            is_active_mock = mock.Mock()
+            is_active_mock.return_value = True
+            self.vg._lv_is_active = is_active_mock
+            self.vg.create_volume('test', '1G')
+            self.assertRaises(exception.VolumeNotDeactivated,
+                              self.vg.deactivate_lv, 'test')
+
+    def test_lv_is_active(self):
+        self.vg.create_volume('test', '1G')
+        with mock.patch.object(self.vg, '_execute',
+                               return_value=['owi-a---', '']):
+            self.assertTrue(self.vg._lv_is_active('test'))
+        with mock.patch.object(self.vg, '_execute',
+                               return_value=['owi-----', '']):
+            self.assertFalse(self.vg._lv_is_active('test'))