Change in vdsm[master]: introducing capabillity to stream data to image
Federico Simoncelli
fsimonce at redhat.com
Wed Feb 12 13:56:45 UTC 2014
Federico Simoncelli 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 3: AuthorDate: 2014-01-12 13:52:24 +0200
Line 4: Commit: Liron Aravot <laravot at redhat.com>
Line 5: CommitDate: 2014-02-09 15:08:28 +0200
Line 6:
Line 7: introducing capabillity to stream data to image
typo: capability
Line 8:
Line 9: This patch introduces the a new capabaillity to vdsm
Line 10: which allows to upload using streaming content to vdsm
Line 11: images.
Line 5: CommitDate: 2014-02-09 15:08:28 +0200
Line 6:
Line 7: introducing capabillity to stream data to image
Line 8:
Line 9: This patch introduces the a new capabaillity to vdsm
typo: capability
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 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 capabillity to stream data to image,
typo: capability
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
http://gerrit.ovirt.org/#/c/23281/9/vdsm/BindingXMLRPC.py
File vdsm/BindingXMLRPC.py:
Line 177:
Line 178: spUUID = self.headers.getheader(self.HEADER_POOL)
Line 179: sdUUID = self.headers.getheader(self.HEADER_DOMAIN)
Line 180: imgUUID = self.headers.getheader(self.HEADER_IMAGE)
Line 181: volUUID = self.headers.getheader(self.HEADER_VOLUME)
We generally validate these when received from the client. We should at least check that they're provided (except volUUID probably).
Line 182:
Line 183: uploadFinishedEvent = threading.Event()
Line 184:
Line 185: def upload_finished():
Line 187:
Line 188: methodArgs = {'method': 'stream',
Line 189: 'fileObj': self.rfile,
Line 190: 'callback': upload_finished,
Line 191: 'contentLength': contentLength}
methodArgs is an external API that shouldn't be stretched to our internal purposes.
I prefer the previous downloadFromStream method implementation (one of the previous abandoned patches) for two reasons:
* there's nothing that prevents a client from using "stream" from the regular upload/download API (which is wrong, it just exposes something potentially dangerous)
* also the additional "callback" and "contentLength" values are dangerous and shouldn't be exposed in the API.
Line 192: image = API.Image(imgUUID, spUUID, sdUUID)
Line 193: response = image.download(methodArgs, volUUID)
Line 194:
Line 195: while not uploadFinishedEvent.is_set():
http://gerrit.ovirt.org/#/c/23281/9/vdsm/storage/image.py
File vdsm/storage/image.py:
Line 1164: domain.deactivateImage(imgUUID)
Line 1165:
Line 1166: def download(self, methodArgs, sdUUID, imgUUID, volUUID=None):
Line 1167: try:
Line 1168: self._download(methodArgs, sdUUID, imgUUID, volUUID=None)
volUUID=None ?
Anyway I think this shouldn't reuse the old download. It should be the new downloadFromStream.
Line 1169: finally:
Line 1170: callback = imageSharing.getCallback(methodArgs)
Line 1171: if callback is not None:
Line 1172: callback()
--
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 <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