Nir Soffer has posted comments on this change.
Change subject: introducing downloadFromStream verb ......................................................................
Patch Set 8:
(7 comments)
This patch is even nicer now - less code and more correct.
Needs little cleanup in the commit message and the comments about constants that Yaniv asked in the previous patch set.
http://gerrit.ovirt.org/#/c/23281/8//COMMIT_MSG Commit Message:
Line 3: AuthorDate: 2014-01-12 13:52:24 +0200 Line 4: Commit: Liron Aravot laravot@redhat.com Line 5: CommitDate: 2014-01-29 14:06:14 +0200 Line 6: Line 7: introducing downloadFromStream verb Seems that this is wrong now, as we don't have new verb now.
So this should describe now the new feature, like:
Add upload to image feature Line 8: Line 9: This patch introduces the downloadFromStream verb in vdsm Line 10: which allows to upload using streaming content to vdsm Line 11: images.
Line 5: CommitDate: 2014-01-29 14:06:14 +0200 Line 6: Line 7: introducing downloadFromStream verb Line 8: Line 9: This patch introduces the downloadFromStream verb in vdsm There is no such verb now. Line 10: which allows to upload using streaming content to vdsm Line 11: images. Line 12: Line 13: Previously we sent ovf data using XMLRPC (UpdateVM verb),
Line 7: introducing downloadFromStream verb Line 8: Line 9: This patch introduces the downloadFromStream verb in vdsm Line 10: which allows to upload using streaming content to vdsm Line 11: images. The previous text explained this better: http://gerrit.ovirt.org/#/c/23281/7//COMMIT_MSG Line 12: Line 13: Previously we sent ovf data using XMLRPC (UpdateVM verb), Line 14: which limits the size of the data, having to encode the Line 15: payload into the xml, and make it hard and inefficient
Line 14: which limits the size of the data, having to encode the Line 15: payload into the xml, and make it hard and inefficient Line 16: to upload lot of data and store it on some image. Line 17: Line 18: This patch adds the downloadFromStream internal verb, No such verb now Line 19: allowing efficient upload of data in any size and format Line 20: and storing it directly on an Line 21: image. As the XML-RPC spec doesn't support streaming and Line 22: to avoid requiring another port by using dedicated http
Line 17: Line 18: This patch adds the downloadFromStream internal verb, Line 19: allowing efficient upload of data in any size and format Line 20: and storing it directly on an Line 21: image. As the XML-RPC spec doesn't support streaming and We already discussed that in previous patche sets. There is no difference between sending xmlrpc payload and sending data using pure http. Both stream data to the server in the same way.
What makes xmlrpc less useful for uploading data is the need to encode the data into the xml, and the need to parse the xml containing the data. The problem is not streaming support in xmlrpc. Line 22: to avoid requiring another port by using dedicated http Line 23: server, in this patch we use the existing xmlrpc server to Line 24: handle upload requests. Line 25:
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. Maybe explain that having a task protect from stopping the spm by the engine. Line 35: Line 36: The PUT request inspected headers are: Line 37: -- Mandatory headers: Line 38: *content-length
http://gerrit.ovirt.org/#/c/23281/8/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 Lets add comments as Yaniv asked:
# Time to wait from finishing writing data to dd, until dd exists, # Ensure that we don't keep the task active forever if dd cannot # access the storage. WAIT_TIMEOUT = 30
# Number of bytes to read from the socket and write # to dd stdin trough the pipe. Based on default socket buffer # size(~80KB) and default pipe buffer size (64K), this should # minimize system call overhead without consuming too much # memory. BUFFER_SIZE = 65536 Line 33: Line 34: Line 35: def httpGetSize(methodArgs): Line 36: headers = curlImgWrap.head(methodArgs.get('url'),