Re: [libvirt] [PATCH v2 22/25] qemu: driver: Allow cancellation of the backup job

[Ján Tomko]

On Thu, Dec 05, 2019 at 02:18:38PM +0100, Peter Krempa wrote:

On Wed, Dec 04, 2019 at 11:20:55 +0000, Daniel Berrange wrote:

On Tue, Dec 03, 2019 at 06:17:44PM +0100, Peter Krempa wrote:
> Use the helper which cancels all blockjobs to perform the backup job
> cancellation in qemuDomainAbortJob.
>
> Signed-off-by: Peter Krempa <pkre...@redhat.com>
> ---
>  src/qemu/qemu_driver.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 95882d9d14..2408b08106 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14054,11 +14054,16 @@ static int qemuDomainAbortJob(virDomainPtr dom)
>      }
>
>      VIR_DEBUG("Cancelling job at client request");
> -    qemuDomainObjAbortAsyncJob(vm);
> -    qemuDomainObjEnterMonitor(driver, vm);
> -    ret = qemuMonitorMigrateCancel(priv->mon);
> -    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> -        ret = -1;
> +    if (priv->job.asyncJob == QEMU_ASYNC_JOB_BACKUP) {
> +        qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
> +        ret = 0;
> +    } else {
> +        qemuDomainObjAbortAsyncJob(vm);
> +        qemuDomainObjEnterMonitor(driver, vm);
> +        ret = qemuMonitorMigrateCancel(priv->mon);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            ret = -1;
> +    }

Hmm, this makes me thing we should have had some better error checking
in here already. IIUC, we have other types async job that are not
related to either migration or backups, so should we do

   switch (priv->job.asyncJob) {
        case QEMU_ASYNC_JOB_BACKUP:
           ...
        case QEMU_ASYNC_JOB_MIGRATE:
           ...
        case QEMU_ASYNC_JOB....
        default:
          report error

This is now done upstream. The new version of the patch after merging
with upstream:


diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 93b6107f6c..72694dc8d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14091,7 +14091,8 @@ static int qemuDomainAbortJob(virDomainPtr dom)
        break;

    case QEMU_ASYNC_JOB_BACKUP:
-        /* TODO: to be implemented later */
+        qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
+        ret = 0;
        break;

Reviewed-by: Ján Tomko <jto...@redhat.com>

Jano

signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list