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

diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py
index 50e417c7d401a7d9adad48170962f22935a8f670..49534a3f5203965aba8cdb4b60968fae0c7aadc0 100644 (file)
--- 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 71873ff2440e05940b429c3a63618ed19e499bb8..65f20e6e424a702fff4211bb5e5280d02cda9154 100644 (file)
--- 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