On Wed, Jul 20, 2022 at 12:31 PM Jiri Denemark <jdenemar@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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?
Thanks!
> > + return false;
> > + }
> > + }
> >
> > /* perform these checks only when migrating to remote hosts */
> > if (remote) {
>
> Jirka
>
