Nir Soffer
2018-Jun-14 18:16 UTC
[Libguestfs] [PATCH] v2v: -o rhv-upload: Optimize http request sending
When sending request with small or no payload, it is simpler and possibly more efficient to use the high level HTTPSConnection.request(), instead of the lower level APIs. The only reason to use the lower level APIs is to avoid copying the payload, or on python 2, to use a bigger buffer size when streaming a file-like object. --- v2v/rhv-upload-plugin.py | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index ed99cc7a9..3fad865f6 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -263,12 +263,12 @@ def pread(h, count, offset): transfer = h['transfer'] transfer_service = h['transfer_service'] - http.putrequest("GET", h['path']) + headers = {"Range", "bytes=%d-%d" % (offset, offset+count-1)} # Authorization is only needed for old imageio. if h['needs_auth']: - http.putheader("Authorization", transfer.signed_ticket) - http.putheader("Range", "bytes=%d-%d" % (offset, offset+count-1)) - http.endheaders() + headers["Authorization"] = transfer.signed_ticket + + http.request("GET", h['path'], headers=headers) r = http.getresponse() # 206 = HTTP Partial Content. @@ -319,11 +319,10 @@ def zero(h, count, offset, may_trim): 'size': count, 'flush': False}).encode() - http.putrequest("PATCH", h['path']) - http.putheader("Content-Type", "application/json") - http.putheader("Content-Length", len(buf)) - http.endheaders() - http.send(buf) + headers = {"Content-Type": "application/json", + "Content-Length", str(len(buf))} + + http.request("PATCH", h['path'], body=buf, headers=headers) r = http.getresponse() if r.status != 200: @@ -368,11 +367,10 @@ def trim(h, count, offset): 'size': count, 'flush': False}).encode() - http.putrequest("PATCH", h['path']) - http.putheader("Content-Type", "application/json") - http.putheader("Content-Length", len(buf)) - http.endheaders() - http.send(buf) + headers = {"Content-Type": "application/json", + "Content-Length", str(len(buf))} + + http.request("PATCH", h['path'], body=buf, headers=headers) r = http.getresponse() if r.status != 200: @@ -387,11 +385,10 @@ def flush(h): # Construct the JSON request for flushing. buf = json.dumps({'op': "flush"}).encode() - http.putrequest("PATCH", h['path']) - http.putheader("Content-Type", "application/json") - http.putheader("Content-Length", len(buf)) - http.endheaders() - http.send(buf) + headers = {"Content-Type": "application/json", + "Content-Length", str(len(buf))} + + http.request("PATCH", h['path'], body=buf, headers=headers) r = http.getresponse() if r.status != 200: -- 2.17.1
Nir Soffer
2018-Jun-14 18:24 UTC
Re: [Libguestfs] [PATCH] v2v: -o rhv-upload: Optimize http request sending
On Thu, Jun 14, 2018 at 9:16 PM Nir Soffer <nirsof@gmail.com> wrote:> When sending request with small or no payload, it is simpler and > possibly more efficient to use the high level HTTPSConnection.request(), > instead of the lower level APIs. > > The only reason to use the lower level APIs is to avoid copying the > payload, or on python 2, to use a bigger buffer size when streaming a > file-like object. > --- > v2v/rhv-upload-plugin.py | 35 ++++++++++++++++------------------- > 1 file changed, 16 insertions(+), 19 deletions(-) > > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py > index ed99cc7a9..3fad865f6 100644 > --- a/v2v/rhv-upload-plugin.py > +++ b/v2v/rhv-upload-plugin.py > @@ -263,12 +263,12 @@ def pread(h, count, offset): > transfer = h['transfer'] > transfer_service = h['transfer_service'] > > - http.putrequest("GET", h['path']) > + headers = {"Range", "bytes=%d-%d" % (offset, offset+count-1)} > # Authorization is only needed for old imageio. > if h['needs_auth']: > - http.putheader("Authorization", transfer.signed_ticket) > - http.putheader("Range", "bytes=%d-%d" % (offset, offset+count-1)) > - http.endheaders() > + headers["Authorization"] = transfer.signed_ticket > + > + http.request("GET", h['path'], headers=headers) > > r = http.getresponse() > # 206 = HTTP Partial Content. > @@ -319,11 +319,10 @@ def zero(h, count, offset, may_trim): > 'size': count, > 'flush': False}).encode() > > - http.putrequest("PATCH", h['path']) > - http.putheader("Content-Type", "application/json") > - http.putheader("Content-Length", len(buf)) > - http.endheaders() > - http.send(buf) > + headers = {"Content-Type": "application/json", > + "Content-Length", str(len(buf))} > + > + http.request("PATCH", h['path'], body=buf, headers=headers) > > r = http.getresponse() > if r.status != 200: > @@ -368,11 +367,10 @@ def trim(h, count, offset): > 'size': count, > 'flush': False}).encode() > > - http.putrequest("PATCH", h['path']) > - http.putheader("Content-Type", "application/json") > - http.putheader("Content-Length", len(buf)) > - http.endheaders() > - http.send(buf) > + headers = {"Content-Type": "application/json", > + "Content-Length", str(len(buf))} > + > + http.request("PATCH", h['path'], body=buf, headers=headers) > > r = http.getresponse() > if r.status != 200: > @@ -387,11 +385,10 @@ def flush(h): > # Construct the JSON request for flushing. > buf = json.dumps({'op': "flush"}).encode() > > - http.putrequest("PATCH", h['path']) > - http.putheader("Content-Type", "application/json") > - http.putheader("Content-Length", len(buf)) > - http.endheaders() > - http.send(buf) > + headers = {"Content-Type": "application/json", > + "Content-Length", str(len(buf))} > + > + http.request("PATCH", h['path'], body=buf, headers=headers) > > r = http.getresponse() > if r.status != 200: > -- > 2.17.1 >WARNING: this is not tested, but I tested similar code here: https://github.com/oVirt/ovirt-imageio/blob/master/examples/upload (which need the same change). Do we have an easy way to test the plugin without running the entire virt-v2v pipeline? Ideally, a way to run nbdkit with the plugin against an existing ovirt system, and then use qemu-img to convert a file to the nbdkit nbd socket. Nir
Richard W.M. Jones
2018-Jun-14 19:18 UTC
Re: [Libguestfs] [PATCH] v2v: -o rhv-upload: Optimize http request sending
On Thu, Jun 14, 2018 at 09:16:01PM +0300, Nir Soffer wrote:> When sending request with small or no payload, it is simpler and > possibly more efficient to use the high level HTTPSConnection.request(), > instead of the lower level APIs. > > The only reason to use the lower level APIs is to avoid copying the > payload, or on python 2, to use a bigger buffer size when streaming a > file-like object.Change seems quite straightforward. I'll have to test this out on my own oVirt system first before I can properly verify it. Thanks, Rich.> v2v/rhv-upload-plugin.py | 35 ++++++++++++++++------------------- > 1 file changed, 16 insertions(+), 19 deletions(-) > > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py > index ed99cc7a9..3fad865f6 100644 > --- a/v2v/rhv-upload-plugin.py > +++ b/v2v/rhv-upload-plugin.py > @@ -263,12 +263,12 @@ def pread(h, count, offset): > transfer = h['transfer'] > transfer_service = h['transfer_service'] > > - http.putrequest("GET", h['path']) > + headers = {"Range", "bytes=%d-%d" % (offset, offset+count-1)} > # Authorization is only needed for old imageio. > if h['needs_auth']: > - http.putheader("Authorization", transfer.signed_ticket) > - http.putheader("Range", "bytes=%d-%d" % (offset, offset+count-1)) > - http.endheaders() > + headers["Authorization"] = transfer.signed_ticket > + > + http.request("GET", h['path'], headers=headers) > > r = http.getresponse() > # 206 = HTTP Partial Content. > @@ -319,11 +319,10 @@ def zero(h, count, offset, may_trim): > 'size': count, > 'flush': False}).encode() > > - http.putrequest("PATCH", h['path']) > - http.putheader("Content-Type", "application/json") > - http.putheader("Content-Length", len(buf)) > - http.endheaders() > - http.send(buf) > + headers = {"Content-Type": "application/json", > + "Content-Length", str(len(buf))} > + > + http.request("PATCH", h['path'], body=buf, headers=headers) > > r = http.getresponse() > if r.status != 200: > @@ -368,11 +367,10 @@ def trim(h, count, offset): > 'size': count, > 'flush': False}).encode() > > - http.putrequest("PATCH", h['path']) > - http.putheader("Content-Type", "application/json") > - http.putheader("Content-Length", len(buf)) > - http.endheaders() > - http.send(buf) > + headers = {"Content-Type": "application/json", > + "Content-Length", str(len(buf))} > + > + http.request("PATCH", h['path'], body=buf, headers=headers) > > r = http.getresponse() > if r.status != 200: > @@ -387,11 +385,10 @@ def flush(h): > # Construct the JSON request for flushing. > buf = json.dumps({'op': "flush"}).encode() > > - http.putrequest("PATCH", h['path']) > - http.putheader("Content-Type", "application/json") > - http.putheader("Content-Length", len(buf)) > - http.endheaders() > - http.send(buf) > + headers = {"Content-Type": "application/json", > + "Content-Length", str(len(buf))} > + > + http.request("PATCH", h['path'], body=buf, headers=headers) > > r = http.getresponse() > if r.status != 200: > -- > 2.17.1-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Nir Soffer
2018-Jun-14 23:21 UTC
Re: [Libguestfs] [PATCH] v2v: -o rhv-upload: Optimize http request sending
On Thu, Jun 14, 2018 at 10:19 PM Richard W.M. Jones <rjones@redhat.com> wrote:> On Thu, Jun 14, 2018 at 09:16:01PM +0300, Nir Soffer wrote: > > When sending request with small or no payload, it is simpler and > > possibly more efficient to use the high level HTTPSConnection.request(), > > instead of the lower level APIs. > > > > The only reason to use the lower level APIs is to avoid copying the > > payload, or on python 2, to use a bigger buffer size when streaming a > > file-like object. > > Change seems quite straightforward. I'll have to test this out on my > own oVirt system first before I can properly verify it. >I tested the same change with imageio example upload script, and it *doubled* the throughput. See https://gerrit.ovirt.org/#/c/92275/ Thanks, Rich.> > > v2v/rhv-upload-plugin.py | 35 ++++++++++++++++------------------- > > 1 file changed, 16 insertions(+), 19 deletions(-) > > > > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py > > index ed99cc7a9..3fad865f6 100644 > > --- a/v2v/rhv-upload-plugin.py > > +++ b/v2v/rhv-upload-plugin.py > > @@ -263,12 +263,12 @@ def pread(h, count, offset): > > transfer = h['transfer'] > > transfer_service = h['transfer_service'] > > > > - http.putrequest("GET", h['path']) > > + headers = {"Range", "bytes=%d-%d" % (offset, offset+count-1)} > > # Authorization is only needed for old imageio. > > if h['needs_auth']: > > - http.putheader("Authorization", transfer.signed_ticket) > > - http.putheader("Range", "bytes=%d-%d" % (offset, offset+count-1)) > > - http.endheaders() > > + headers["Authorization"] = transfer.signed_ticket > > + > > + http.request("GET", h['path'], headers=headers) > > > > r = http.getresponse() > > # 206 = HTTP Partial Content. > > @@ -319,11 +319,10 @@ def zero(h, count, offset, may_trim): > > 'size': count, > > 'flush': False}).encode() > > > > - http.putrequest("PATCH", h['path']) > > - http.putheader("Content-Type", "application/json") > > - http.putheader("Content-Length", len(buf)) > > - http.endheaders() > > - http.send(buf) > > + headers = {"Content-Type": "application/json", > > + "Content-Length", str(len(buf))} > > + > > + http.request("PATCH", h['path'], body=buf, headers=headers) > > > > r = http.getresponse() > > if r.status != 200: > > @@ -368,11 +367,10 @@ def trim(h, count, offset): > > 'size': count, > > 'flush': False}).encode() > > > > - http.putrequest("PATCH", h['path']) > > - http.putheader("Content-Type", "application/json") > > - http.putheader("Content-Length", len(buf)) > > - http.endheaders() > > - http.send(buf) > > + headers = {"Content-Type": "application/json", > > + "Content-Length", str(len(buf))} > > + > > + http.request("PATCH", h['path'], body=buf, headers=headers) > > > > r = http.getresponse() > > if r.status != 200: > > @@ -387,11 +385,10 @@ def flush(h): > > # Construct the JSON request for flushing. > > buf = json.dumps({'op': "flush"}).encode() > > > > - http.putrequest("PATCH", h['path']) > > - http.putheader("Content-Type", "application/json") > > - http.putheader("Content-Length", len(buf)) > > - http.endheaders() > > - http.send(buf) > > + headers = {"Content-Type": "application/json", > > + "Content-Length", str(len(buf))} > > + > > + http.request("PATCH", h['path'], body=buf, headers=headers) > > > > r = http.getresponse() > > if r.status != 200: > > -- > > 2.17.1 > > -- > Richard Jones, Virtualization Group, Red Hat > http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-top is 'top' for virtual machines. Tiny program with many > powerful monitoring features, net stats, disk stats, logging, etc. > http://people.redhat.com/~rjones/virt-top >
Tomáš Golembiovský
2018-Jun-14 23:56 UTC
Re: [Libguestfs] [PATCH] v2v: -o rhv-upload: Optimize http request sending
On Thu, 14 Jun 2018 21:16:01 +0300 Nir Soffer <nirsof@gmail.com> wrote:> When sending request with small or no payload, it is simpler and > possibly more efficient to use the high level HTTPSConnection.request(), > instead of the lower level APIs. > > The only reason to use the lower level APIs is to avoid copying the > payload, or on python 2, to use a bigger buffer size when streaming a > file-like object. > --- > v2v/rhv-upload-plugin.py | 35 ++++++++++++++++------------------- > 1 file changed, 16 insertions(+), 19 deletions(-)FWIW LGTM
Richard W.M. Jones
2018-Jun-18 10:37 UTC
Re: [Libguestfs] [PATCH] v2v: -o rhv-upload: Optimize http request sending
On Thu, Jun 14, 2018 at 09:24:48PM +0300, Nir Soffer wrote:> On Thu, Jun 14, 2018 at 9:16 PM Nir Soffer <nirsof@gmail.com> wrote: > > + headers = {"Content-Type": "application/json", > > + "Content-Length", str(len(buf))}There were a few Python syntax errors such as this one. They can be found by running: make -C v2v check TESTS=test-v2v-python-syntax.sh However I have now fixed them.> Do we have an easy way to test the plugin without running the entire > virt-v2v pipeline?Not really. In fact we don't have any unit tests for -o rhv-upload functionality because there's no way to simulate the imageio server. However you can run virt-v2v locally against an oVirt instance without needing VMware. The command is rather lengthy, but here it is: $ virt-builder fedora-27 $ ./run virt-v2v -i disk /var/tmp/fedora-27.img \ -o rhv-upload \ -oc https://ovirt-engine.example.com/ovirt-engine/api \ -os ovirt-data \ -op /tmp/password \ -of raw \ -oo rhv-cafile=/tmp/ca.pem \ -oo rhv-direct /tmp/password should contain the oVirt admin password. /tmp/ca.pem should contain the oVirt CA cert. This will create a guest called ‘fedora-27’ which you'll need to delete (on oVirt) afterwards. You can add ‘-on name’ to name it something else. In any case I have fixed and verified this patch and will push it soon, thanks. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Seemingly Similar Threads
- Re: [PATCH] v2v: -o rhv-upload: Optimize http request sending
- [PATCH] v2v: -o rhv-upload: Optimize http request sending
- Re: [PATCH] v2v: -o rhv-upload: Optimize http request sending
- [PATCH 0/2] rhv-upload: Complete refactoring
- Re: [PATCH] v2v: -o rhv-upload: Optimize http request sending