On Thu, Jul 9, 2020 at 10:53 AM Richard W.M. Jones <rjones(a)redhat.com> wrote:
On Thu, Jul 09, 2020 at 01:51:44AM +0300, Nir Soffer wrote:
> We can use now ImageioClient to communicate with ovirt-imageio server
> on oVirt host.
>
> Using the client greatly simplifies the plugin, and enables new features
> like transparent proxy support. The client will use transfer_url if
> possible, or fall back to proxy_url.
>
> Since the client implements the buffer protocol, move to version 2 of
> the API for more efficient pread().
This will require nbdkit >= 1.17.3 which implies RHEL AV 8.3.0, but
that's fine since we only ship the new virt-v2v (>= 1.42) on AV 8.3.0.
I'll update the spec if it is in the source then.
> Another advantage the client is maintained by oVirt, and fixes
are
> delivered quickly in oVirt, without depending on RHEL release schedule.
>
> Not ready yet, we have several issues:
>
> - The client does not support "http", so the tests will fail now. This
> is good since we should test with real imageio server. I will work on
> better tests later.
I think having standalone tests is still worthwhile as it's the only
way that the plugin gets tested on a regular basis. IIRC at the
moment we only test against a faked ovirt SDK. I guess this would be
easy to adjust?
Testing with a real server is easy. I have incomplete patch using
imageio server with some manual setup.
This is the setup needed:
$ git clone
https://github.com/oVirt/ovirt-imageio.git
In another shell, start the server:
$ cd ovirt-imageio/daemon
$ ./ovirt-imageio -c test
2020-07-09 02:55:57,839 INFO (MainThread) [server] Starting
(pid=200640, version=2.0.10)
2020-07-09 02:55:57,842 INFO (MainThread) [services] remote.service
listening on ('::', 54322)
2020-07-09 02:55:57,842 INFO (MainThread) [services] local.service
listening on '\x00/org/ovirt/imageio'
2020-07-09 02:55:57,843 INFO (MainThread) [services]
control.service listening on 'test/daemon.sock'
2020-07-09 02:55:57,844 INFO (MainThread) [server] Ready for requests
To make uploads possible, we need to instal a ticket matching the
transfer_url:
$ curl --unix-socket ovirt-imageio/daemon/test/daemon.sock \
-X PUT \
--upload-file tests/test-v2v-o-rhv-upload-module/file.json \
http://localhost/tickets/test
Finally we need to create the target image:
qemu-img create -f raw /var/tmp/test.raw 512m
With this we can run virt-v2v, and since we use a real server, we can
check that the image was imported correctly:
qemu-img compare overlay.qcow2 /var/tmp/test.raw
To test imageio ndb support, need to import to qcow2 disks, we need to
install the nbd ticket:
$ curl --unix-socket ovirt-imageio/daemon/test/daemon.sock \
-X PUT \
--upload-file tests/test-v2v-o-rhv-upload-module/nbd.json \
http://localhost/tickets/test
Now we can create qcow2 target image:
qemu-img create -f qcow2 /var/tmp/test.qcow2 512m
And run also qemu-nbd:
$ qemu-nbd --socket=/tmp/nbd.sock \
--persistent \
--shared=8 \
--format=qcow2 \
--aio=native \
--cache=none \
--discard=unmap \
/var/tmp/test.qcow2
With this manual setup, the rhv-upload test works for both raw and qcow2
target disks.
$ cat tests/test-v2v-o-rhv-upload-module/file.json
{
"uuid": "test",
"size": 536870912,
"url": "file:///var/tmp/test.raw",
"timeout": 3000,
"sparse": true,
"ops": ["write"]
}
$ cat tests/test-v2v-o-rhv-upload-module/nbd.json
{
"uuid": "test",
"size": 536870912,
"url": "nbd:unix:/tmp/nbd.sock",
"timeout": 3000,
"sparse": true,
"ops": ["write"]
}
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.
I can start with replacing the fake server with fake client, and document
somewhere how to test using a real server.
To unit test against a real imageio server is difficult I think:
These
tests would need to run in Fedora with minimal large dependencies. We
might create a fake web server like the one we use in nbdkit:
https://github.com/libguestfs/nbdkit/blob/master/tests/web-server.h
https://github.com/libguestfs/nbdkit/blob/master/tests/web-server.c
This only supports http but we could put stunnel in front to provide
https.
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.
> - 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.
I can try to add imageio to Fedora, this is a good idea anyway. We don't
have any dependencies on other oVirt components now.
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?
> - 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?
[...]
> -# 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...
Rest of the patch looks good and as you say above both simplifies and
improves performance.
The performance part was not tested yet. I will results in the next version.
Nir