Change in vdsm[ovirt-3.5]: multipath: Wait for udev events after rescan

ahino at redhat.com ahino at redhat.com
Tue Apr 7 13:38:24 UTC 2015


Hello Douglas Schilling Landgraf, Nir Soffer, Dan Kenigsberg, Allon Mureinik,

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

    https://gerrit.ovirt.org/39606

to review the following change.

Change subject: multipath: Wait for udev events after rescan
......................................................................

multipath: Wait for udev events after rescan

Previously we were using time.sleep(2) for waiting at least 2 seconds
after starting iSCSI sessions rescan. This wait was useless for two
reasons:
- The sleep was interrupted as soon as iscsiadm process terminated
- Udev events generated by the SCSI scan and by processing udev rules
  may not been handled yet when iscsiadm existed.

This patch replaces the minimal scsi timeout with udevadm.settle(),
waiting until current udev events are handled.  Since we perform both FC
and iSCSI scan, the correct place for waiting is in multipath.rescan()
and not in iscsi.rescan().

A new configuration was introduced, allowing fine tuning of settle
timeout in the field.

    [irs]
    scsi_settle_timeout = 5

Change-Id: Ieeffbdd447e96c4ec92b829388da1dc6222a2dae
Relates-To: https://bugzilla.redhat.com/1205877
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/39288
Reviewed-by: Douglas Schilling Landgraf <dougsland at redhat.com>
Tested-by: Douglas Schilling Landgraf <dougsland at redhat.com>
Reviewed-by: Allon Mureinik <amureini at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M lib/vdsm/config.py.in
M vdsm/storage/iscsi.py
M vdsm/storage/multipath.py
3 files changed, 17 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/39606/1

diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 9dcc487..a5f1f9b 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -266,12 +266,13 @@
         ('vol_size_sample_interval', '60',
             'How often should the volume size be checked (seconds).'),
 
-        ('scsi_rescan_minimal_timeout', '2',
-            'The minimum number of seconds to wait for scsi scan to return.'),
-
         ('scsi_rescan_maximal_timeout', '30',
             'The maximal number of seconds to wait for scsi scan to return.'),
 
+        ('scsi_settle_timeout', '5',
+            'Maximum number of seconds to wait until udev events are handled '
+            'after modifying scsi interconnects.'),
+
         ('sd_health_check_delay', '10',
             'Storage domain health check delay, the amount of seconds to '
             'wait between two successive run of the domain health check.'),
diff --git a/vdsm/storage/iscsi.py b/vdsm/storage/iscsi.py
index e7c25a0..5fbd4db 100644
--- a/vdsm/storage/iscsi.py
+++ b/vdsm/storage/iscsi.py
@@ -412,29 +412,11 @@
 
 
 @misc.samplingmethod
-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')
-
-    if (minTimeout > maxTimeout or minTimeout < 0):
-        minTimeout = 2
-        maxTimeout = 30
-        log.warning("One of the following configuration arguments has an "
-                    "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)
-
+def rescan():
+    timeout = config.getint('irs', 'scsi_rescan_maximal_timeout')
+    log.debug("Performing SCSI scan, this will take up to %s seconds", timeout)
     rescanOp = iscsiadm.session_rescan_async()
-    time.sleep(minTimeout)
-    rescanOp.wait(timeout=(maxTimeout - minTimeout))
+    rescanOp.wait(timeout=timeout)
 
 
 def devIsiSCSI(dev):
diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index c69d30f..bcfedd8 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -32,11 +32,14 @@
 
 from vdsm import constants
 from vdsm import utils
+from vdsm.config import config
+
 import hba
 import misc
 import iscsi
 import supervdsm
 import devicemapper
+import udevadm
 
 import storage_exception as se
 
@@ -127,6 +130,12 @@
     # Now let multipath daemon pick up new devices
     misc.execCmd([constants.EXT_MULTIPATH], sudo=True)
 
+    # Scanning SCSI interconnects starts a storm of udev events. Wait until all
+    # events are processed, ensuring detection of new devices and creation or
+    # update of multipath devices.
+    timeout = config.getint('irs', 'scsi_settle_timeout')
+    udevadm.settle(timeout)
+
 
 def isEnabled():
     """


-- 
To view, visit https://gerrit.ovirt.org/39606
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieeffbdd447e96c4ec92b829388da1dc6222a2dae
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list