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

[Nir Soffer]

On Thu, Jul 9, 2020 at 4:12 PM Richard W.M. Jones <rjones at redhat.com> wrote:
>
> 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.

So I'll create a fake client, and document how to test with a real server.

We can later try to do something smarter, like run the real server only if
you made it available, or added some environment variable.

> > 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?

Sure I do, this is a major issue with this change.

Currently imageio is available via:
copr: https://copr.fedorainfracloud.org/coprs/nsoffer/ovirt-imageio-preview/
    fedora, centos, centos-stream
ovit repositories: https://resources.ovirt.org/pub/ovirt-4.4/
     el8/x86_64/ppc64le

I'll try to get this into Fedora.

> ...
> > > 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.

We never tried to build on other arches, but it should work. We have
few lines of C but they should be portable.
https://github.com/oVirt/ovirt-imageio/blob/master/daemon/ovirt_imageio/_internal/ioutil.c

I added aarch64 builds in copr, lets see how it goes:
https://copr.fedorainfracloud.org/coprs/nsoffer/ovirt-imageio-preview/build/1541540/

> > > > - 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.

Sounds good.

> > > [...]
> > >
> > > > -# 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
>