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