On Wed, May 11, 2022 at 10:37:25 +0200, Peter Krempa wrote:
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@redhat.com> --- 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@redhat.com>
This patch was replaced with the "qemu: Drop QEMU_CAPS_MIGRATION_EVENT" series. Jirka