Saggi Mizrahi has uploaded a new change for review. Change subject: iscsi: Iscsi rescan cleanup ......................................................................
iscsi: Iscsi rescan cleanup * Remove forceIscsiScan(): iscsiadm already does that when refreshing sessions * Cleanup iscsi.rescan(): Replace complex logic with a simpler implementation since new iscsiadm takes care of all of this itself * Make iscsi.rescan() more testable * Test confusing timeout semantics of iscsi.rescan() Bug-Url: https://bugzilla.redhat.com/964595 Change-Id: I842eb37cea3bd5e8cd357a310dd966081b242abd Signed-off-by: Saggi Mizrahi <smizr...@redhat.com> --- M tests/Makefile.am A tests/iscsiTests.py M vdsm.spec.in M vdsm/storage/iscsi.py M vdsm/storage/iscsiadm.py M vdsm/storage/multipath.py M vdsm/supervdsmServer 7 files changed, 111 insertions(+), 62 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/21136/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index f87eddd..5efae38 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -37,6 +37,7 @@ guestIFTests.py \ hooksTests.py \ ipwrapperTests.py \ + iscsiTests.py \ jsonRpcTests.py \ jsonRpcUtils.py \ libvirtconnectionTests.py \ diff --git a/tests/iscsiTests.py b/tests/iscsiTests.py new file mode 100644 index 0000000..44f6962 --- /dev/null +++ b/tests/iscsiTests.py @@ -0,0 +1,80 @@ +import threading +import time +from contextlib import contextmanager + +from testrunner import VdsmTestCase as TestCaseBase + +from storage import iscsi + + +class AsyncStubOperation(object): + def __init__(self, timeout): + self._evt = threading.Event() + if timeout == 0: + self._evt.set() + else: + threading.Timer(timeout, self._evt.set) + + def wait(self, timeout=None, cond=None): + if cond is not None: + raise Exception("TODO!!!") + + self._evt.wait(timeout) + + def stop(self): + self._evt.set() + + def result(self): + if self._evt.is_set(): + return (None, None) + else: + return None + + +class RescanTimeoutTests(TestCaseBase): + def setUp(self): + self._iscsiadm_rescan_async = \ + iscsi.iscsiadm.session_rescan_async + iscsi.iscsiadm.session_rescan_async = self._iscsi_stub + self._timeout = 0 + + def tearDown(self): + iscsi.iscsiadm.session_rescan_async = \ + self._iscsiadm_rescan_async + + def _iscsi_stub(self): + return AsyncStubOperation(self._timeout) + + @contextmanager + def assertMaxDuration(self, maxtime): + start = time.time() + try: + yield + finally: + end = time.time() + elapsed = end - start + if maxtime < elapsed: + self.fail("Operation was too slow %fs > %fs" % + (elapsed, maxtime)) + + @contextmanager + def assertMinDuration(self, mintime): + start = time.time() + try: + yield + finally: + end = time.time() + elapsed = end - start + if mintime > elapsed: + self.fail("Operation was too fast %fs > %fs" % + (elapsed, mintime)) + + def testFast(self): + self._timeout = 0 + with self.assertMinDuration(2): + iscsi.rescan(2, 4) + + def testSlow(self): + self._timeout = 60 + with self.assertMaxDuration(3): + iscsi.rescan(1, 2) diff --git a/vdsm.spec.in b/vdsm.spec.in index b0b89a1..cae9cab 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -146,7 +146,7 @@ # Update the qemu-kvm requires when block_stream will be included Requires: qemu-kvm >= 2:0.12.1.2-2.295.el6_3.4 Requires: qemu-img >= 2:0.12.1.2-2.295.el6_3.4 -Requires: iscsi-initiator-utils >= 6.2.0.872-15 +Requires: iscsi-initiator-utils >= 6.2.0.873-3 Requires: device-mapper-multipath >= 0.4.9-52 Requires: e2fsprogs >= 1.41.12-11 Requires: fence-agents diff --git a/vdsm/storage/iscsi.py b/vdsm/storage/iscsi.py index 7da94ab..f4fdceb 100644 --- a/vdsm/storage/iscsi.py +++ b/vdsm/storage/iscsi.py @@ -30,7 +30,6 @@ import time from collections import namedtuple -from vdsm import constants import misc from vdsm.config import config import devicemapper @@ -373,38 +372,15 @@ @misc.samplingmethod -def rescan(): - try: - iscsiadm.session_rescan() - except iscsiadm.IscsiError: - pass +def rescan(minTimeout=None, maxTimeout=None): + # FIXME: This whole thing is wrong from the core. We need to make rescan + # completely async and have methods timeout on their own if they + # can't find the devices they are looking for + if minTimeout is None: + minTimeout = config.getint('irs', 'scsi_rescan_minimal_timeout') + if maxTimeout is None: + maxTimeout = config.getint('irs', 'scsi_rescan_maximal_timeout') - -@misc.samplingmethod -def forceScsiScan(): - processes = [] - minTimeout = config.getint('irs', 'scsi_rescan_minimal_timeout') - maxTimeout = config.getint('irs', 'scsi_rescan_maximal_timeout') - for hba in glob.glob(SCAN_PATTERN): - cmd = [constants.EXT_DD, 'of=' + hba] - p = misc.execCmd(cmd, sudo=False, sync=False) - try: - p.stdin.write("- - -") - p.stdin.flush() - p.stdin.close() - except OSError as e: - if p.wait(0) is False: - log.error("pid %s still running", p.pid) - log.warning("Error in rescan of hba:%s with returncode:%s and " - "error message: %s", hba, p.returncode, - p.stderr.read(1000)) - if e.errno != errno.EPIPE: - raise - else: - log.warning("Ignoring error in rescan of hba %s: ", - hba, exc_info=True) - continue - processes.append((hba, p)) if (minTimeout > maxTimeout or minTimeout < 0): minTimeout = 2 maxTimeout = 30 @@ -412,24 +388,13 @@ "illegal value: scsi_rescan_minimal_timeout or " "scsi_rescan_maximal_timeout. Set to %s and %s seconds " "respectively.", minTimeout, maxTimeout) + log.debug("Performing SCSI scan, this will take up to %s seconds", maxTimeout) + + rescanOp = iscsiadm.session_rescan_async() time.sleep(minTimeout) - for i in xrange(maxTimeout - minTimeout): - for p in processes[:]: - (hba, proc) = p - if proc.wait(0): - if proc.returncode != 0: - log.warning('returncode for: %s is: %s', hba, - proc.returncode) - processes.remove(p) - if not processes: - break - else: - time.sleep(1) - else: - log.warning("Still waiting for scsi scan of hbas: %s", - tuple(hba for p in processes)) + rescanOp.wait(timeout=(maxTimeout - minTimeout)) def devIsiSCSI(dev): diff --git a/vdsm/storage/iscsiadm.py b/vdsm/storage/iscsiadm.py index cd28db8..9bb27ea 100644 --- a/vdsm/storage/iscsiadm.py +++ b/vdsm/storage/iscsiadm.py @@ -1,6 +1,7 @@ from threading import Lock import misc from vdsm import constants +from vdsm.utils import AsyncProcessOperation # iscsiadm exit statuses ISCSI_ERR_SESS_EXISTS = 15 @@ -71,7 +72,7 @@ _iscsiadmLock = Lock() -def _runCmd(args, hideValue=False): +def _runCmd(args, hideValue=False, sync=True): # FIXME: I don't use supervdsm because this entire module has to just be # run as root and there is no such feature yet in supervdsm. When such # feature exists please change this. @@ -88,7 +89,7 @@ if i < (len(printCmd) - 1): printCmd[i + 1] = "****" - return misc.execCmd(cmd, printable=printCmd, sudo=True) + return misc.execCmd(cmd, printable=printCmd, sudo=True, sync=sync) def iface_exists(interfaceName): @@ -294,12 +295,21 @@ raise IscsiNodeError(rc, out, err) -def session_rescan(): - rc, out, err = _runCmd(["-m", "session", "-R"]) - if rc == 0: - return +def session_rescan_async(): + proc = _runCmd(["-m", "session", "-R"], sync=False) - raise IscsiSessionError(rc, out, err) + def parse_result(rc, out, err): + if rc == 0: + return + + raise IscsiSessionError(rc, out, err) + + return AsyncProcessOperation(proc, parse_result) + + +def session_rescan(): + aop = session_rescan_async() + return aop.result() def session_logout(sessionId): diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py index 1f9773c..29851f3 100644 --- a/vdsm/storage/multipath.py +++ b/vdsm/storage/multipath.py @@ -105,8 +105,6 @@ # First ask iSCSI to rescan all its sessions iscsi.rescan() - supervdsm.getProxy().forceScsiScan() - # Now let multipath daemon pick up new devices cmd = [constants.EXT_MULTIPATH, "-r"] misc.execCmd(cmd, sudo=True) diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer index 40ec9df..6c2d3f5 100755 --- a/vdsm/supervdsmServer +++ b/vdsm/supervdsmServer @@ -51,7 +51,6 @@ from lsblk import getLsBlk as _getLsBlk from storage.multipath import getScsiSerial as _getScsiSerial -from storage.iscsi import forceScsiScan as _forceScsiScan from storage.iscsi import getDevIscsiInfo as _getdeviSCSIinfo from storage.iscsi import readSessionInfo as _readSessionInfo from supervdsm import _SuperVdsmManager @@ -142,10 +141,6 @@ @logDecorator def getScsiSerial(self, *args, **kwargs): return _getScsiSerial(*args, **kwargs) - - @logDecorator - def forceScsiScan(self, *args, **kwargs): - return _forceScsiScan(*args, **kwargs) @logDecorator def removeDeviceMapping(self, devName): -- To view, visit http://gerrit.ovirt.org/21136 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I842eb37cea3bd5e8cd357a310dd966081b242abd Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Saggi Mizrahi <smizr...@redhat.com> _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches