On Wed, May 07, 2025 at 10:07:41 +0300, Dmitry Frolov wrote:
> Enum variable of type qemuMigrationCapability is checked for zero in
> src/qemu/qemu_migration_params.c:729.
>
> "if (item->optional) { ..."
>
> Actualy, QEMU_MIGRATION_CAP_XBZRLE enum constant has value 0.
> So, at least, the condition is incorrect.
>
> v1: introducing a separate enum for optional capabilities
> v2: another approach: fix only the incorrect condition
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Frolov <fro...@swemel.ru>
> ---
> src/qemu/qemu_migration_params.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_migration_params.c
> b/src/qemu/qemu_migration_params.c
> index c10660d6f2..98d314cf2d 100644
> --- a/src/qemu/qemu_migration_params.c
> +++ b/src/qemu/qemu_migration_params.c
> @@ -700,6 +700,12 @@
> qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParams *migParams,
> ignore_value(virBitmapClearBit(migParams->caps,
> QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS));
> }
>
> +static bool
> +qemuMigrationCapabilityIsOptional(qemuMigrationCapability cap)
> +{
> + return cap == QEMU_MIGRATION_CAP_POSTCOPY_PREEMPT ||
> + cap == QEMU_MIGRATION_CAP_SWITCHOVER_ACK;
> +}
>
> qemuMigrationParams *
> qemuMigrationParamsFromFlags(virTypedParameterPtr params,
> @@ -725,7 +731,7 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
> qemuMigrationCapabilityTypeToString(item->cap));
> ignore_value(virBitmapSetBit(migParams->caps, item->cap));
>
> - if (item->optional) {
> + if (qemuMigrationCapabilityIsOptional(item->optional)) {
> qemuMigrationCapability opt = item->optional;
> ignore_value(virBitmapSetBit(migParams->optional, opt));
> if (item->party != party)
The issue was that item->optional is initialized to a value that matches
one of the capabilities and this patch doesn't do anything with it. The
check whether item->optional is non-zero works fine so changing just
this check does not make any sense.
Jirka
