Change in vdsm[ovirt-3.3]: iscsi: Iscsi rescan cleanup
smizrahi at redhat.com
smizrahi at redhat.com
Mon Nov 11 13:53:56 UTC 2013
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 <smizrahi at 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')
-
- at 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 <smizrahi at redhat.com>
More information about the vdsm-patches
mailing list