From c74efd7765e33379c50056359123eb6f00fedb07 Mon Sep 17 00:00:00 2001
From: Dirk Mueller <dirk@dmllr.de>
Date: Fri, 13 Jun 2014 00:24:23 +0200
Subject: [PATCH] Retry lvremove with ignore_suspended_devices

A lvremove -f might leave behind suspended devices
when it is racing with udev or other processes
still accessing any of the device files. The previous
solution of using lvchange -an on the LV had the
side-effect of deactivating origin LVs alongway in
the thick volume case, which was undesired.

It turns out retrying the deactivation twice and
ignoring the suspended devices on the second iteration
avoids the hang of all LVM operations after an initial
failure.

Change-Id: I0d6fb74084d049ea184e68f2dcc4e74f400b7dbd
Closes-Bug: #1317075
Related-Bug: #1270192
(cherry picked from commit da9597aed0186e68dbf1c7304b30e49f8e6a54ff)
---
 cinder/brick/local_dev/lvm.py | 50 +++++++++++++++--------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index e8d4e88a943..b7fc057f3b7 100644
--- a/cinder/brick/local_dev/lvm.py
+++ b/cinder/brick/local_dev/lvm.py
@@ -565,31 +565,17 @@ class LVM(executor.Executor):
                           root_helper=self._root_helper, run_as_root=True,
                           check_exit_code=False)
 
+        # LV removal seems to be a race with other writers or udev in
+        # some cases (see LP #1270192), so we enable retry deactivation
+        LVM_CONFIG = 'activation { retry_deactivation = 1} '
+
         try:
-            # LV removal seems to be a race with udev in
-            # some cases (see LP #1270192), so we do it in several steps:
-
-            # - Deactivate the LV/Snapshot, which triggers udev events
-            # - Wait for udev to finish its job with udevadmn settle
-            # - Remove the LV
-
-            try:
-                self._execute('lvchange', '-y', '-an',
-                              '%s/%s' % (self.vg_name, name),
-                              root_helper=self._root_helper, run_as_root=True)
-            except putils.ProcessExecutionError as err:
-                mesg = (_('Error during lvchange -an: CMD: %(command)s, '
-                        'RESPONSE: %(response)s') %
-                        {'command': err.cmd, 'response': err.stderr})
-                LOG.debug(mesg)
-
-            run_udevadm_settle()
-
-            # Note(jdg): use -f for bug #1336811
-            cmd = ['lvremove', '-f']
-            cmd.append('%s/%s' % (self.vg_name, name))
-            self._execute(*cmd,
-                          root_helper=self._root_helper, run_as_root=True)
+            self._execute(
+                'lvremove',
+                '--config', LVM_CONFIG,
+                '-f',
+                '%s/%s' % (self.vg_name, name),
+                root_helper=self._root_helper, run_as_root=True)
         except putils.ProcessExecutionError as err:
             mesg = (_('Error reported running lvremove: CMD: %(command)s, '
                     'RESPONSE: %(response)s') %
@@ -599,10 +585,18 @@ class LVM(executor.Executor):
             LOG.debug(_('Attempting udev settle and retry of lvremove...'))
             run_udevadm_settle()
 
-            self._execute('lvremove',
-                          '-f',
-                          '%s/%s' % (self.vg_name, name),
-                          root_helper=self._root_helper, run_as_root=True)
+            # The previous failing lvremove -f might leave behind
+            # suspended devices; when lvmetad is not available, any
+            # further lvm command will block forever.
+            # Therefore we need to skip suspended devices on retry.
+            LVM_CONFIG += 'devices { ignore_suspended_devices = 1}'
+
+            self._execute(
+                'lvremove',
+                '--config', LVM_CONFIG,
+                '-f',
+                '%s/%s' % (self.vg_name, name),
+                root_helper=self._root_helper, run_as_root=True)
 
     def revert(self, snapshot_name):
         """Revert an LV from snapshot.