Re: [libvirt PATCH v2 4/5] qemu_migration: get migration blockers before hardcoded checks

On Wed, Jul 20, 2022 at 12:31 PM Jiri Denemark <jdenemar(a)redhat.com&gt; wrote: > > On Wed, Jul 20, 2022 at 11:11:53 +0200, Eugenio Pérez wrote: > > since qemu 6.0, if migration is blocked for some reason, 'query-migrate' > > will return an array of error strings describing the migration blockers. > > This can be used to check whether there are any devices blocking > > migration, etc. > > > > Enable qemuMigrationSrcIsAllowed to query it. > > > > Signed-off-by: Eugenio Pérez <eperezma(a)redhat.com&gt; > > --- > > src/qemu/qemu_migration.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > > index b12cb518ee..4224339f39 100644 > > --- a/src/qemu/qemu_migration.c > > +++ b/src/qemu/qemu_migration.c > > @@ -1414,6 +1414,20 @@ qemuMigrationSrcIsAllowedHostdev(const virDomainDef *def) > > return true; > > } > > > > +static int > > +qemuDomainGetMigrationBlockers(virQEMUDriver *driver, > > + virDomainObj *vm, > > + char ***blockers) > > +{ > > + qemuDomainObjPrivate *priv = vm->privateData; > > + int rc; > > + > > + qemuDomainObjEnterMonitor(driver, vm); > > + rc = qemuMonitorGetMigrationBlockers(priv->mon, blockers); > > + qemuDomainObjExitMonitor(vm); > > + > > + return rc; > > +} > > > > /** > > * qemuMigrationSrcIsAllowed: > > @@ -1439,6 +1453,25 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, > > int nsnapshots; > > int pauseReason; > > size_t i; > > + int r; > > + > > + /* Ask qemu if it have a migration blocker */ > > + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { > > + g_auto(GStrv) blockers = NULL; > > + r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); > > + if (r != 0) { > > + virReportError(VIR_ERR_OPERATION_INVALID, > > + _("cannot migrate domain, %s"), > > + _("error getting blockers")); > > If qemuDomainGetMigrationBlockers returned -1 a better error message > should already be set and you would overwrite it here. Also with your > current patch you would report this error even in case there was no > migration blocker reported by QEMU. But this part would be fixed by > letting qemuDomainGetMigrationBlockers return 0 when the blocked-reasons > field is missing. > Got it. > > + return false; > > + } > > + > > + if (blockers[0]) { > > blockers && blockers[0] > > > + virReportError(VIR_ERR_OPERATION_INVALID, > > + _("cannot migrate domain, %s"), blockers[0]); > > I would prefer a colon there: "cannot migrate domain: %s". And we got a > list of blockers from QEMU, but only use the first one? Please, join > them with g_strjoinv(). > So the resulting message should be: "cannot migrate domain: <error1>, <error2>, ..., <errorN>", right?