From da9597aed0186e68dbf1c7304b30e49f8e6a54ff Mon Sep 17 00:00:00 2001 From: Dirk Mueller 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 --- cinder/brick/local_dev/lvm.py | 55 ++++++++++++++--------------------- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 0cca76af0..b7fc057f3 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -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. -- 2.45.2