Change in vdsm[master]: core: imageSharing - export logic to functions
nsoffer at redhat.com
nsoffer at redhat.com
Tue May 13 11:20:49 UTC 2014
Nir Soffer has posted comments on this change.
Change subject: core: imageSharing - export logic to functions
......................................................................
Patch Set 2: Code-Review-1
(4 comments)
Seems that no changes were posted in this version.
http://gerrit.ovirt.org/#/c/26759/2/vdsm/storage/imageSharing.py
File vdsm/storage/imageSharing.py:
Line 82: p = utils.execCmd(cmd, sudo=False, sync=False,
Line 83: deathSignal=signal.SIGKILL)
Line 84: try:
Line 85: _copyData(stream, p.stdin, bytes_left)
Line 86: _ensureProcessSucceded(p, constants.EXT_DD, se.StorageException)
You promised to change this to _waitProcessEnd
http://gerrit.ovirt.org/#/c/26759/1..2/vdsm/storage/imageSharing.py
I think that _waitForProcess reads better - we know that meaning "wait" when we talk about processes.
Line 87: except Exception:
Line 88: if p.returncode is None:
Line 89: p.kill()
Line 90: raise
Line 91:
Line 92: except Exception:
Line 93: if p.returncode is None:
Line 94: p.kill()
Line 95: raise
You cannot merge this with this copy and paste error, even if you cleaned it i another patch. Each patch must be correct logic change.
Line 96:
Line 97:
Line 98: def _ensureProcessSucceded(p, command, failureException):
Line 99: if not p.wait(WAIT_TIMEOUT):
Line 105: str(command), p.returncode, p.stderr.read(1000))
Line 106: raise failureException()
Line 107:
Line 108:
Line 109: def _copyData(inBuffer, outBuffer, bytes_left):
Done, but there is no change?
Line 110: total_size = bytes_left
Line 111: while bytes_left > 0:
Line 112: to_read = min(BUFFER_SIZE, bytes_left)
Line 113:
Line 118: raise se.MiscFileReadException()
Line 119:
Line 120: if not data:
Line 121: log.error("partial data %s from %s",
Line 122: total_size - bytes_left, total_size)
You replied "Done" but there is not change here?
Line 123: raise se.MiscFileReadException()
Line 124:
Line 125: try:
Line 126: outBuffer.write(data)
--
To view, visit http://gerrit.ovirt.org/26759
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I861b40cc62c3332b887b64c2525fc512cdc6a22a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Yoav Kleinberger <ykleinbe 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