Nir Soffer has posted comments on this change. Change subject: Moving cpopen import to compat to allow working with python 3 ......................................................................
Patch Set 1: Code-Review-1 (1 comment) Nice, but we are not ready for this yet. https://gerrit.ovirt.org/#/c/48384/1/lib/vdsm/qemuimg.py File lib/vdsm/qemuimg.py: Line 222: Line 223: cmd = cmdutils.wrap_command(cmd, with_nice=utils.NICENESS.HIGH, Line 224: with_ioclass=utils.IOCLASS.IDLE) Line 225: _log.debug(cmdutils.command_log_line(cmd, cwd=cwd)) Line 226: self._command = Popen(cmd, cwd=cwd, deathSignal=signal.SIGKILL) cpopen.CPopen defaults are different than subprocess.Popen, so this usage is incorrect - you don't get a pipe for stdin, stdout and stderr, and the progress code will fail. We must add stdin=PIPE, stdout=PIPE, stderr=PIPE to all Popen calls to keep the current semantics. We may have other stuff which is not compatible, like close_fds. Also this can be done only after removing CPopen options which are not supported by subprocess.Popen - deathSignal, childUmas and probably other. Line 227: self._stream = utils.CommandStream( Line 228: self._command, self._recvstdout, self._recvstderr) Line 229: Line 230: def _recvstderr(self, buffer): -- To view, visit https://gerrit.ovirt.org/48384 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I181cd2e52706bfe7b24073d0f0eb42fec15ac490 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
