Synchronize all LVM operations

This makes us synchronize/serialize all of our external calls to LVM
operations in an attempt to avoid contention of LVM's own locking
scheme, as well as avoid potentially overlapping operations that may
be flushing large amounts of data to disk.

Related-Bug: #1739482
Related-Bug: #1763712
Change-Id: Idb4053685486f2163e7cb135cd579eba5d78f369

cinder/brick/local_dev/lvm.py

@@ -22,6 +22,7 @@ import os
 import re
 
 from os_brick import executor
+from oslo_concurrency import lockutils
 from oslo_concurrency import processutils as putils
 from oslo_log import log as logging
 from oslo_utils import excutils
@@ -126,6 +127,19 @@ class LVM(executor.Executor):
             self.activate_lv(self.vg_thin_pool)
         self.pv_list = self.get_all_physical_volumes(root_helper, vg_name)
 
+    @lockutils.synchronized('cinder-lvm-exec', external=True)
+    def _execute(self, *args, **kwargs):
+        """Wrap _execute() with a lock.
+
+        We want to make sure we are only doing one LVM operation at a time
+        to avoid contention with the LVM locking system as well as other
+        potential places where doing these operations in parallel has shown
+        to cause spurious lockups and hangs. So, we wrap
+        Executor._execute() with this synchronized decorator to ensure
+        serialization.
+        """
+        return super(LVM, self)._execute(*args, **kwargs)
+
     def _vg_exists(self):
         """Simple check to see if VG exists.
 

cinder/tests/unit/brick/test_brick_lvm.py

@@ -405,6 +405,20 @@ class BrickLvmTestCase(test.TestCase):
 
             self.vg.activate_lv('my-lv')
 
+    @mock.patch('oslo_concurrency.lockutils.lock')
+    def test_activate_lv_execute_with_lock(self, mock_lock):
+        with mock.patch('os_brick.executor.Executor._execute') as mock_ex:
+            self.vg.activate_lv('my-lv')
+            mock_ex.assert_called()
+
+        # NOTE(danms): This is a little icky because it assumes internal
+        # behavior of oslo_concurrency, but we need to make sure that
+        # the decorator (which has already run here) is wrapping our
+        # _execute() method. It calls lock() on __enter__, so we use that
+        # here to validate that we are synchronized.
+        mock_lock.assert_called()
+        mock_lock.return_value.__enter__.assert_called_once_with()
+
     def test_get_mirrored_available_capacity(self):
         self.assertEqual(2.0, self.vg.vg_mirror_free_space(1))