On Thu, May 12, 2022 at 12:40:54 +0200, Peter Krempa wrote:
> On Thu, May 12, 2022 at 10:55:47 +0200, Jiri Denemark wrote:
> > Every running QEMU process we are willing to reconnect (i.e., at least
> > 3.1.0) supports migration events and we can assume the capability is
> > already enabled since last time libvirt daemon connected to its monitor.
> >
> > Well, it's not guaranteed though. If libvirt 1.2.17 or older was used to
> > start QEMU 3.1.0 or newer, migration events would not be enabled. And if
> > the user decides to upgrade libvirt from 1.2.17 to 8.4.0 while the QEMU
> > process is still running, they would not be able to migrate the domain
>
> But doesn't the function below actually enable the events? Or is there
> something else that needs to be done?
>
> Such that we simply could enable the events and be done with it?
We can't blindly enable the events all the time as it is a migration
capability and as such can only be set when there's no active migration.
> > because of disabled migration events. I think we do not really need to
> > worry about this scenario as libvirt 1.2.17 is 7 years old while QEMU
> > 3.1.0 was released only 3.5 years ago. Thus a chance someone would be
> > running such configuration should be fairly small and a combination with
> > upgrading 1.2.17 to 8.4.0 (or newer) with running domains should get it
> > pretty much to zero. The issue would disappear ff the ancient libvirt is
> > first upgraded to something older than 8.4.0 and then to the current
> > libvirt.
> >
> > Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> > ---
> >
> > Notes:
> > I was hoping we could do this without any downside, but it appeared this
> > was not possible. In case someone still thinks this would be an issue, I
> > can take an alternative solution and check whether migration events are
> > enabled when reconnecting to QEMU monitor.
> >
> > src/qemu/qemu_migration_params.c | 26 ++++++++++++++------------
> > src/qemu/qemu_migration_params.h | 3 ++-
> > src/qemu/qemu_process.c | 14 +++++++++-----
> > 3 files changed, 25 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
> > index 26754f03f8..95fd773645 100644
> > --- a/src/qemu/qemu_migration_params.c
> > +++ b/src/qemu/qemu_migration_params.c
> > @@ -1385,10 +1385,10 @@ qemuMigrationParamsParse(xmlXPathContextPtr ctxt,
> > int
> > qemuMigrationCapsCheck(virQEMUDriver *driver,
> > virDomainObj *vm,
> > - int asyncJob)
> > + int asyncJob,
> > + bool reconnect)
> > {
> > qemuDomainObjPrivate *priv = vm->privateData;
> > - g_autoptr(virBitmap) migEvent = NULL;
> > g_autoptr(virJSONValue) json = NULL;
> > g_auto(GStrv) caps = NULL;
> > char **capStr;
> > @@ -1419,22 +1419,24 @@ qemuMigrationCapsCheck(virQEMUDriver *driver,
> > }
> > }
> >
> > - migEvent = virBitmapNew(QEMU_MIGRATION_CAP_LAST);
> > + if (!reconnect) {
> > + g_autoptr(virBitmap) migEvent = virBitmapNew(QEMU_MIGRATION_CAP_LAST);
> >
> > - ignore_value(virBitmapSetBit(migEvent, QEMU_MIGRATION_CAP_EVENTS));
> > + ignore_value(virBitmapSetBit(migEvent, QEMU_MIGRATION_CAP_EVENTS));
>
> Here you assert the flag ...
>
> >
> > - if (!(json = qemuMigrationCapsToJSON(migEvent, migEvent)))
> > - return -1;
> > + if (!(json = qemuMigrationCapsToJSON(migEvent, migEvent)))
> > + return -1;
> >
> > - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> > - return -1;
> > + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> > + return -1;
> >
> > - rc = qemuMonitorSetMigrationCapabilities(priv->mon, &json);
> > + rc = qemuMonitorSetMigrationCapabilities(priv->mon, &json);
>
>
> ... and ask the monitor to enable it. To me that looks like we're good
> even with older qemus, right?
This all happens only when !reconnect, that is when we're starting a
fresh QEMU. When reconnecting to an already running QEMU, we just expect
the capability was enabled when it was first started. Which is true for
any libvirt > 1.2.17 starting QEMU 3.1.0 and newer (and we won't even
reconnect to an older one).
Jirka
