Nir Soffer has posted comments on this change. Change subject: udevadm: Handle errors and timeouts in udevadm.settle() ......................................................................
Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/39594/5/vdsm/storage/udevadm.py File vdsm/storage/udevadm.py: Line 56: Line 57: try: Line 58: _run_command(args) Line 59: except Error as e: Line 60: logging.error("%s", e) > We probably need to catch and report the timeout. Best thing would be to us The only way to detect timeout is: if e.rc == 1 and "usevadm settle - timeout" in e.out: # timeout else: # other error This depends on undocumented behaivor in systemd-208 and udev-147, writing a message about timeout to stdout, and writing the pending events in udev event qeueue: printf("\nudevadm settle - timeout of %i seconds reached, the event queue contains:\n", timeout); This will not work with upstream which never times out, or with my timeout fix http://lists.freedesktop.org/archives/systemd-devel/2015-April/030324.html which does not print anything to stdout. I don't think we should write code based on undocumented behavior if we can avoid it. We can handle errors only in multipath.rescan(), but then we may fail an iscsi connect (we settle now after connect), which may not be very helpful (we got connected, udevadm freaked out). For long term, I think we should drop udevadm and check devices availability when possible. When we cannot, better use libudev to connect to udev and implement the waiting ourself, maybe wait for specifc events (subsystem, device). Line 61: Line 62: Line 63: def _run_command(args): Line 64: cmd = [_UDEVADM.cmd] -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Candace Sheremeta <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
