On Tue, May 10, 2022 at 17:20:25 +0200, Jiri Denemark wrote:
> When connecting to a QEMU monitor, we always try to enable migration
> events, but this is an invalid operation during migration. Thus
> reconnecting to a domain with active migration would fail. Let's check
> the state of migration events capability and only try to enable it when
> it is disabled.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> src/qemu/qemu_migration_params.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
[...]
> @@ -1416,10 +1419,20 @@ qemuMigrationCapsCheck(virQEMUDriver *driver,
> } else {
> ignore_value(virBitmapSetBit(priv->migrationCaps, cap));
> VIR_DEBUG("Found migration capability: '%s'", *capStr);
> +
> + if (virBitmapIsBitSet(capState, i))
> + ignore_value(virBitmapSetBit(enabled, cap));
> }
> }
>
> - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) {
> + if (virBitmapIsBitSet(enabled, QEMU_MIGRATION_CAP_EVENTS)) {
> + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) {
> + VIR_DEBUG("Migration events already enabled");
> + } else {
> + VIR_DEBUG("Migration events enabled; setting capability");
> + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
> + }
Is all of the "DEBUG" and checking 'qemuCaps' dance really needed? I
think you can simply unconditionally enable the capability, but is there
any reason it would not be enabled?
> + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) {
> migEvent = virBitmapNew(QEMU_MIGRATION_CAP_LAST);
>
> ignore_value(virBitmapSetBit(migEvent, QEMU_MIGRATION_CAP_EVENTS));
If you provide a good enough explanation:
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
