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@redhat.com Reviewed-on: http://gerrit.ovirt.org/34245 Reviewed-by: Adam Litke alitke@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com Reviewed-by: Federico Simoncelli fsimonce@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"

+@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):