[PATCH 11/19] qemu: migration_params: Add infrastructure for 'dirty-bitmaps' migration feature

[Jiri Denemark]

On Thu, Feb 11, 2021 at 16:37:50 +0100, Peter Krempa wrote:
> Add the migration capability flag and the propagation of the
> corresponding mapping configuration. The mapping will be produced from
> the bitmaps on disk depending on both sides of the migration and the
> necessity to perform merges.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>  src/qemu/qemu_migration_params.c | 24 ++++++++++++++++++++++++
>  src/qemu/qemu_migration_params.h |  5 +++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
> index 510dad783a..8f491e0ed2 100644
> --- a/src/qemu/qemu_migration_params.c
> +++ b/src/qemu/qemu_migration_params.c
...
> @@ -524,6 +527,20 @@ qemuMigrationParamsSetCompression(virTypedParameterPtr params,
>  }
> 
> 
> +void
> +qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParamsPtr migParams,
> +                                              virJSONValuePtr *params)
> +{
> +    virJSONValueFree(migParams->blockDirtyBitmapMapping);
> +    migParams->blockDirtyBitmapMapping = g_steal_pointer(params);
> +
> +    if (migParams->blockDirtyBitmapMapping)
> +        ignore_value(virBitmapSetBit(migParams->caps, QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS));
> +    else
> +        ignore_value(virBitmapClearBit(migParams->caps, QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS));
> +}
> +
> +
>  qemuMigrationParamsPtr
>  qemuMigrationParamsFromFlags(virTypedParameterPtr params,
>                               int nparams,
> @@ -747,6 +764,12 @@ qemuMigrationParamsToJSON(qemuMigrationParamsPtr migParams)
>              return NULL;
>      }
> 
> +    if (migParams->blockDirtyBitmapMapping &&
> +        virJSONValueObjectAppend(params, "block-bitmap-mapping",
> +                                 migParams->blockDirtyBitmapMapping) < 0)
> +        return NULL;
> +    migParams->blockDirtyBitmapMapping = NULL;

I checked all (hopefully) call paths leading to this function and what
you're doing is safe, but I still would not expect a function creating
json from migParams to eat anything from the source. Especially when the
corresponding bit in migParams->caps would remain set. Can we make a
copy here instead to avoid (unlikely, though) surprises in the future?

> +
>      return g_steal_pointer(&params);
>  }
> 
> @@ -1202,6 +1225,7 @@ qemuMigrationParamsReset(virQEMUDriverPtr driver,
>          goto cleanup;
> 
>      qemuMigrationParamsResetTLS(driver, vm, asyncJob, origParams, apiFlags);
> +    /* We don't reset 'block-bitmap-mapping' as it can't be unset */

So what happens if migration fails? Will mapping be cleared as a side
effect of removing the temporary migration bitmaps or will it stay set
for future migration? Although I guess the next attempt would either set
the appropriate capability and replace the mapping or not set the
capability and thus any possibly existing mapping would be ignored in
case it was not cleared, right?

Jirka