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

On Thu, Jul 9, 2020 at 4:12 PM Richard W.M. Jones <rjones(a)redhat.com&gt; 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/_... I added aarch64 builds in copr, lets see how it goes: https://copr.fedorainfracloud.org/coprs/nsoffer/ovirt-imageio-preview/bui... > > > > - 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/784be60842d088596d7af938f90c6... > > 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/24c59f2e0ace784d9c993f6044475... > > 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 >