Nir Soffer has posted comments on this change. Change subject: multipath configurator: reload only if service is active ......................................................................
Patch Set 5: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/36387/5/lib/vdsm/tool/configurators/multipath.py File lib/vdsm/tool/configurators/multipath.py: Line 144 Line 145 Line 146 Line 147 Line 148 > There can be 2 cases in which exceptions when reloading a service are raise If service.service_reload() raises, the code should be fixed like this: try: reload service except serviceOperationError: if service is up: raise I don't see a reason to move the racy pattern: if service is up: reload service Since multipathd is usually up, current code seems better. On the first time vdsm is installed, we will fail in reload and check the status, but when reconfiguring or upgrading vdsm, multipathd will be always up. In this case there is an theoretical race, when the service is started by someone else just after we called reload, but before we check service status. If multipathd returns a specific exit code in reload which means "service is not running", we could check this exit code and avoid this theoretical racy status check. -- To view, visit http://gerrit.ovirt.org/36387 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e8eceaaa486f05b1ea4f1733df669b2a44682a Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[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
