On Thu, Jul 9, 2020 at 4:49 PM Nir Soffer <nsof...@redhat.com> wrote: > > On Thu, Jul 9, 2020 at 4:12 PM Richard W.M. Jones <rjo...@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
This includes now also aarch64: https://copr.fedorainfracloud.org/coprs/nsoffer/ovirt-imageio-preview/build/1541540/ Do you think this is good enough for introducing this change upstream? Having to enable copr repo to import vms from Fedora machine? > 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 > > _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
