[libvirt PATCH 36/80] qemu: Handle migration job in qemuMigrationDstFinish

[Peter Krempa]

On Tue, May 10, 2022 at 17:20:57 +0200, Jiri Denemark wrote:
> The function which started a migration phase should also finish it by
> calling qemuMigrationJobFinish/qemuMigrationJobContinue so that the code
> is easier to follow.

[1]

> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/qemu/qemu_migration.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index d02e8132e4..b62f7256c4 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -6017,7 +6017,8 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver,
>                               int retcode,
>                               bool v3proto,
>                               unsigned long long timeReceived,
> -                             virErrorPtr *orig_err)
> +                             virErrorPtr *orig_err,
> +                             bool *finishJob)

Come on! Really?!? [1]

>  {
>      virDomainPtr dom = NULL;
>      g_autoptr(qemuMigrationCookie) mig = NULL;

[....]

> @@ -6162,18 +6159,21 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
>                                                  cookiein, cookieinlen,
>                                                  cookieout, cookieoutlen);
>          }
> +    } else {
> +        bool finishJob = true;
>  
> -        qemuMigrationJobFinish(vm);
> -        goto cleanup;
> +        dom = qemuMigrationDstFinishActive(driver, dconn, vm, cookie_flags,
> +                                           cookiein, cookieinlen,
> +                                           cookieout, cookieoutlen,
> +                                           flags, retcode, v3proto, timeReceived,
> +                                           &orig_err, &finishJob);
> +        if (!finishJob) {
> +            qemuMigrationJobContinue(vm);
> +            goto cleanup;
> +        }
>      }

If I overlook the fact that you have to remember what qemuMigrationDstFinishActive
you used probably the least easy variant of seeing what is happening
with that extra jump. Do it without that jump:

    if (finishJob)
        qemuMigrationJobFinish(vm);
    else
        qemuMigrationJobContinue(vm);

With that I'll maybe buy the argument from the commit message, thus:

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


>  
> -    dom = qemuMigrationDstFinishActive(driver, dconn, vm, cookie_flags,
> -                                       cookiein, cookieinlen,
> -                                       cookieout, cookieoutlen,
> -                                       flags, retcode, v3proto, timeReceived,
> -                                       &orig_err);
> -    if (!dom)
> -        goto cleanup;
> +    qemuMigrationJobFinish(vm);
>  
>   cleanup:
>      virPortAllocatorRelease(port);
> -- 
> 2.35.1
>