From 1aef7b6304af6c19b8ceb81cbd9a3c83bb4880de Mon Sep 17 00:00:00 2001
From: John Griffith <john.griffith@solidfire.com>
Date: Tue, 1 Oct 2013 12:05:14 -0600
Subject: [PATCH] Check for backing lun on iscsi target create

Check to verify the backing lun was actually created and not just
the controller lun.  If it was NOT created, attempt to issue
tgtadm --op new to see if we can recover.

If this fails, then we want to actually fail in Cinder rather than
pretending that everything went well, so we'll log the error and raise.

Change-Id: I3cabab0d214c051267638a627664df2b673236e3
Closes-Bug: #1226337
---
 cinder/brick/iscsi/iscsi.py | 69 ++++++++++++++++++++++++++++++++++++-
 cinder/tests/test_iscsi.py  |  5 +++
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py
index 50e417c7d..49534a3f5 100644
--- a/cinder/brick/iscsi/iscsi.py
+++ b/cinder/brick/iscsi/iscsi.py
@@ -24,6 +24,7 @@ import contextlib
 import os
 import re
 import stat
+import time
 
 from oslo.config import cfg
 
@@ -127,6 +128,51 @@ class TgtAdm(TargetAdmin):
 
         return None
 
+    def _verify_backing_lun(self, iqn, tid):
+        backing_lun = True
+        capture = False
+        target_info = []
+
+        (out, err) = self._execute('tgt-admin', '--show', run_as_root=True)
+        lines = out.split('\n')
+
+        for line in lines:
+            if iqn in line and "Target %s" % tid in line:
+                capture = True
+            if capture:
+                target_info.append(line)
+            if iqn not in line and 'Target ' in line:
+                capture = False
+
+        if '        LUN: 1' not in target_info:
+            backing_lun = False
+
+        return backing_lun
+
+    def _recreate_backing_lun(self, iqn, tid, name, path):
+        LOG.warning(_('Attempting recreate of backing lun...'))
+
+        # Since we think the most common case of this is a dev busy
+        # (create vol from snapshot) we're going to add a sleep here
+        # this will hopefully give things enough time to stabilize
+        # how long should we wait??  I have no idea, let's go big
+        # and error on the side of caution
+
+        time.sleep(10)
+        try:
+            (out, err) = self._execute('tgtadm', '--lld', 'iscsi',
+                                       '--op', 'new', '--mode',
+                                       'logicalunit', '--tid',
+                                       tid, '--lun', '1', '-b',
+                                       path, run_as_root=True)
+            LOG.debug('StdOut from recreate backing lun: %s' % out)
+            LOG.debug('StdErr from recreate backing lun: %s' % err)
+        except putils.ProcessExecutionError as e:
+            LOG.error(_("Failed to recover attempt to create "
+                        "iscsi backing lun for volume "
+                        "id:%(vol_id)s: %(e)s")
+                      % {'vol_id': name, 'e': str(e)})
+
     def create_iscsi_target(self, name, tid, lun, path,
                             chap_auth=None, **kwargs):
         # Note(jdg) tid and lun aren't used by TgtAdm but remain for
@@ -167,6 +213,10 @@ class TgtAdm(TargetAdmin):
                                        '--update',
                                        name,
                                        run_as_root=True)
+
+            LOG.debug("StdOut from tgt-admin --update: %s" % out)
+            LOG.debug("StdErr from tgt-admin --update: %s" % err)
+
             # Grab targets list for debug
             # Consider adding a check for lun 0 and 1 for tgtadm
             # before considering this as valid
@@ -199,6 +249,23 @@ class TgtAdm(TargetAdmin):
                         })
             raise exception.NotFound()
 
+        # NOTE(jdg): Sometimes we have some issues with the backing lun
+        # not being created, believe this is due to a device busy
+        # or something related, so we're going to add some code
+        # here that verifies the backing lun (lun 1) was created
+        # and we'll try and recreate it if it's not there
+        if not self._verify_backing_lun(iqn, tid):
+            try:
+                self._recreate_backing_lun(iqn, tid, name, path)
+            except putils.ProcessExecutionError:
+                os.unlink(volume_path)
+                raise exception.ISCSITargetCreateFailed(volume_id=vol_id)
+
+        # Finally check once more and if no go, fail and punt
+        if not self._verify_backing_lun(iqn, tid):
+            os.unlink(volume_path)
+            raise exception.ISCSITargetCreateFailed(volume_id=vol_id)
+
         if old_persist_file is not None and os.path.exists(old_persist_file):
             os.unlink(old_persist_file)
 
@@ -495,7 +562,7 @@ class LioAdm(TargetAdmin):
                           auth_pass,
                           connector['initiator'],
                           run_as_root=True)
-        except putils.ProcessExecutionError as e:
+        except putils.ProcessExecutionError:
             LOG.error(_("Failed to add initiator iqn %s to target") %
                       connector['initiator'])
             raise exception.ISCSITargetAttachFailed(volume_id=volume['id'])
diff --git a/cinder/tests/test_iscsi.py b/cinder/tests/test_iscsi.py
index 71873ff24..65f20e6e4 100644
--- a/cinder/tests/test_iscsi.py
+++ b/cinder/tests/test_iscsi.py
@@ -42,6 +42,11 @@ class TargetAdminTestCase(object):
         self.stubs.Set(iscsi.TgtAdm, '_get_target', self.fake_get_target)
         self.stubs.Set(iscsi.LioAdm, '_get_target', self.fake_get_target)
         self.stubs.Set(iscsi.LioAdm, '__init__', self.fake_init)
+        self.stubs.Set(iscsi.TgtAdm, '_verify_backing_lun',
+                       self.fake_verify_backing_lun)
+
+    def fake_verify_backing_lun(obj, iqn, tid):
+        return True
 
     def fake_init(obj, root_helper):
         return
-- 
2.45.2