[PATCH 5/5] qemu: blockcopy: Allow late opening of the backing chain of a shallow copy

[Eric Blake]

On 3/10/20 10:54 AM, Peter Krempa wrote:
> oVirt used a quirk in the pre-blockdev semantics of drive-mirror which
> opened the backing chain of the mirror destination only once
> 'block-job-complete' was called.
> 
> Our introduction of blockdev made qemu open the backing chain images
> right at the start of the job. This broke oVirt's usage of this API
> because they copy the data into the backing chain during the time the
> block copy job is running.
> 
> Re-introduce late open of the backing chain if qemu allows to use

reminder: "allows to ${verb}" is not idiomatic; I'd suggest either 
"allows us to use" or "allows the use of"

> blockdev-snapshot on write-only nodes as it can be used to install the
> backing chain even for an existing image now.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>   src/qemu/qemu_driver.c | 57 +++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 53 insertions(+), 4 deletions(-)
> 

> @@ -17273,6 +17276,27 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
>                                                   blockNamedNodeData,
>                                                   shallow, &actions) < 0)
>                   return -1;
> +
> +            /* Open and install the backing chain of 'mirror' late if we can use
> +             * blockdev-snapshot to do it. This is to appease oVirt that wants
> +             * to copy data into the backing chain while the top image is being
> +             * copied shallow */
> +            if (reuse && shallow &&
> +                virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY) &&
> +                virStorageSourceHasBacking(disk->mirror)) {
> +
> +                if (!(chainattachdata = qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->mirror->backingStore,
> +                                                                                         priv->qemuCaps)))
> +                    return -1;

Long lines, but I'm not sure how to improve the situation.

The comments are good at explaining the situation - we have a window of 
qemu releases that regress when using -blockdev, but as long as oVirt 
can force either the old -drive behavior or insist on new-enough libvirt 
coupled with new-enough qemu that restores the write-only-reopen feature 
that we need, then oVirt won't hit the regression.  I'm not sure how you 
plan to advertise to oVirt if this is a new-enough libvirt + detection 
of new-enough qemu to tell oVirt they don't need to cobble libvirt into 
using -drive rather than -blockdev (they might solve that by minimum 
required versions, rather than having to ask libvirt), but answering 
that question doesn't interfere with the validity of this patch.

Reviewed-by: Eric Blake <eblake at redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org