From 90f200682c94b1e4b69e04befc045e7ae9111cf7 Mon Sep 17 00:00:00 2001
From: John Griffith <john.griffith@solidfire.com>
Date: Thu, 19 Sep 2013 20:47:22 -0600
Subject: [PATCH] Add a retry to create_iscsi_target for LVM

There's a bug in the tgt driver where under certain
conditions where a race condition is present that can
result in tgtadm attempting re-use a target ID.

Testing this it seems that a retry will address this nicely
and cause tgtd to grab a new target ID and things are ok.

This patch adds a simple check/retry mechanism around the
iscsi_target_create in the LVM driver to catch this issue and
deal with it.

There are a number of bugs that it appears can be attributed to
this, but this patch is associated with the most predominant
clear cut version.

Closes-Bug #1223469

Change-Id: I5126009f196adcafad55e73ff99a59262dd93dfe
---
 cinder/volume/drivers/lvm.py | 61 +++++++++++++++++++++++++++---------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index fc3b10ccb..5a696b7ef 100644
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -400,6 +400,39 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver):
         super(LVMISCSIDriver, self).set_execute(execute)
         self.tgtadm.set_execute(execute)
 
+    def _create_tgtadm_target(self, iscsi_name, iscsi_target,
+                              volume_path, chap_auth, lun=0,
+                              check_exit_code=False, old_name=None):
+        # NOTE(jdg): tgt driver has an issue where with alot of activity
+        # (or sometimes just randomly) it will get *confused* and attempt
+        # to reuse a target ID, resulting in a target already exists error
+        # Typically a simple retry will address this
+
+        # For now we have this while loop, might be useful in the
+        # future to throw a retry decorator in common or utils
+        attempts = 2
+        while attempts > 0:
+            attempts -= 1
+            try:
+                # NOTE(jdg): For TgtAdm case iscsi_name is all we need
+                # should clean this all up at some point in the future
+                tid = self.tgtadm.create_iscsi_target(
+                    iscsi_name,
+                    iscsi_target,
+                    0,
+                    volume_path,
+                    chap_auth,
+                    check_exit_code=check_exit_code,
+                    old_name=old_name)
+
+            except brick_exception.ISCSITargetCreateFailed:
+                if attempts == 0:
+                    raise
+                else:
+                    LOG.warning(_('Error creating iSCSI target, retrying '
+                                  'creation for target: %s') % iscsi_name)
+        return tid
+
     def ensure_export(self, context, volume):
         """Synchronously recreates an export for a logical volume."""
         # NOTE(jdg): tgtadm doesn't use the iscsi_targets table
@@ -427,9 +460,9 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver):
                                           volume['name'])
             iscsi_target = 1
 
-            self.tgtadm.create_iscsi_target(iscsi_name, iscsi_target,
-                                            0, volume_path, chap_auth,
-                                            check_exit_code=False)
+            self._create_tgtadm_target(iscsi_name, iscsi_target,
+                                       volume_path, chap_auth)
+
             return
 
         if not isinstance(self.tgtadm, iscsi.TgtAdm):
@@ -466,10 +499,13 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver):
 
         # NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need
         # should clean this all up at some point in the future
-        self.tgtadm.create_iscsi_target(iscsi_name, iscsi_target,
-                                        0, volume_path, chap_auth,
-                                        check_exit_code=False,
-                                        old_name=old_name)
+        self._create_tgtadm_target(iscsi_name, iscsi_target,
+                                   volume_path, chap_auth,
+                                   lun=0,
+                                   check_exit_code=False,
+                                   old_name=old_name)
+
+        return
 
     def _fix_id_migration(self, context, volume):
         """Fix provider_location and dev files to address bug 1065702.
@@ -563,13 +599,10 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver):
         chap_password = utils.generate_password()
         chap_auth = self._iscsi_authentication('IncomingUser', chap_username,
                                                chap_password)
-        # NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need
-        # should clean this all up at some point in the future
-        tid = self.tgtadm.create_iscsi_target(iscsi_name,
-                                              iscsi_target,
-                                              0,
-                                              volume_path,
-                                              chap_auth)
+
+        tid = self._create_tgtadm_target(iscsi_name, iscsi_target,
+                                         volume_path, chap_auth)
+
         model_update['provider_location'] = self._iscsi_location(
             self.configuration.iscsi_ip_address, tid, iscsi_name, lun)
         model_update['provider_auth'] = self._iscsi_authentication(
-- 
2.45.2