From 4dc367f7d736ae506db9532270e92977c8b3954d Mon Sep 17 00:00:00 2001 From: John Griffith Date: Thu, 15 Jan 2015 08:56:28 -0700 Subject: [PATCH] Add retry for tgtadm update when tgt exists For target creation using tgtadm driver, we create a persistence file for the target, then send a tgt-admin --update 'name' where name is the specific persistence file we want to read in and update from. It turns out that we can hit race conditions where the persistence file is written, and an update is requested but target has already done work to make it believe that the target has already been created. One thought was to just use "update ALL" but this still seems to have issues, and changes the error to an account exists failure. This patch takes the brute force approach and adds the cinder.utils retry decorator to the tgt-admin --update command. To do this we just break out the tgt-admin --update call into it's own method and add the decorator to it. Closes-Bug: #1398078 Change-Id: Ic72fb5f4fca70a7e7d1c306efe19674fd7e42cd6 --- cinder/volume/targets/tgt.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py index 0704a8d52..4cae868e8 100644 --- a/cinder/volume/targets/tgt.py +++ b/cinder/volume/targets/tgt.py @@ -147,6 +147,13 @@ class TgtAdm(iscsi.ISCSITarget): LOG.debug('Failed to find CHAP auth from config for %s' % vol_id) return None + @utils.retry(putils.ProcessExecutionError) + def _do_tgt_update(self, name): + (out, err) = utils.execute('tgt-admin', '--update', name, + run_as_root=True) + LOG.debug("StdOut from tgt-admin --update: %s", out) + LOG.debug("StdErr from tgt-admin --update: %s", err) + def ensure_export(self, context, volume, volume_path): chap_auth = None old_name = None @@ -222,10 +229,8 @@ class TgtAdm(iscsi.ISCSITarget): # by creating the entry in the persist file # and then doing an update to get the target # created. - (out, err) = utils.execute('tgt-admin', '--update', name, - run_as_root=True) - LOG.debug("StdOut from tgt-admin --update: %s", out) - LOG.debug("StdErr from tgt-admin --update: %s", err) + + self._do_tgt_update(name) except putils.ProcessExecutionError as e: if "target already exists" in e.stderr: # Adding the additional Warning message below for a clear @@ -233,15 +238,14 @@ class TgtAdm(iscsi.ISCSITarget): LOG.warning(_LW('Could not create target because ' 'it already exists for volume: %s'), vol_id) LOG.debug('Exception was: %s', e) - pass - else: - LOG.error(_LE("Failed to create iscsi target for volume " - "id:%(vol_id)s: %(e)s"), - {'vol_id': vol_id, 'e': e}) - # Don't forget to remove the persistent file we created - os.unlink(volume_path) - raise exception.ISCSITargetCreateFailed(volume_id=vol_id) + LOG.error(_LE("Failed to create iscsi target for volume " + "id:%(vol_id)s: %(e)s"), + {'vol_id': vol_id, 'e': e}) + + # Don't forget to remove the persistent file we created + os.unlink(volume_path) + raise exception.ISCSITargetCreateFailed(volume_id=vol_id) # Grab targets list for debug # Consider adding a check for lun 0 and 1 for tgtadm -- 2.45.2