From de257d1a2b91e8060ff3532ced25cb2a67b14267 Mon Sep 17 00:00:00 2001 From: John Griffith Date: Fri, 20 Sep 2013 20:55:46 +0000 Subject: [PATCH] Fix issues with failed lvremove There are some race conditions that can cause problems with lvremove commands. In most cases these seem to recover nicely just with a simple retry of the lvremove. Adding a udev settle seems to elimate the rest of them. This is a difficult issue to reproduce, and there's a suspiscion that it relates to failed target creeates. The patch adds a catch on the lvremove failure, followed by a udevadm settle and a retry of the lvremove. With the setup that I've been able to reproduce this issue these changes have eliminated any force delete failures. The other option that had been proposed was using dmsetup remove but there are concerns that this may cause problems. Change-Id: I2a2b0d0f4fefd0daf9424ab96aaf87ba53ebc171 Closes-Bug: #1191960 --- cinder/brick/local_dev/lvm.py | 24 ++++++++++++++++++++---- cinder/volume/drivers/lvm.py | 7 +++++++ etc/cinder/rootwrap.d/volume.filters | 3 ++- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index bee80ba20..3c0338f5a 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -408,10 +408,26 @@ class LVM(executor.Executor): :param name: Name of LV to delete """ - self._execute('lvremove', - '-f', - '%s/%s' % (self.vg_name, name), - root_helper=self._root_helper, run_as_root=True) + try: + self._execute('lvremove', + '-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') % + {'command': err.cmd, 'response': err.stderr}) + LOG.error(mesg) + + LOG.warning(_('Attempting udev settle and retry of lvremove...')) + self._execute('udevadm', 'settle', + root_helper=self._root_helper, + run_as_root=True) + + self._execute('lvremove', + '-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. diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index fc3b10ccb..85a0f60e5 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -179,11 +179,18 @@ class LVMVolumeDriver(driver.VolumeDriver): def delete_volume(self, volume): """Deletes a logical volume.""" + + # NOTE(jdg): We don't need to explicitly call + # remove export here because we already did it + # in the manager before we got here. + if self._volume_not_present(volume['name']): # If the volume isn't present, then don't attempt to delete return True if self.vg.lv_has_snapshot(volume['name']): + LOG.error(_('Unabled to delete due to existing snapshot ' + 'for volume: %s') % volume['name']) raise exception.VolumeIsBusy(volume_name=volume['name']) self._delete_volume(volume) diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index 99f75ed6c..e5b7ab00e 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -43,6 +43,7 @@ dmsetup: CommandFilter, dmsetup, root ln: CommandFilter, ln, root qemu-img: CommandFilter, qemu-img, root env: CommandFilter, env, root +udevadm: CommandFilter, udevadm, root # cinder/volume/driver.py: utils.read_file_as_root() cat: CommandFilter, cat, root @@ -61,7 +62,7 @@ find: CommandFilter, find, root # cinder/volume/drivers/glusterfs.py mv: CommandFilter, mv, root -# cinder/volumes/drivers/hds/hds.py: +# cinder/volumes/drivers/hds/hds.py: hus-cmd: CommandFilter, hus-cmd, root # cinder/brick/initiator/connector.py: -- 2.45.2