Change in vdsm[ovirt-3.5]: hba: Rescan using SCSI layer

nsoffer at redhat.com nsoffer at redhat.com
Thu Nov 27 10:02:39 UTC 2014


Hello Adam Litke, Federico Simoncelli, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/35650

to review the following change.

Change subject: hba: Rescan using SCSI layer
......................................................................

hba: Rescan using SCSI layer

Since commit b6a27493a4 (Rescan SAN devices before rebuilding multipath
bindings) in Jan 2009, we perform scsi scan during multiapth rescan flow.

In Sep 2013, commit 66c24c1996 (iscsi: Iscsi rescan cleanup) removed scsi scan,
assuming that iscsiadm --rescan was good enough.  iscsiadm do perform scsi
scan, but only for scsi_host with iscsi connection.  Since this change, new
LUNs created on FC storage server are not discovered by hosts.

In commit 674337d8f9 (multipath: Rescan also FC devices), we tried to fix HBA
rescanning by using issue_lip.  But issue_lip was proved to be disruptive, (see
bug 1157681) and was disabled in commit 48fa75287e (multiapth: Disable hba
rescanning 22 by default).

This patch restores the missing scsi scan for fc_hosts, enabling discovery of
new FC LUNs in the multiapth rescan flow.

Since scsi scan is a blocking operation, we perform the scan using external
command line tool, ensuring that vdsm cannot get stuck. This tool can be also
useful outside of vdsm.

In the fc-scan tool, we perform the scan in parallel, so blocking on
unresponsive fc_host will not prevent discovery of new devices on other
fc_hosts.

Change-Id: I7deb3047f1b75c4c65d59602b908835515290993
Bug-Url: https://bugzilla.redhat.com/1159839
Relates-To: https://bugzilla.redhat.com/1123637
Relates-To: https://bugzilla.redhat.com/1157681
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/34245
Reviewed-by: Adam Litke <alitke at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce at redhat.com>
---
M Makefile.am
M debian/vdsm.install
M lib/vdsm/config.py.in
M lib/vdsm/constants.py.in
M vdsm.spec.in
M vdsm/storage/Makefile.am
A vdsm/storage/fc-scan
M vdsm/storage/hba.py
M vdsm/storage/multipath.py
M vdsm/sudoers.vdsm.in
M vdsm/supervdsmServer
11 files changed, 151 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/35650/1

diff --git a/Makefile.am b/Makefile.am
index 838c53d..8686f29 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -70,6 +70,7 @@
 	vdsm/vdsm \
 	vdsm/vdsm-restore-net-config \
 	vdsm/storage/curl-img-wrap \
+	vdsm/storage/fc-scan \
 	vdsm-tool/vdsm-tool \
 	vds_bootstrap/setup \
 	$(NULL)
diff --git a/debian/vdsm.install b/debian/vdsm.install
index 594aeb2..823a37d 100644
--- a/debian/vdsm.install
+++ b/debian/vdsm.install
@@ -25,6 +25,7 @@
 ./usr/lib/python2.7/dist-packages/vdsmapi.py
 ./var/lib/vdsm/bonding-defaults.json
 ./usr/libexec/vdsm/curl-img-wrap
+./usr/libexec/vdsm/fc-scan
 ./usr/libexec/vdsm/ovirt_functions.sh
 ./usr/libexec/vdsm/persist-vdsm-hooks
 ./usr/libexec/vdsm/safelease
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 583ad77..ab330be 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -266,11 +266,6 @@
         ('vol_size_sample_interval', '60',
             'How often should the volume size be checked (seconds).'),
 
-        ('hba_rescan', 'false',
-            'Enable hba scanning when rescanning multipath. Required to '
-            'detect new or removed luns on FCP server. Disabled by default '
-            'since it may be disruptive.'),
-
         ('scsi_rescan_minimal_timeout', '2',
             'The minimum number of seconds to wait for scsi scan to return.'),
 
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 01c170a..4539d26 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -167,3 +167,4 @@
 EXT_VDSM_TOOL = os.path.join('@BINDIR@', 'vdsm-tool')
 
 EXT_CURL_IMG_WRAP = '@LIBEXECDIR@/curl-img-wrap'
