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

Peter Krempa posted 1 patch 4 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/a66ce1010766f61ab067c5edfa16a2663b14ad7b.1587651223.git.pkrempa@redhat.com
src/qemu/qemu_driver.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
[PATCH] qemu: Move interlocking of blockjobs and checkpoints after liveness check
Posted by Peter Krempa 4 years ago
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@redhat.com>
---
 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

Re: [PATCH] qemu: Move interlocking of blockjobs and checkpoints after liveness check
Posted by Eric Blake 4 years ago
On 4/23/20 9:13 AM, 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@redhat.com>
> ---
>   src/qemu/qemu_driver.c | 17 ++++++-----------
>   1 file changed, 6 insertions(+), 11 deletions(-)

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

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

Re: [PATCH] qemu: Move interlocking of blockjobs and checkpoints after liveness check
Posted by Pavel Mores 4 years ago
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@redhat.com>

Reviewed-by: Pavel Mores <pmores@redhat.com>

> ---
>  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
>