[Libguestfs] [PATCH v2 3/3] v2v: -o rhv-upload: Add a test.

[Nir Soffer]

On Mon, Oct 15, 2018 at 6:21 PM Richard W.M. Jones <rjones at redhat.com>
wrote:

> On Tue, Oct 09, 2018 at 02:28:10PM +0300, Nir Soffer wrote:
> > > +# Create a background thread running a web server which is
> > > +# simulating the imageio server.
> > >
> >
> > This functionality should be separated from the fake SDK module, since
> it is
> > not part of the SDK, and may be replaced by real imageio server later.
>
> Well possibly, but it was very convenient to put it in the class here,
> and this test is meant for running completely standalone without any
> other service available.
>
> > > +class RequestHandler(BaseHTTPRequestHandler):
> > >
> >
> > This request handler is using HTTP/1.0, and will close the connection
> after
> > every request. This is not a good implementation of the imageio server,
> and
> > also
> > hides bugs in this code.
> >
> > Should be fixed by adding:
> >
> >     protocol_version = "HTTP/1.1"
>
> I tried the attached patch, but for some reason it just hangs.
>

The issue is probably missing "content-length: 0" header in the
response. If we don't close the connection and don't send
content-length the client cannot do much but wait :-)

> > +    def do_OPTIONS(self):
> >
> > +        self.send_response(200)
> > > +        self.send_header("Content-type", "application/json;
> > > charset=UTF-8")
> > > +        self.end_headers()
> > > +        # Advertize only zero support.
> > > +        self.wfile.write(b'''{ "features": [ "zero" ] }''')
> > > +
> > > +    # eg. zero request.  Just ignore it.
> > > +    def do_PATCH(self):
> > >
> >
> > This must read content-length bytes from self.rfile.
> >
> >
> > > +        self.send_response(200)
> > > +        self.end_headers()
> > > +
> > > +    # Flush request.  Ignore it.
> > > +    def do_PUT(self):
> > >
> >
> > This must read content-length bytes from self.rfile.
> >
> > Both bugs are hidden by the fact that this server close the connection at
> > the end
> > of the request.
>
> Also this is covered by the attached patch.
>
> > > +        self.send_response(200)
> > > +        self.end_headers()
> > > +
> > > +def server():
> > > +    server_address = ("", imageio_port)
> > > +    httpd = HTTPServer(server_address, RequestHandler)
> > > +    httpd.serve_forever()
> > >
> >
> > Using HTTP instead of HTTPS is not a good idea. We are not testing
> > the same code on the client side.
> >
> > It is easy to run HTTPS server using pre-created certificate, if we
> > disable certificate verification on the client side
> > (e.g. --insecure).
>
> This one doesn't seem to be as simple to fix.  However point taken, so
> I added a note in the code.
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20181015/c502b03b/attachment.htm>