+EXT_FC_SCAN = '@LIBEXECDIR@/fc-scan'
diff --git a/vdsm.spec.in b/vdsm.spec.in
index a099f9d..bbd384a 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1020,6 +1020,7 @@
 %{_datadir}/%{vdsm_name}/logUtils.py*
 %{_datadir}/%{vdsm_name}/dsaversion.py*
 %{_libexecdir}/%{vdsm_name}/curl-img-wrap
+%{_libexecdir}/%{vdsm_name}/fc-scan
 %{_libexecdir}/%{vdsm_name}/persist-vdsm-hooks
 %{_libexecdir}/%{vdsm_name}/unpersist-vdsm-hook
 %{_libexecdir}/%{vdsm_name}/ovirt_functions.sh
diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am
index 0cfa5de..9145d0e 100644
--- a/vdsm/storage/Makefile.am
+++ b/vdsm/storage/Makefile.am
@@ -72,7 +72,9 @@
 	volume.py
 
 dist_vdsmexec_SCRIPTS = \
-	curl-img-wrap
+	curl-img-wrap \
+	fc-scan \
+	$(NULL)
 
 nodist_vdsmstorage_DATA = \
 	lvm.env \
diff --git a/vdsm/storage/fc-scan b/vdsm/storage/fc-scan
new file mode 100755
index 0000000..344345d
--- /dev/null
+++ b/vdsm/storage/fc-scan
@@ -0,0 +1,115 @@
+#!/usr/bin/python
+#
+# Copyright 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+"""
+Usage: fc-scan [-v|-h]
+
+Perform SCSI scan on Fibre Channel scsi_hosts, adding new LUNs. This procedure
+will not remove existing LUNs. Must run as root.
+
+Options:
+  -v        enable verbose logging
+  -h        display this help and exit
+
+Exit codes:
+  0         found fc_hosts scanned successfully
+  1         scaning some hosts failed
+"""
+
+import glob
+import logging
+import os
+import sys
+import threading
+
+log = logging.getLogger("fc-scan")
+
+
+class Scan(object):
+
+    def __init__(self, host):
+        self.host = host
+        self.succeeded = False
+        self.thread = None
+
+    def start(self):
+        self.thread = threading.Thread(target=self.run)
+        self.thread.daemon = True
+        self.thread.start()
+
+    def wait(self):
+        self.thread.join()
+
+    def run(self):
+        try:
+            path = "/sys/class/scsi_host/%s/scan" % self.host
+            log.debug("Scanning %s", path)
+            start = monotonic_time()
+            fd = os.open(path, os.O_WRONLY)
+            try:
+                os.write(fd, "- - -")
+            finally:
+                os.close(fd)
+            self.succeeded = True
+            elapsed = monotonic_time() - start
+            log.debug("Scanned %s in %.2f seconds", path, elapsed)
+        except OSError as e:
+            log.error("Scanning %s failed: %s", path, e)
+        except Exception:
+            log.exception("Scanning %s failed", path)
+
+
+def main(args):
+    if '-h' in args:
+        print __doc__
+        return 0
+
+    logging.basicConfig(
+        level=logging.DEBUG if '-v' in args else logging.INFO,
+        format="%(name)s: %(message)s")
+
+    hosts = [os.path.basename(path)
+             for path in glob.glob("/sys/class/fc_host/host*")]
+
+    if not hosts:
+        log.debug("No fc_host found")
+        return 0
+
+    scans = []
+
+    for host in hosts:
+        s = Scan(host)
+        s.start()
+        scans.append(s)
+
+    for s in scans:
+        s.wait()
+
+    if not all(s.succeeded for s in scans):
+        return 1
+
+
+def monotonic_time():
+    return os.times()[4]
+
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv[1:]))
diff --git a/vdsm/storage/hba.py b/vdsm/storage/hba.py
index 050bec5..dadbd60 100644
--- a/vdsm/storage/hba.py
+++ b/vdsm/storage/hba.py
@@ -22,8 +22,15 @@
 Collect HBA information
 """
 import glob
