Liron Ar has posted comments on this change.
Change subject: introducing downloadFromStream verb ......................................................................
Patch Set 7:
(6 comments)
http://gerrit.ovirt.org/#/c/23281/7/vdsm/BindingXMLRPC.py File vdsm/BindingXMLRPC.py:
Line 144: HEADER_VOLUME = 'X-Volume-Id' Line 145: HEADER_CONTENT_LENGTH = 'content-length' Line 146: HEADER_CONTENT_TYPE = 'content-type' Line 147: CONTENT_STREAM_TYPE = 'application/octet-stream' Line 148:
redundant. ask for a reason.. if you use it only on do_PUT, put it there at
there's no ned to move it to do_PUT and to have it defined everytime the method is called. I prefer to keep it as constants as Nir and Anotoni suggested. Line 149: def setup(self): Line 150: threadLocal.client = self.client_address[0] Line 151: return basehandler.setup(self) Line 152:
http://gerrit.ovirt.org/#/c/23281/7/vdsm/storage/hsm.py File vdsm/storage/hsm.py:
Line 1679: volUUID=None): Line 1680: """ Line 1681: Download an image from a stream synchronously. Line 1682: Line 1683: Warning: Internal use only.
I still don't understand that and I prefer to understand what "internal use
internal use only means that theres no meaning to use it besides our use from the server. you can't pass the socket.fileObject parameter and therefore this verb is for internal use only. Line 1684: """ Line 1685: sdCache.produce(sdUUID) Line 1686: pool = self.getPool(spUUID) Line 1687: pool.downloadImage(methodArgs, sdUUID, imgUUID, volUUID)
http://gerrit.ovirt.org/#/c/23281/7/vdsm/storage/imageSharing.py File vdsm/storage/imageSharing.py:
Line 28: Line 29: Line 30: log = logging.getLogger("Storage.ImageSharing") Line 31: WAIT_TIMEOUT = 30 Line 32: BUFFER_SIZE = 65536
why this size and timeout? can you add short explanation above?
what explanation would you suggest? those are arbitrary values that can always be tuned. Line 33: Line 34: Line 35: def httpGetSize(methodArgs): Line 36: headers = curlImgWrap.head(methodArgs.get('url'),
Line 53: return size Line 54: Line 55: Line 56: def streamGetSize(methodArgs): Line 57: return methodArgs['contentLength']
you can't compare. getSharingMethods catches KeyError and translates the ou
that's internal implementation, and as i said - it's called from sp.py, take a look there. Line 58: Line 59: Line 60: def httpDownloadImage(dstImgPath, methodArgs): Line 61: curlImgWrap.download(methodArgs.get('url'), dstImgPath,
Line 72: stream = methodArgs['fileObj'] Line 73: Line 74: cmd = [constants.EXT_DD, "of=%s" % dstImgPath, "bs=%s" % constants.MEGAB] Line 75: p = utils.execCmd(cmd, sudo=False, sync=False, Line 76: deathSignal=signal.SIGKILL)
what do you mean? it does dd from source to destination . i prefer using ut
ddWatchCopy use to copy from file, using specified path, offset , seek, etc. here we don't copy from a file but piping data to the dd process, therefore there's no meaning to use that method. the resuse here is to use the utils.execCmd verb like we do execute dd by from other places when needed, we need a specific dd here. Line 77: try: Line 78: while bytes_left > 0: Line 79: to_read = min(BUFFER_SIZE, bytes_left) Line 80:
Line 109: p.returncode, p.stderr.read(1000)) Line 110: raise se.MiscFileWriteException() Line 111: Line 112: except Exception: Line 113: if p.returncode is None:
please do. i read the code and it doesn't. see in class AsyncProc in utils.
see there @property def returncode(self): Line 114: p.kill() Line 115: raise Line 116: Line 117: