Change in vdsm[master]: qemuimg: add support for convert progress
nsoffer at redhat.com
nsoffer at redhat.com
Tue Apr 28 11:27:44 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: qemuimg: add support for convert progress
......................................................................
Patch Set 6:
(2 comments)
Nice, but I think we should simplify and clarify the code receiving data.
Looked only at the new QimgProgress code, will look at the rest later today.
https://gerrit.ovirt.org/#/c/33910/6/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:
Line 224: self._term_lock = threading.Lock()
Line 225:
Line 226: _log.debug(utils.commandLogFormat(cmd, cwd=cwd))
Line 227: self._command = CPopen(cmd, cwd=cwd, deathSignal=signal.SIGKILL)
Line 228: self._execution = utils.CommandStream(
_execution is a strange term here - this is a CommandStream - this is a good example that CommandStream is not a good name.
How about:
self._receiver = utils.CommandRceiver(...)
I think this is what this class does - allow you to register recv_stdout and recv_stderr and call wait until there is nothing to receive.
Line 229: self._command, self._recvstdout, self._recvstderr)
Line 230:
Line 231: def _recvstderr(self, buffer):
Line 232: self._stderr += buffer
Line 241: except ValueError:
Line 242: break
Line 243:
Line 244: last_progress = self._stdout[:idx]
Line 245: del self._stdout[:idx + 1]
I think it will be simpler and more clear:
end = buffer.rindex('\r')
if end < 0:
self._stdout += buffer
return # Needs more data
received = self._stdout + buffer[:end]
self._stdout = buffer[end + 1:]
# Skip older progress chunks
last_progress = received.rsplit('\r', 1)[-1]
handle last_progress...
Line 246:
Line 247: if last_progress:
Line 248: m = self.REGEXPR.match(last_progress.strip())
Line 249: if m is None:
--
To view, visit https://gerrit.ovirt.org/33910
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0b53e418c62bb2e91444ba5f351c916ca417299
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list