-import os
 import logging
+import os
+
+from vdsm import constants
+from vdsm import utils
+from vdsm.config import config
+import zombiereaper
+
+import misc
 
 log = logging.getLogger("Storage.HBA")
 
@@ -36,29 +43,27 @@
 NODE_NAME = "node_name"
 
 
+ at misc.samplingmethod
 def rescan():
     """
-    Rescan HBAs connections, updating available devices.
-
-    This operation performs a Loop Initialization Protocol (LIP) and then
-    scans the interconnect and causes the SCSI layer to be updated to reflect
-    the devices currently on the bus. A LIP is, essentially, a bus reset, and
-    will cause device addition and removal.
-
-    Bear in mind that issue_lip is an asynchronous operation. The command may
-    complete before the entire scan has completed.
-
-    Note: Must be executed as root.
-    TODO: Some drivers do not support this operation.
+    Rescan HBAs discovering new devices.
     """
-    log.info("Rescanning HBAs")
-    for path in glob.glob(FC_HOST_MASK + '/issue_lip'):
-        log.debug("Issuing lip %s", path)
-        try:
-            with open(path, 'w') as f:
-                f.write('1')
-        except IOError as e:
-            logging.error("Error issuing lip: %s", e)
+    timeout = config.getint('irs', 'scsi_rescan_maximal_timeout')
+
+    log.debug("Starting scan")
+    proc = utils.execCmd([constants.EXT_FC_SCAN], sync=False, sudo=True,
+                         execCmdLogger=log)
+    try:
+        proc.wait(timeout)
+    finally:
+        if proc.returncode is None:
+            log.warning("Timeout scanning")
+            zombiereaper.autoReapPID(proc.pid)
+        elif proc.returncode != 0:
+            stderr = proc.stderr.read(512)
+            log.error("Scan failed: %r", stderr)
+        else:
+            log.debug("Scan finished")
 
 
 def getiSCSIInitiators():
diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index 34573c2..176e050 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -32,7 +32,7 @@
 
 from vdsm import constants
 from vdsm import utils
-from vdsm.config import config
+import hba
 import misc
 import iscsi
 import supervdsm
@@ -123,9 +123,7 @@
 
     # First rescan iSCSI and FCP connections
     iscsi.rescan()
-
-    if config.getboolean('irs', 'hba_rescan'):
-        supervdsm.getProxy().hbaRescan()
+    hba.rescan()
 
     # Now let multipath daemon pick up new devices
     misc.execCmd([constants.EXT_MULTIPATH], sudo=True)
diff --git a/vdsm/sudoers.vdsm.in b/vdsm/sudoers.vdsm.in
index 584807d..dff5539 100644
--- a/vdsm/sudoers.vdsm.in
+++ b/vdsm/sudoers.vdsm.in
@@ -17,6 +17,7 @@
     @BINDIR@/vdsm-tool service-restart multipathd, \
     @BINDIR@/vdsm-tool service-reload multipathd, \
     @ISCSIADM_PATH@ *, \
+    @LIBEXECDIR@/fc-scan, \
     @LVM_PATH@, \
     @CAT_PATH@ /sys/block/*/device/../../*, \
     @CAT_PATH@ /sys/devices/platform/host*, \
diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 5d09ec1..a329eb4 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -62,7 +62,6 @@
 from network.api import (addNetwork, delNetwork, editNetwork, setupNetworks,
                          setSafeNetworkConfig)
 
-from storage import hba
 from network.tc import setPortMirroring, unsetPortMirroring
 from storage.multipath import getScsiSerial as _getScsiSerial
 from storage.iscsi import getDevIscsiInfo as _getdeviSCSIinfo
@@ -279,10 +278,6 @@
     @logDecorator
     def setSafeNetworkConfig(self):
         return setSafeNetworkConfig()
-
-    @logDecorator
-    def hbaRescan(self):
-        return hba.rescan()
 
     @logDecorator
     def udevTrigger(self, guid):


-- 
To view, visit http://gerrit.ovirt.org/35650
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7deb3047f1b75c4c65d59602b908835515290993
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>


More information about the vdsm-patches mailing list