Yaniv Bronhaim has posted comments on this change.

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


Patch Set 2:

(3 comments)

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

Line 209:     def _initctlStatus(srvName):
Line 210:         cmd = [_INITCTL.cmd, "status", srvName]
Line 211:         rc, out, err = execCmd(cmd)
Line 212:         if rc == 0:
Line 213:             # initctl rc is 0 even though the service is stopped
we even have comment for that ..
Line 214:             rc = _isStopped(out)
Line 215:         return (rc, out, err)
Line 216: 
Line 217:     @_initctlNative


Line 210:         cmd = [_INITCTL.cmd, "status", srvName]
Line 211:         rc, out, err = execCmd(cmd)
Line 212:         if rc == 0:
Line 213:             # initctl rc is 0 even though the service is stopped
Line 214:             rc = _isStopped(out)
and it should mock it rightly already ..
Line 215:         return (rc, out, err)
Line 216: 
Line 217:     @_initctlNative
Line 218:     def _initctlRestart(srvName):


Line 224:     @_initctlNative
Line 225:     def _initctlReload(srvName):
Line 226:         cmd = [_INITCTL.cmd, "reload", srvName]
Line 227:         rc, out, err = execCmd(cmd)
Line 228:         status = service_status(srvName, False)
> no! we should work with the return values as is.. no magic hacks
if anything.. you should change service_status return value
Line 229:         if (rc == 0) and (status != 0):
Line 230:             rc = 1
Line 231:         return (rc, out, err)
Line 232: 


-- 
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to