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

Eugenio Pérez posted 4 patches 3 years, 6 months ago
[libvirt PATCH v4 3/4] qemu_migration: get migration blockers before hardcoded checks
Posted by Eugenio Pérez 3 years, 6 months ago
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@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@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;
+}
 
 /**
  * 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 */
+    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) {
+        g_auto(GStrv) blockers = NULL;
+        r = qemuDomainGetMigrationBlockers(driver, vm, &blockers);
+        if (r != 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) {
-- 
2.31.1

Re: [libvirt PATCH v4 3/4] qemu_migration: get migration blockers before hardcoded checks
Posted by Laine Stump 3 years, 6 months ago
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@redhat.com>
> Reviewed-by: Jiri Denemark <jdenemar@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) {