Re: [PATCH] qemu: Move interlocking of blockjobs and checkpoints after liveness check

[Pavel Mores <pmores@xxxxxxxxxx>]

On Thu, Apr 23, 2020 at 04:13:43PM +0200, Peter Krempa wrote:
> qemuDomainSupportsCheckpointsBlockjobs checks if the
> QEMU_CAPS_INCREMENTAL_BACKUP capability is supported to do the
> interlocking. Capabilities are not present when the VM isn't running
> though which would create false errors.
> 
> Move the checks after the liveness check in block job implementations.
> 
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>

Reviewed-by: Pavel Mores <pmores@xxxxxxxxxx>

> ---
>  src/qemu/qemu_driver.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 00d276061e..5ecf12deef 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17439,6 +17439,9 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
>      if (virDomainObjCheckActive(vm) < 0)
>          goto endjob;
> 
> +    if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
> +        goto endjob;
> +
>      if (!(disk = qemuDomainDiskByName(vm->def, path)))
>          goto endjob;
> 
> @@ -17994,6 +17997,9 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
>      if (virDomainObjCheckActive(vm) < 0)
>          goto endjob;
> 
> +    if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
> +        goto endjob;
> +
>      if (!(disk = qemuDomainDiskByName(vm->def, path)))
>          goto endjob;
> 
> @@ -18278,9 +18284,6 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
>      if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0)
>          goto cleanup;
> 
> -    if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
> -        goto cleanup;
> -
>      /* For normal rebase (enhanced blockpull), the common code handles
>       * everything, including vm cleanup. */
>      if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
> @@ -18364,9 +18367,6 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml,
>      if (virDomainBlockCopyEnsureACL(dom->conn, vm->def) < 0)
>          goto cleanup;
> 
> -    if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
> -        goto cleanup;
> -
>      for (i = 0; i < nparams; i++) {
>          virTypedParameterPtr param = &params[i];
> 
> @@ -18429,11 +18429,6 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth,
>          return -1;
>      }
> 
> -    if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) {
> -        virDomainObjEndAPI(&vm);
> -        return -1;
> -    }
> -
>      /* qemuDomainBlockPullCommon consumes the reference on @vm */
>      return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags);
>  }
> -- 
> 2.26.0
>