[Libguestfs] [PATCH virt-v2v] v2v: -o rhv-upload: Enable multi-conn

[Nir Soffer]

On Tue, Aug 3, 2021 at 12:46 AM Richard W.M. Jones <rjones at 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 at 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/42a917915b551254eab83d0b7fad56112fcde12b

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