[PATCH virt-v2v] v2v: -o rhv-upload: Enable multi-conn
Hi Nir, The experimental patch attached (for discussion) would enable multi-conn. For current virt-v2v this would have no effect since "qemu-img convert" does not make multi-conn NBD connections. However for modular virt-v2v[1] which uses nbdcopy this would allow nbdcopy to make multiple NBD connections to nbdkit running the plugin. (By default 4 connections). If I understand the rhv-upload-plugin code correctly, there is a pool per NBD connection at the moment, so this would create 4 pools and 4*4 backend connections to imageio. This is possibly not desirable, but I guess we can turn the pool into a global object. (Note each NBD connection has the same (destination_url, host, options) tuple). Naturally I've not actually tested any of this. There are some advantages from the nbdcopy side of things, especially because it is able to query extents on the input side in parallel which seems to be advantageous for inputs with slow extent querying (like VDDK). There may be further advantages on the output side because it would allow us to write data to the RHV upload plugin over four sockets, allowing better use of multiple cores. Rich. [1] https://github.com/rwmjones/virt-v2v/commits/2021-virt-v2v-split
--- v2v/rhv-upload-plugin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index a3d578176..51e9b33f7 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -169,6 +169,10 @@ def open(readonly): } +def can_multi_conn(h): + return True + + @failing def can_trim(h): return False -- 2.32.0
On Mon, Aug 02, 2021 at 08:41:20AM +0100, Richard W.M. Jones wrote:
--- v2v/rhv-upload-plugin.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index a3d578176..51e9b33f7 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -169,6 +169,10 @@ def open(readonly): }
+def can_multi_conn(h): + return True
Should this be h['can_flush'] instead of True? Are we guaranteed that imageio guarantees a consistent image across all other http connections when a flush is received on a single http connection? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Tue, Aug 03, 2021 at 02:20:01PM -0500, Eric Blake wrote:
On Mon, Aug 02, 2021 at 08:41:20AM +0100, Richard W.M. Jones wrote:
--- v2v/rhv-upload-plugin.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index a3d578176..51e9b33f7 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -169,6 +169,10 @@ def open(readonly): }
+def can_multi_conn(h): + return True
Should this be h['can_flush'] instead of True?
In the later version this is different(-ly bad): https://listman.redhat.com/archives/libguestfs/2021-August/msg00022.html
Are we guaranteed that imageio guarantees a consistent image across all other http connections when a flush is received on a single http connection?
That's a question for Nir (CC'd). However it's likely true anyway since even in the the current implementation it flushes all HTTP connections inside the nbdkit plugin: https://github.com/libguestfs/virt-v2v/blob/0d6d15ebdcad4a4df54f397cbc1b46fc... and I didn't change this behaviour in the proposed patches. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On Tue, Aug 3, 2021 at 10:26 PM Eric Blake <eblake@redhat.com> wrote:
On Mon, Aug 02, 2021 at 08:41:20AM +0100, Richard W.M. Jones wrote:
--- v2v/rhv-upload-plugin.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index a3d578176..51e9b33f7 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -169,6 +169,10 @@ def open(readonly): }
+def can_multi_conn(h): + return True
Should this be h['can_flush'] instead of True?
Are we guaranteed that imageio guarantees a consistent image across all other http connections when a flush is received on a single http connection?
imageio keeps multiple connections to qemu-nbd, and pass all requests to qemu-nbd. No caching is done in imageio. But we don't do any synchronization, so we may have one flush command in the middle of a write/write_zeros commands. Does qemu-nbd wait until the write/wirte_zeros commands complete before flushing?
On Tue, Aug 03, 2021 at 10:36:47PM +0300, Nir Soffer wrote:
On Tue, Aug 3, 2021 at 10:26 PM Eric Blake <eblake@redhat.com> wrote:
On Mon, Aug 02, 2021 at 08:41:20AM +0100, Richard W.M. Jones wrote:
--- v2v/rhv-upload-plugin.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index a3d578176..51e9b33f7 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -169,6 +169,10 @@ def open(readonly): }
+def can_multi_conn(h): + return True
Should this be h['can_flush'] instead of True?
Are we guaranteed that imageio guarantees a consistent image across all other http connections when a flush is received on a single http connection?
imageio keeps multiple connections to qemu-nbd, and pass all requests to qemu-nbd. No caching is done in imageio.
But we don't do any synchronization, so we may have one flush command in the middle of a write/write_zeros commands.
Does qemu-nbd wait until the write/wirte_zeros commands complete before flushing?
I should note that virt-v2v (the only place this plugin is used) is a special case because we control the NBD client and we're only doing a copy. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On Tue, Aug 03, 2021 at 10:36:47PM +0300, Nir Soffer wrote:
On Tue, Aug 3, 2021 at 10:26 PM Eric Blake <eblake@redhat.com> wrote:
On Mon, Aug 02, 2021 at 08:41:20AM +0100, Richard W.M. Jones wrote:
--- v2v/rhv-upload-plugin.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index a3d578176..51e9b33f7 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -169,6 +169,10 @@ def open(readonly): }
+def can_multi_conn(h): + return True
Should this be h['can_flush'] instead of True?
Are we guaranteed that imageio guarantees a consistent image across all other http connections when a flush is received on a single http connection?
imageio keeps multiple connections to qemu-nbd, and pass all requests to qemu-nbd. No caching is done in imageio.
But we don't do any synchronization, so we may have one flush command in the middle of a write/write_zeros commands.
Does qemu-nbd wait until the write/wirte_zeros commands complete before flushing?
The rules on the client is that if you want action A to be visible by B, you have to wait until the server's response to action A has been received, then send the flush, then wait for the server's response to the flush, then send action B. If the server supports multi-conn, then those rules apply regardless of which connection sends action A, the flush, or action B; if multi-conn is not relied on, then all of A, the flush, and B must be on the same connection. qemu-nbd is built on top of the qemu block layer which has its own internal serialization that guarantees that any flush action acted on by the block layer will stabilize any earlier writes and finish before any further writes - but that does NOT take into account the possibility that parallel clients can race to have out-of-order transmission effects. Even with a single client connection, where the client batch-sends A/flush/B over the wire, the qemu-nbd coroutine code is such that based on the order in which the OS wakes up semaphores in response to data being available to read from the socket, the calls into the block layer could still be rearranged as B/flush/A. That is, if an NBD client issues action A but does not wait for A's response before issuing a flush, or the client issues action B before getting the response to the flush, you cannot ensure that qemu-nbd processes those commands in the same order. So in general, the client has to actually wait for the response to the write before sending the flush, if the write is supposed to be picked up as part of the flush. But that's only when you care about the effects of action A definitely being visible to action B. There is nothing stopping a client from issuing a flush in parallel with in-flight write/zero commands; BUT that client must also be prepared that the writes that are in flight might not be flushed by the flush command. In the case of nbdcopy, we know that our writes are non-overlapping and at a large enough granularity to not trigger read-modify-write sharding, so we really don't care whether action A is visible before starting action B; all we REALLY care about is that after ALL write actions are done, we flush the disk at least once before disconnecting, so that there is no data loss due to disconnecting too early. In this style of operation, you aren't going to issue the flush until after all other commands have completed, so there are no in-flight commands in parallel with the flush to worry about in the first place. Having a flush in parallel with other in-flight operations tends to happen more with actual guest use of a disk, when the guest OS really does want to ensure that action A is visible to action B (say two cooperating processes manipulating a common file in the file system), but does not care about action C being performed in parallel by an independent process (managing a different file in the file system). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Mon, Aug 2, 2021 at 10:41 AM Richard W.M. Jones <rjones@redhat.com> wrote:
Hi Nir,
The experimental patch attached (for discussion) would enable multi-conn. For current virt-v2v this would have no effect since "qemu-img convert" does not make multi-conn NBD connections.
However for modular virt-v2v[1] which uses nbdcopy this would allow nbdcopy to make multiple NBD connections to nbdkit running the plugin. (By default 4 connections).
If I understand the rhv-upload-plugin code correctly, there is a pool per NBD connection at the moment, so this would create 4 pools and 4*4 backend connections to imageio. This is possibly not desirable, but I guess we can turn the pool into a global object. (Note each NBD connection has the same (destination_url, host, options) tuple).
Creating 4*4 connections will not work since on imageio side we have qem-nbd supporting up to 8 connections. What will happen is that http request will block once there are 8 connected client on imageio side. The http client will connect only when some other clients have disconnected, which may never happen, so this will likely end with a deadlock. If we have multi_conn we don't need the pool, we can have a configuration value to control the number of connections, and use single connection (that looks like a pool) in this case. I'm not about how multi_con works in the python plugin - do we get one open() call or 4 open() calls? If we get 4 open() calls, this cannot work since we mix control flow and data flow in the rhv plugin. Each open call will try to create a new disk and every connection will upload to a different disk. multi_con can work if we have: - single open() call - many write/zero/flush calls - single close() call If we have: - one open() for nbd connection - many write/zero calls per connection - one close() for nbd connection We need to separate the control flow stuff (e.g. create disk) from the plugin, and do it in another step of the process.
Naturally I've not actually tested any of this. There are some advantages from the nbdcopy side of things, especially because it is able to query extents on the input side in parallel which seems to be advantageous for inputs with slow extent querying (like VDDK). There may be further advantages on the output side because it would allow us to write data to the RHV upload plugin over four sockets, allowing better use of multiple cores.
It may work better when using HTTPS, the TLS part is implemented in C and can run in parallel. Nir
On Mon, Aug 02, 2021 at 03:35:36PM +0300, Nir Soffer wrote:
I'm not about how multi_con works in the python plugin - do we get one open() call or 4 open() calls?
The NBD client is supposed to: - make a single NBD connection - test the multi-conn flag on that connection - if it is true and the client wants to proceed with multi-conn, make N-1 further NBD connections - each NBD connection operates separately, except for the multi-conn guarantees related to accurate flushing and write tearing: * if you flush or FUA on one connection, before you return on that connection, any caches in the other connections must also be flushed nbdcopy: - correctly implements the NBD protocol and multi-conn (or if it doesn't, it's a bug!) - makes large enough requests that write tearing is unlikely to be an issue - doesn't use FUA - issues separate flush calls on every connection at the end So there will be multiple open() calls in the Python plugin.
If we get 4 open() calls, this cannot work since we mix control flow and data flow in the rhv plugin. Each open call will try to create a new disk and every connection will upload to a different disk.
Interesting. I did try it now. It's actually slower :-( But I didn't change how HTTP pools worked. However it didn't appear to create multiple disks, unless it only attached one and the others are hidden somehow. I'll PM you the admin details for my RHV cluster so you can take a look. It has to be said this assumption that open() creates the disk is not a good one. It's certainly not guaranteed that even with "qemu-img convert" only a single NBD connection will ever be opened. It's quite valid for an NBD client to open a connection and query properties about it, and then perhaps close it and open another one. So I guess we have to fix this anyway.
multi_con can work if we have:
- single open() call - many write/zero/flush calls - single close() call
If we have:
- one open() for nbd connection - many write/zero calls per connection - one close() for nbd connection
We need to separate the control flow stuff (e.g. create disk) from the plugin, and do it in another step of the process.
Naturally I've not actually tested any of this. There are some advantages from the nbdcopy side of things, especially because it is able to query extents on the input side in parallel which seems to be advantageous for inputs with slow extent querying (like VDDK). There may be further advantages on the output side because it would allow us to write data to the RHV upload plugin over four sockets, allowing better use of multiple cores.
It may work better when using HTTPS, the TLS part is implemented in C and can run in parallel.
Yes, or even with kTLS it may run in multiple kernel threads. 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
On Mon, Aug 02, 2021 at 01:50:05PM +0100, Richard W.M. Jones wrote:
However it didn't appear to create multiple disks, unless it only attached one and the others are hidden somehow. I'll PM you the admin details for my RHV cluster so you can take a look.
It turns out that it does create a new disk on every connection -- I can see it under the Storage tab -- so that's quite bad. I'll see if I can come up with a patch. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On Mon, Aug 2, 2021 at 6:49 PM Richard W.M. Jones <rjones@redhat.com> wrote:
On Mon, Aug 02, 2021 at 01:50:05PM +0100, Richard W.M. Jones wrote:
However it didn't appear to create multiple disks, unless it only attached one and the others are hidden somehow. I'll PM you the admin details for my RHV cluster so you can take a look.
It turns out that it does create a new disk on every connection -- I can see it under the Storage tab -- so that's quite bad. I'll see if I can come up with a patch.
It shows that the code is robust, but not very useful :-) I guess that the vm was created with the last disk id written to disk. The issue is not only creating the disk, but creating an image transfer per plugin open(). The wanted flow is: 1. precheck (vm already exists? etc.) 2. convert source disk We need to know the size and required allocation of the disk before creating the disk, for example for qcow2 disks on block storage. 3. create disk Note that the disk id can be an input to ovirt, there is no need to depend on ovirt for generating the disk id, and pass it back top the code creating the vm. See https://gerrit.ovirt.org/c/ovirt-engine-sdk/+/115630 This step is slow (for no good reason), so it you have multiple disks you may want to create them in parallel. See https://gerrit.ovirt.org/c/ovirt-engine-sdk/+/113862 4. create image_transfer, keep transfer_url, proxy_url 5. run nbdkit with the rhv plugin and the transfer urls from step 4. There is no need to create more than one connection to imageio per plugin instance. 6. finalize image_transfer 7. create the vm using disk id generated for step 3 Nir
On Tue, Aug 03, 2021 at 12:16:48AM +0300, Nir Soffer wrote:
On Mon, Aug 2, 2021 at 6:49 PM Richard W.M. Jones <rjones@redhat.com> wrote:
On Mon, Aug 02, 2021 at 01:50:05PM +0100, Richard W.M. Jones wrote:
However it didn't appear to create multiple disks, unless it only attached one and the others are hidden somehow. I'll PM you the admin details for my RHV cluster so you can take a look.
It turns out that it does create a new disk on every connection -- I can see it under the Storage tab -- so that's quite bad. I'll see if I can come up with a patch.
It shows that the code is robust, but not very useful :-)
I guess that the vm was created with the last disk id written to disk.
The issue is not only creating the disk, but creating an image transfer per plugin open(). The wanted flow is:
1. precheck (vm already exists? etc.)
2. convert source disk
We need to know the size and required allocation of the disk before creating the disk, for example for qcow2 disks on block storage.
3. create disk
Note that the disk id can be an input to ovirt, there is no need to depend on ovirt for generating the disk id, and pass it back to the code creating the vm. See https://gerrit.ovirt.org/c/ovirt-engine-sdk/+/115630
This step is slow (for no good reason), so it you have multiple disks you may want to create them in parallel. See https://gerrit.ovirt.org/c/ovirt-engine-sdk/+/113862
4. create image_transfer, keep transfer_url, proxy_url
5. run nbdkit with the rhv plugin and the transfer urls from step 4.
There is no need to create more than one connection to imageio per plugin instance.
What does this sentence mean? Do you mean one HTTP connection?
6. finalize image_transfer
7. create the vm using disk id generated for step 3
Nir
I have something: https://github.com/rwmjones/virt-v2v/commit/42a917915b551254eab83d0b7fad5611... This creates the disk in .get_ready (refer to the lifecycle diagram here: https://libguestfs.org/nbdkit-plugin.3.html#Callback-lifecycle ) Doing finalization one time is more complicated. The obvious place would be in .cleanup() but for various reasons we cannot use that with the architecture of new virt-v2v, so instead we use a hack to signal to the plugin that we want to finalize. (Aside: I really wish the RHV API was more sane. What even is "finalization"? I still don't understand why this step is necessary.) The above works, as in it doesn't give any errors, but there are two very large problems. For some reason I don't understand it still slower than the old method using qemu-img convert. But the bigger problem is that it corrupts the disks. Is the target disk always filled with zeroes before we start copying? I think architecturally your suggested flow above will be better, if AIUI you mean that we should run the creation and finalization code outside the nbdkit plugin, and only pass the transfer URLs to nbdkit. I might have a go at that tomorrow when I'm less tired. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On Tue, Aug 3, 2021 at 12:46 AM Richard W.M. Jones <rjones@redhat.com> wrote:
On Tue, Aug 03, 2021 at 12:16:48AM +0300, Nir Soffer wrote:
On Mon, Aug 2, 2021 at 6:49 PM Richard W.M. Jones <rjones@redhat.com> wrote:
On Mon, Aug 02, 2021 at 01:50:05PM +0100, Richard W.M. Jones wrote:
However it didn't appear to create multiple disks, unless it only attached one and the others are hidden somehow. I'll PM you the admin details for my RHV cluster so you can take a look.
It turns out that it does create a new disk on every connection -- I can see it under the Storage tab -- so that's quite bad. I'll see if I can come up with a patch.
It shows that the code is robust, but not very useful :-)
I guess that the vm was created with the last disk id written to disk.
The issue is not only creating the disk, but creating an image transfer per plugin open(). The wanted flow is:
1. precheck (vm already exists? etc.)
2. convert source disk
We need to know the size and required allocation of the disk before creating the disk, for example for qcow2 disks on block storage.
3. create disk
Note that the disk id can be an input to ovirt, there is no need to depend on ovirt for generating the disk id, and pass it back to the code creating the vm. See https://gerrit.ovirt.org/c/ovirt-engine-sdk/+/115630
This step is slow (for no good reason), so it you have multiple disks you may want to create them in parallel. See https://gerrit.ovirt.org/c/ovirt-engine-sdk/+/113862
4. create image_transfer, keep transfer_url, proxy_url
5. run nbdkit with the rhv plugin and the transfer urls from step 4.
There is no need to create more than one connection to imageio per plugin instance.
What does this sentence mean? Do you mean one HTTP connection?
Yes, one HTTP connection to imageio per plugin instance (h dict).
6. finalize image_transfer
7. create the vm using disk id generated for step 3
Nir
I have something: https://github.com/rwmjones/virt-v2v/commit/42a917915b551254eab83d0b7fad5611...
I'll check this tomorrow.
This creates the disk in .get_ready (refer to the lifecycle diagram here: https://libguestfs.org/nbdkit-plugin.3.html#Callback-lifecycle )
Doing finalization one time is more complicated.
This is really required, there is no way to finalize more than once.
The obvious place would be in .cleanup() but for various reasons we cannot use that with the architecture of new virt-v2v, so instead we use a hack to signal to the plugin that we want to finalize.
(Aside: I really wish the RHV API was more sane. What even is "finalization"? I still don't understand why this step is necessary.)
When you start a transfer the system: - takes a lock on the disk - mark the disk is illegal so it cannot be used by the system - select a host for the transfer - attach the disk to the host - start nbd server on the host - configure imageio server to serve the disk - return a transfer url When you finalize a transfer the system does: - unconfigure the imageio server on the host so it does not serve the image - stop the nbd server - verify the disk contents (e.g, raw disk is raw, backing file matches ovirt metadata, virtual size match, etc.) if verification fails the system will delete the disk and the image transfer will fail. - mark the disk as legal so the system can use it - detach the disk from the transfer host - release the lock on the disk How do you suggest doing all this without an API call?
The above works, as in it doesn't give any errors, but there are two very large problems. For some reason I don't understand it still slower than the old method using qemu-img convert.
But the bigger problem is that it corrupts the disks. Is the target disk always filled with zeroes before we start copying?
With file storage (NFS, Glsuter) yes, but with block storage the contents of the disk is undefined and you must zero the areas you don't write data to.
I think architecturally your suggested flow above will be better, if AIUI you mean that we should run the creation and finalization code outside the nbdkit plugin, and only pass the transfer URLs to nbdkit.
Exactly! Nir
participants (3)
-
Eric Blake -
Nir Soffer -
Richard W.M. Jones