Change in vdsm[master]: introducing downloadFromStream verb

nsoffer at redhat.com nsoffer at redhat.com
Wed Jan 29 14:23:11 UTC 2014


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 at 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'),


-- 
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken 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: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list