[libvirt PATCH v4 3/4] qemu_migration: get migration blockers before hardcoded checks

[Laine Stump]

On 7/20/22 12:05 PM, 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.

I reworded the commit log message a bit. Rather than repeating it here, 
I'll just point you to the branch I pushed to gitlab (link in my 
response to the cover letter).
> 
> Signed-off-by: Eugenio Pérez <eperezma at redhat.com>
> Reviewed-by: Jiri Denemark <jdenemar at redhat.com>
> ---
> v4:
> * Do not override qemuDomainGetMigrationBlockers error calling again
>    virReportError.
> * Replace ", " with "; " in blockers separators.
> 
> v3:
> * Report message with a colon.
> * Report all blockers instead of only the first.
> ---
>   src/qemu/qemu_migration.c | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index b12cb518ee..2e3044289a 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;
> +}
>   

Note that we've been trying to keep 2 blank lines between each function, 
but here you've added a new function in between those 2 blank lines, so 
there's only a single blank before and after. I added in the extra blanks.

>   /**
>    * qemuMigrationSrcIsAllowed:
> @@ -1439,6 +1453,22 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
>       int nsnapshots;
>       int pauseReason;
>       size_t i;
> +    int r;
> +
> +    /* Ask qemu if it have a migration blocker */

    "has 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)
> +            return false;

in libvirt we usually return -1 on failure, and check with "if (r < 0)". 
And since that is the only use of "r", we prefer to not have the extra 
stack variable cluttering things up, so:

      if (qemuDomainGetMigrationBlockers(driver, vm, &blockers) < 0)
          return false;

> +
> +        if (blockers && blockers[0]) {
> +            g_autofree char *reasons = g_strjoinv("; ", blockers);
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("cannot migrate domain: %s"), reasons);
> +            return false;
> +        }
> +    }
>   
>       /* perform these checks only when migrating to remote hosts */
>       if (remote) {