[Libguestfs] [PATCH] RFC: rhv-upload-plugin: Use imageio client

[Richard W.M. Jones]

On Thu, Jul 09, 2020 at 03:30:22PM +0300, Nir Soffer wrote:
> Testing with a real server is easy. I have incomplete patch using
> imageio server with some manual setup.
[...]

For a certain definition of "easy" :-)

Our QE team tests -o rhv-upload from time to time, and will do an
end-to-end test of the system when RHV 4.4 arrives with the new
feature.

For “make check” we need something that will run in a few seconds and
doesn't have any major dependencies.

> If we want to test with a fake, we can create fake client - no need for runing
> http sever:
> 
> class FakeImageioClient:
> 
>     def write(self, offset, buf):
>         pass
> 
>     ...
> 
> This is easy, but we don't test much.

Right, this is what I was thinking of for “make check”.  While it's
not a comprehensive test, it is still testing something valuable:
whether the Python code can compile and run from beginning to end
without runtime errors.

> I don't think this is the right approach. We already failed last time when
> we added the fake http server  the tests passed but the change to support
> them broke the real code.
>
> Wrong tests are the worst.

Agreed.

> > > - Need to require ovirt-imageio-client, providing the client library.
> >
> > That's a simple change in virt-v2v packaging.  I don't see this
> > package in Fedora Koji.
> 
> Is this an issue? oVirt is not really supported on Fedora these days.

No it's not an issue, but the feature will not work on Fedora.  A bit
surprised though - wouldn't you want people to be able to upload disk
images to oVirt from their Fedora clients?

...
> > In RHEL I can see the package and the
> > dependencies look quite light, basically just Python and python3-six.
> > Why is it only available for x86_64 and ppc64le?
> 
> Probably because RHV is not built for other arches.
> 
> Is this an issue? e.g. do we support importing vms in virt-v2v runing on
> arm, importing to oVirt/RHV running on x86/ppc?

Virt-v2v is only built for x86-64 in RHEL 8, so it's not really an issue.

I was more wondering if there was some problem that prevented it from
working on the other architectures thinking about the Fedora case, but
if we're not going to package this up in Fedora it doesn't matter.

> > > - params['rhv_direct'] is ignored, we always try direct transfer now.
> >
> > We should drop it from the OCaml code?
> 
> And break users that used this flag?

Actually I meant just drop the params['rhv_direct'] field, ie:
https://github.com/libguestfs/virt-v2v/blob/784be60842d088596d7af938f90c689083677dca/v2v/output_rhv_upload.ml#L191

As you say we do need to preserve the command line option, but it'll
be ignored.

> > [...]
> >
> > > -# Modify http.client.HTTPConnection to work over a Unix domain socket.
> > > -# Derived from uhttplib written by Erik van Zijst under an MIT license.
> > > -# (https://pypi.org/project/uhttplib/)
> > > -# Ported to Python 3 by Irit Goihman.
> > > -
> > > -
> > > -class UnixHTTPConnection(HTTPConnection):
> >
> > Why drop this part?
> 
> Not used now, imageio client includes this class:
> https://github.com/oVirt/ovirt-imageio/blob/24c59f2e0ace784d9c993f6044475bb370058e70/daemon/ovirt_imageio/_internal/backends/http.py#L610

Good thing.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW