]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Retry lvremove with ignore_suspended_devices
authorDirk Mueller <dirk@dmllr.de>
Thu, 12 Jun 2014 22:24:23 +0000 (00:24 +0200)
committerDirk Mueller <dirk@dmllr.de>
Fri, 13 Jun 2014 08:42:59 +0000 (10:42 +0200)
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

cinder/brick/local_dev/lvm.py

index 0cca76af0f4b51b99c81d46eb6f4a88340fd661e..b7fc057f3b71c053da384044888cc697c7678fa9 100644 (file)
@@ -565,36 +565,17 @@ class LVM(executor.Executor):
                           root_helper=self._root_helper, run_as_root=True,
                           check_exit_code=False)
 
-        try:
-            need_force_remove = False
-            # 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)
-                need_force_remove = True
+        # 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} '
 
-            run_udevadm_settle()
-
-            cmd = ['lvremove', ]
-
-            # if deactivation failed, use the --force, lvm!
-            if need_force_remove:
-                cmd.append('-f')
-            cmd.append('%s/%s' % (self.vg_name, name))
-            self._execute(*cmd,
-                          root_helper=self._root_helper, run_as_root=True)
+        try:
+            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') %
@@ -604,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.