Nir Soffer has posted comments on this change. Change subject: introducing capabillity to stream data to image ......................................................................
Patch Set 9: (6 comments) http://gerrit.ovirt.org/#/c/23281/9//COMMIT_MSG Commit Message: Line 30: today for request handling ('/', '/RPC2') will be treated Line 31: as upload requests. Line 32: The upload itself is being executed within a task, that's Line 33: needed to indicate that there's an operation executed by Line 34: the host. > Liron, Nir: I did not understand the spm-ness issue: beyond the problem of The purpose of the upload/download task is to prevent spm replacement during the operation. When and pool has tasks on the vdsm side, the engine will not let you stop the spm before the task are completed. We can probably remove this requirement if the engine manage a persistent lock for the ovf image, so only one host is permitted to write to this image. Until we have such infrastructure, the SPM lease is used to synchronize access to the ovf image. This text can probably used in the commit message. Line 35: Line 36: The PUT request inspected headers are: Line 37: -- Mandatory headers: Line 38: *content-length Line 40: -- Mandatory custom headers: Line 41: *X-Storage-Pool-Id : mandatory for succesfull execution Line 42: *X-Storage-Domain-Id: mandatory for succesfull execution Line 43: *X-Image-Id: mandatory for succesfull execution Line 44: *X-Volume-Id: mandatory for succesfull execution Lets remove the details from the commit message, and put them were they belong, in the do_PUT docstring. Line 45: Line 46: Change-Id: I768b84799ed9fb2769c6d4240519d036f8988b99 http://gerrit.ovirt.org/#/c/23281/9/vdsm/BindingXMLRPC.py File vdsm/BindingXMLRPC.py: Line 137: # timeout for the request socket Line 138: timeout = 60 Line 139: log = logging.getLogger("BindingXMLRPC.RequestHandler") Line 140: Line 141: HEADER_POOL = 'X-Storage-Pool-Id' > Deprecating the "X-" Prefix and Similar Constructs in Application Protocols +1 for removing the "X-" prefix. Line 142: HEADER_DOMAIN = 'X-Storage-Domain-Id' Line 143: HEADER_IMAGE = 'X-Image-Id' Line 144: HEADER_VOLUME = 'X-Volume-Id' Line 145: HEADER_CONTENT_LENGTH = 'content-length' Line 189: 'fileObj': self.rfile, Line 190: 'callback': upload_finished, Line 191: 'contentLength': contentLength} Line 192: image = API.Image(imgUUID, spUUID, sdUUID) Line 193: response = image.download(methodArgs, volUUID) > download() seems synchronous to me - why is it needed to have a callback? download is scheduling a task in hsm and return immediately with a task id. Line 194: Line 195: while not uploadFinishedEvent.is_set(): Line 196: uploadFinishedEvent.wait() Line 197: http://gerrit.ovirt.org/#/c/23281/9/vdsm/storage/imageSharing.py File vdsm/storage/imageSharing.py: Line 18: # Line 19: Line 20: import logging Line 21: import curlImgWrap Line 22: import signal > please import stlib modules first, and Vdsm constructs only later. This is unrelated to this patch, can be cleanup later to keep this patch minimal. Line 23: import socket Line 24: Line 25: import storage_exception as se Line 26: from vdsm import constants Line 100: p.stdin.write(data) Line 101: # Process stdin is not a real file object but a wrapper using Line 102: # StringIO buffer. To ensure that we don't use more memory if we Line 103: # get data faster then dd read it from the pipe, we flush on every Line 104: # write. We can remove if we can limit the buffer size used by > We can remove *flush()* Did you check the underlying file object wrapper? Line 105: # this stdin wrapper. Line 106: p.stdin.flush() Line 107: bytes_left = bytes_left - len(data) Line 108: -- To view, visit http://gerrit.ovirt.org/23281 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I768b84799ed9fb2769c6d4240519d036f8988b99 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Ar <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
