Nir Soffer has posted comments on this change.

Change subject: service: make service compatible with systemctl reload rc
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/36491/4/lib/vdsm/tool/service.py
File lib/vdsm/tool/service.py:

Line 297:         rc, out, err = _execSysvEnv(cmd)
Line 298:         status = service_status(srvName, False)
Line 299:         if (rc == 0) and (status != 0):
Line 300:             rc = 1
Line 301:             err = 'reload failed because service was not running'
> Correcting myself, raising the exit code would not solve the differences be
There is some other issues simulating systemd:

- systemd exit code when reloading inactive service is not documented, so it 
may change behind our back
- systemd current behavior is bad, it returns the same error (1) if service is 
inactive or if reload failed with any error, hiding the original error. So we 
cannot tell anything from the error code returned by systemd, otherwise we may 
hide real reload error.

So to fully simulate systemd, we need to do:

    if service is not running:
        return 1
    if reload service failed:
        return 1
    return 0

The caller will have to check service status to tell
if this is a failure to reload or the service was not running.
Line 302:         return (rc, out, err)
Line 303: 
Line 304:     @_sysvNative
Line 305:     def _serviceIsManaged(srvName):


-- 
To view, visit http://gerrit.ovirt.org/36491
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I878fb898204f7a8a564941b43b12c024ef208765
Gerrit-PatchSet: 4
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

Reply via email to