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

2022-07-20 Thread 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 
Reviewed-by: Jiri Denemark 
---
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, );
+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, ) < 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) {




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

2022-07-20 Thread Eugenio Pérez
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 
Reviewed-by: Jiri Denemark 
---
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, );
+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