From 709b49516cbd94a87c19ce3f93cf49d3f54da0cf Mon Sep 17 00:00:00 2001 From: Sean McGinnis Date: Tue, 18 Dec 2018 00:18:41 +0000 Subject: [PATCH] Revert "Synchronize all LVM operations" This reverts commit 206f980cc2f09610ffb86cfc7728222fd39eee4f. This was part of troubleshooting long LVM operations. The hope is https://review.openstack.org/#/c/625269/ actually fixes the issue since this change, at least by itself, still had failing runs due to lvchange operations taking longer than 60 seconds. Change-Id: I9c3dee2ba94c3779ff43db578c59448acbcf6653 --- cinder/brick/local_dev/lvm.py | 14 -------------- cinder/tests/unit/brick/test_brick_lvm.py | 14 -------------- 2 files changed, 28 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index d520fd437f1..69636bbf445 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -22,7 +22,6 @@ 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 @@ -127,19 +126,6 @@ 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. diff --git a/cinder/tests/unit/brick/test_brick_lvm.py b/cinder/tests/unit/brick/test_brick_lvm.py index 46d976252a8..3a9f1d0f7ae 100644 --- a/cinder/tests/unit/brick/test_brick_lvm.py +++ b/cinder/tests/unit/brick/test_brick_lvm.py @@ -405,20 +405,6 @@ 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))