Change in vdsm[master]: udevadm: Handle errors and timeouts in udevadm.settle()

ahino at redhat.com ahino at redhat.com
Tue Apr 7 11:03:54 UTC 2015


Ala Hino has posted comments on this change.

Change subject: udevadm: Handle errors and timeouts in udevadm.settle()
......................................................................


Patch Set 1:

(6 comments)

https://gerrit.ovirt.org/#/c/39594/1//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-04-06 19:09:41 +0300
Line 6: 
Line 7: udevadm: Handle errors and timeouts in udevadm.settle()
Line 8: 
Line 9: After using udevadm.settle() introduced in commit 86535ddef2, we run into an
> s/run/ran/
Done
Line 10: issue where vdsm didn't start and stayed in initializing state.  This occurred
Line 11: because, during startup vdsm calls storage refresh, that calls udevadm
Line 12: settle(). At this point, udevadm tool fails and returns value != 0.  As a
Line 13: result, udevadm.settle() raises an error that interrupts vdsm startup process.


Line 14: 
Line 15: The reason to add this error handling is because we couldn't identify when
Line 16: timeout occurs. Timeout behavior is not documented in udevadm tool, and
Line 17: looking into the code, we found two types of logic:
Line 18: - On rhel 6 and 7 and on fed 20, if timeout occurs, rc=1 and 'timeout' info
> s/rhel/RHEL/g, s/fed/Fedora/g
Done
Line 19:   printed to stdout
Line 20: - On fed 21, timeout is calculated only when process runs under root.
Line 21:   vdsm doesn't run under root hence, timeout is ignored.
Line 22: 


Line 14: 
Line 15: The reason to add this error handling is because we couldn't identify when
Line 16: timeout occurs. Timeout behavior is not documented in udevadm tool, and
Line 17: looking into the code, we found two types of logic:
Line 18: - On rhel 6 and 7 and on fed 20, if timeout occurs, rc=1 and 'timeout' info
> - On rhel 6 and 7 and on fed 20 -> On el6, el7, and f20
Done
Line 19:   printed to stdout
Line 20: - On fed 21, timeout is calculated only when process runs under root.
Line 21:   vdsm doesn't run under root hence, timeout is ignored.
Line 22: 


Line 15: The reason to add this error handling is because we couldn't identify when
Line 16: timeout occurs. Timeout behavior is not documented in udevadm tool, and
Line 17: looking into the code, we found two types of logic:
Line 18: - On rhel 6 and 7 and on fed 20, if timeout occurs, rc=1 and 'timeout' info
Line 19:   printed to stdout
> No, real programmers write error messages to stdout.
Done
Line 20: - On fed 21, timeout is calculated only when process runs under root.
Line 21:   vdsm doesn't run under root hence, timeout is ignored.
Line 22: 
Line 23: We decided to to handle the errors in settle() rather than propagating them


Line 19:   printed to stdout
Line 20: - On fed 21, timeout is calculated only when process runs under root.
Line 21:   vdsm doesn't run under root hence, timeout is ignored.
Line 22: 
Line 23: We decided to to handle the errors in settle() rather than propagating them
> s/to to/to/
Done
Line 24: because, this settle api is like enhanced-sleep. If sleep fails, we should not
Line 25: fail the complete operation. Failing the complete operation is more severe than
Line 26: simply ignoring the error in this case, that is not common.
Line 27: This is the same behavior implemented in hba.rescan.


Line 23: We decided to to handle the errors in settle() rather than propagating them
Line 24: because, this settle api is like enhanced-sleep. If sleep fails, we should not
Line 25: fail the complete operation. Failing the complete operation is more severe than
Line 26: simply ignoring the error in this case, that is not common.
Line 27: This is the same behavior implemented in hba.rescan.
> ignoring -> handling
Done
Line 28: 
Line 29: Once udev handles timeout behavior correctly, i.e. by returning a value indicating
Line 30: that timeout occurred, we will be able to enhance our error handling as well.
Line 31: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7eac964ca9bd399ff6e8a23d591a7f78651740f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Candace Sheremeta <cshereme at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list