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@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>
This patch was replaced with the "qemu: Drop QEMU_CAPS_MIGRATION_EVENT"
series.
Jirka
