]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Check for silent failure of tgtadm remove
authorJohn Griffith <john.griffith@solidfire.com>
Thu, 10 Apr 2014 00:10:08 +0000 (00:10 +0000)
committerjohn-griffith <john.griffith@solidfire.com>
Mon, 14 Apr 2014 19:24:14 +0000 (13:24 -0600)
In order to work around a failure to remove targets that
had a reconnect the force flag was added to the tgt delete cmd.
    (https://bugs.launchpad.net/cinder/+bug/1159948)

It turns out there's a bug in tgt where some versions will
sometimes silently fail when using the force flag.
    (https://bugs.launchpad.net/ubuntu/+source/tgt/+bug/1305343)

The problem is that in the gates since we merged the force change
there's a very high number of cases where lvremove fails because
the device is still active.  It appears that this is a result of
the silent force failures in target.

This patch adds a simple check after the force target removal,
if it detects the target is still present it reverts back to the old
non-force method to catch the cases that don't actually need the
force option.

This is a work-around until the version of target in the distros
is updated and can be used reliably, but closes the gate issue
bug that was reported.

Change-Id: I9150669040815e4831bd570964d12676b83ecbc9
Close-Bug: #1304122
(cherry picked from commit f9519182f4a6e6573513d8dbc9438702fc7b8644)

cinder/brick/iscsi/iscsi.py
cinder/tests/test_iscsi.py

index 754b9e1220de63c2de3bf41ad7ac52d278c2ff6f..b28c83f22348d70660950fd3d4ecc91659bc0fae 100644 (file)
@@ -276,6 +276,30 @@ class TgtAdm(TargetAdmin):
                       % {'vol_id': vol_id, 'e': e})
             raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
 
+        # NOTE(jdg): There's a bug in some versions of tgt that
+        # will sometimes fail silently when using the force flag
+        #    https://bugs.launchpad.net/ubuntu/+source/tgt/+bug/1305343
+        # For now work-around by checking if the target was deleted,
+        # if it wasn't, try again without the force.
+
+        # This will NOT do any good for the case of mutliple sessions
+        # which the force was aded for but it will however address
+        # the cases pointed out in bug:
+        #    https://bugs.launchpad.net/cinder/+bug/1304122
+        if self._get_target(iqn):
+            try:
+                LOG.warning(_('Silent failure of target removal '
+                              'detected, retry....'))
+                self._execute('tgt-admin',
+                              '--delete',
+                              iqn,
+                              run_as_root=True)
+            except putils.ProcessExecutionError as e:
+                LOG.error(_("Failed to remove iscsi target for volume "
+                            "id:%(vol_id)s: %(e)s")
+                          % {'vol_id': vol_id, 'e': e})
+                raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
+
         # NOTE(jdg): This *should* be there still but incase
         # it's not we don't care, so just ignore it if was
         # somehow deleted between entry of this method
index 87bb1c6e096c798ca45ba8d1b637475bcf551d88..d24989be203f7d992c14034989b3b91fe25acbfe 100644 (file)
@@ -114,6 +114,7 @@ class TgtAdmTestCase(test.TestCase, TargetAdminTestCase):
         self.flags(volumes_dir=self.persist_tempdir)
         self.script_template = "\n".join([
             'tgt-admin --update %(target_name)s',
+            'tgt-admin --delete %(target_name)s',
             'tgt-admin --force '
             '--delete %(target_name)s',
             'tgtadm --lld iscsi --op show --mode target'])