[libvirt PATCH 2/3] qemu: Create wrapper for qemuMonitorMigrateCancel

[Peter Krempa]

On Thu, Sep 01, 2022 at 14:47:40 +0200, Jiri Denemark wrote:
> We will need a little bit more code around qemuMonitorMigrateCancel to
> make sure it works as expected. The new qemuMigrationSrcCancel helper
> will avoid repeating the code in several places.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/qemu/qemu_driver.c    |  9 +--------
>  src/qemu/qemu_migration.c | 37 +++++++++++++++++++++++--------------
>  src/qemu/qemu_migration.h |  4 ++++
>  src/qemu/qemu_process.c   |  5 +----
>  4 files changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 707f4cc1bb..a86efc769a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12808,17 +12808,10 @@ qemuDomainGetJobStats(virDomainPtr dom,
>  static int
>  qemuDomainAbortJobMigration(virDomainObj *vm)
>  {
> -    qemuDomainObjPrivate *priv = vm->privateData;
> -    int ret;
> -
>      VIR_DEBUG("Cancelling migration job at client request");
>  
>      qemuDomainObjAbortAsyncJob(vm);
> -    qemuDomainObjEnterMonitor(vm);
> -    ret = qemuMonitorMigrateCancel(priv->mon);

So this caller cared about the return value of
'qemuMonitorMigrateCancel' ...

> -    qemuDomainObjExitMonitor(vm);
> -
> -    return ret;
> +    return qemuMigrationSrcCancel(vm, VIR_ASYNC_JOB_NONE);
>  }
>  
>  
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 67d83ca743..5845dfdb9c 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -4611,6 +4611,24 @@ qemuMigrationSrcStart(virDomainObj *vm,
>  }
>  
>  
> +int
> +qemuMigrationSrcCancel(virDomainObj *vm,
> +                       virDomainAsyncJob asyncJob)
> +{
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +
> +    VIR_DEBUG("Cancelling outgoing migration of domain %s", vm->def->name);
> +
> +    if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
> +        return -1;
> +
> +    qemuMonitorMigrateCancel(priv->mon);

... but here you don't propagate it out. Instead the only possibility
for this function to fail is from 'qemuDomainObjEnterMonitorAsync', but
any caller that pases non-NONE asyncjob doesn't care about the return
value at all.

> +    qemuDomainObjExitMonitor(vm);
> +
> +    return 0;
> +}
> +
> +

Also consider adding a comment too.

With the above bug fixed:

Reviewed-by: Peter Krempa <pkrempa at redhat.com>