Nir Soffer has posted comments on this change. Change subject: Using startCmd func for running async execution ......................................................................
Patch Set 1: (4 comments) https://gerrit.ovirt.org/#/c/60237/1/lib/vdsm/commands.py File lib/vdsm/commands.py: Line 39 Line 40 Line 41 Line 42 Line 43 run_cmd? Line 46: resetCpuAffinity=True): Line 47: """ Line 48: Duplication of startCmd method without the use of deathSignal and sync Line 49: parameters. Using simple_exec_cmd will execute the command by popen Line 50: in sync to the call and return outputs. We cannot duplicate code like this. We need to extract the common parts to a private helper and call it from both functions. I can work like this: def run_cmd(...): proc = _start_process(...) log... communicate... log... return rc, out, err def start_cmd(...): proc = _start_process(...) wrap it ... return proc def _start_process(...): wrap command... start it return it Line 51: Line 52: This is partial work to keep the usage of asyncProc in v2v and Line 53: imageSharing. Line 54: """ Line 83: Line 84: return p.returncode, out, err Line 85: Line 86: Line 87: def startCmd(command, sudo=False, cwd=None, data=None, raw=False, start_cmd? Line 88: printable=None, env=None, nice=None, ioclass=None, Line 89: ioclassdata=None, setsid=False, execCmdLogger=logging.root, Line 90: deathSignal=0, resetCpuAffinity=True): Line 91: """ Line 114: p = AsyncProc(CPopen(command, close_fds=True, cwd=cwd, env=env, Line 115: deathSignal=deathSignal)) Line 116: if data is not None: Line 117: p.stdin.write(data) Line 118: p.stdin.flush() Do we have code using this? If not, better remove this. Code that want to write to stdin should do it. See imagesharing for example of such code, it does not use this feature. Line 119: Line 120: return p Line 121: Line 122: -- To view, visit https://gerrit.ovirt.org/60237 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id38507a04a124918f3865cf011bbf5adc7bc31d5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
