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: Ifc9dcb20e17c60a835e9f3b38c5bffd836fa5188
Closes-bug: #1495560

cinder/brick/local_dev/lvm.py

@@ -577,6 +577,18 @@ 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()
+            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']
@@ -592,6 +604,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.
 

cinder/exception.py

@@ -821,6 +821,10 @@ class VolumeGroupCreationFailed(CinderException):
     message = _('Failed to create Volume Group: %(vg_name)s')
 
 
+class VolumeNotDeactivated(CinderException):
+    message = _('Volume %(name)s was not deactivated in time.')
+
+
 class VolumeDeviceNotFound(CinderException):
     message = _('Volume device not found at %(device)s.')
 

cinder/tests/unit/brick/test_brick_lvm.py

@@ -372,3 +372,20 @@ class BrickLvmTestCase(test.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')