Nir Soffer has uploaded a new change for review.
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@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/88/39288/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in index b664cbd..f5572ec 100644 --- a/lib/vdsm/config.py.in +++ b/lib/vdsm/config.py.in @@ -247,12 +247,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 f12f798..a1c42b3 100644 --- a/vdsm/storage/multipath.py +++ b/vdsm/storage/multipath.py @@ -31,11 +31,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
DEV_ISCSI = "iSCSI" DEV_FCP = "FCP" @@ -66,6 +69,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 deduceType(a, b): if a == b:
automation@ovirt.org has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17264/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17438/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17264/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17438/ : FAILURE
Nir Soffer has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1: Verified+1
Tested with previous and next patch on el7
Douglas Schilling Landgraf has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1: Verified+1 Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1:
Douglas backported this to vdsm-4.16.12.1-3.el6ev and tested on rhev-h 6.6.
Allon Mureinik has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
https://gerrit.ovirt.org/#/c/39288/1//COMMIT_MSG Commit Message:
Line 14: may not been handled yet when iscsiadm existed. Line 15: Line 16: This patch replaces the minimal scsi timeout with udevadm.settle(), Line 17: waiting until current udev events are handled. Since we perform both FC Line 18: and iSCSI scan, the correct place for waiting is in multipath.rescan() I'm not sure I understand the logical inference here. Do you make only multipath.rescan() wait for device files because it is called last? (it makes sense to wait only once; I just don't understand the "sunce"). Line 19: and not in iscsi.rescan(). Line 20: Line 21: A new configuration was introduced, allowing fine tuning of settle Line 22: timeout in the field.
https://gerrit.ovirt.org/#/c/39288/1/vdsm/storage/iscsi.py File vdsm/storage/iscsi.py:
Line 412 Line 413 Line 414 Line 415 Line 416 can you refer to this FIXME removal?
https://gerrit.ovirt.org/#/c/39288/1/vdsm/storage/multipath.py File vdsm/storage/multipath.py:
Line 72: # Scanning SCSI interconnects starts a storm of udev events. Wait until all Line 73: # events are processed, ensuring detection of new devices and creation or Line 74: # update of multipath devices. Line 75: timeout = config.getint('irs', 'scsi_settle_timeout') Line 76: udevadm.settle(timeout) does `udevadm settle` give any indeication if the timeout was enough? it would be beneficial to learn if 5 seconds were enough. Line 77: Line 78: Line 79: def deduceType(a, b): Line 80: if a == b:
Nir Soffer has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1:
(3 comments)
https://gerrit.ovirt.org/#/c/39288/1//COMMIT_MSG Commit Message:
Line 14: may not been handled yet when iscsiadm existed. Line 15: Line 16: This patch replaces the minimal scsi timeout with udevadm.settle(), Line 17: waiting until current udev events are handled. Since we perform both FC Line 18: and iSCSI scan, the correct place for waiting is in multipath.rescan()
I'm not sure I understand the logical inference here. Do you make only mult
Previously we "waited" after iscsi scan, so I could replace the time.sleep() in iscsi.rescan() with udevadm.settle(). Since we do also fc scan, we should wait also there. Instead of waiting twice, we wait in the single place calling both. Line 19: and not in iscsi.rescan(). Line 20: Line 21: A new configuration was introduced, allowing fine tuning of settle Line 22: timeout in the field.
https://gerrit.ovirt.org/#/c/39288/1/vdsm/storage/iscsi.py File vdsm/storage/iscsi.py:
Line 412 Line 413 Line 414 Line 415 Line 416
can you refer to this FIXME removal?
I don't think this is broken from the core and the suggested fix is complex and does not cover the main use case.
When trying to detect new devices, you don't know yet the new devices, so we cannot wait for specific devices.
When we know what we are looking for, we can wait until iscsiadm finish (typically it is very fast), and then wait for a specific device (e.g, in getDevicesVisibility).
Async scan can help when one host is very slow, and we are looking for a device on a different host, and can finish the scan quickly without being blocked by the slow host.
To support this negative flow, we will have to create a much more complex code (maybe a thread dedicated to scanning). This can be lot of fun but is not our roadmap.
If you like I can improve the comment so it document the limits of the current code.
https://gerrit.ovirt.org/#/c/39288/1/vdsm/storage/multipath.py File vdsm/storage/multipath.py:
Line 72: # Scanning SCSI interconnects starts a storm of udev events. Wait until all Line 73: # events are processed, ensuring detection of new devices and creation or Line 74: # update of multipath devices. Line 75: timeout = config.getint('irs', 'scsi_settle_timeout') Line 76: udevadm.settle(timeout)
does `udevadm settle` give any indeication if the timeout was enough? it wo
No Line 77: Line 78: Line 79: def deduceType(a, b): Line 80: if a == b:
Dan Kenigsberg has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/39288/1//COMMIT_MSG Commit Message:
Line 14: may not been handled yet when iscsiadm existed. Line 15: Line 16: This patch replaces the minimal scsi timeout with udevadm.settle(), Line 17: waiting until current udev events are handled. Since we perform both FC Line 18: and iSCSI scan, the correct place for waiting is in multipath.rescan()
Previously we "waited" after iscsi scan, so I could replace the time.sleep(
ok, that what I understood. Line 19: and not in iscsi.rescan(). Line 20: Line 21: A new configuration was introduced, allowing fine tuning of settle Line 22: timeout in the field.
Dan Kenigsberg has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 1: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
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@redhat.com Reviewed-on: https://gerrit.ovirt.org/39288 Reviewed-by: Douglas Schilling Landgraf dougsland@redhat.com Tested-by: Douglas Schilling Landgraf dougsland@redhat.com Reviewed-by: Allon Mureinik amureini@redhat.com Reviewed-by: Dan Kenigsberg danken@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(-)
Approvals: Douglas Schilling Landgraf: Verified; Looks good to me, but someone else must approve Nir Soffer: Verified Dan Kenigsberg: Looks good to me, approved Allon Mureinik: Looks good to me, but someone else must approve
automation@ovirt.org has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (1/9) -> http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/829/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (2/9) -> http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-fc21-x86_64_me...
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (3/9) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4809/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (4/9) -> http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/800/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (5/9) -> http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/834/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (6/9) -> http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-el7-x86_64_mer...
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (7/9) -> http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-fc20-x86_64_me...
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (8/9) -> http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/834/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Started (9/9) -> http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-el6-x86_64_mer...
oVirt Jenkins CI Server has posted comments on this change.
Change subject: multipath: Wait for udev events after rescan ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/829/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-fc21-x86_64_me... : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4809/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/800/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-el6-x86_64_mer... : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/834/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-el7-x86_64_mer... : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-fc20-x86_64_me... : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/834/ : FAILURE
vdsm-patches@lists.fedorahosted.org