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 qemu monitor to send this query. This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication.

I reworded this commit log a bit - see the branch I linked in my response to the cover letter and let me know if you approve.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> --- v4: * Separate return type into its own line

v3: * Squash some patches * Return ok in qemuMonitorJSONGetMigrationBlockers is there are no blockers. * Move note to function doc. --- src/qemu/qemu_monitor.c | 11 +++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 44 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 62 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 109107eaae..e0939beecd 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4486,3 +4486,14 @@ qemuMonitorMigrateRecover(qemuMonitor *mon,

return qemuMonitorJSONMigrateRecover(mon, uri); } + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers)

We try to keep 2 blank lines between each function. I added in the extra blank lines before and after this function.

+{ + VIR_DEBUG("blockers=%p", blockers); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetMigrationBlockers(mon, blockers); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cc1a0bc8c9..b82f198285 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1543,3 +1543,7 @@ qemuMonitorChangeMemoryRequestedSize(qemuMonitor *mon, int qemuMonitorMigrateRecover(qemuMonitor *mon, const char *uri); + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..6b26dfcb54 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,50 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; }

+/* + * Get the exposed migration blockers. + * + * This function assume qemu has the capability of request them. + * + * It returns a NULL terminated array on blockers if there are any, or it set + * it to NULL otherwise. + */ +int +qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + virJSONValue *jblockers; + size_t i; + + *blockers = NULL; + if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + return -1; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(jblockers = virJSONValueObjectGetArray(data, "blocked-reasons"))) + return 0; + + *blockers = g_new0(char *, virJSONValueArraySize(jblockers) + 1); + for (i = 0; i < virJSONValueArraySize(jblockers); i++) { + virJSONValue *jblocker = virJSONValueArrayGet(jblockers, i); + const char *blocker = virJSONValueGetString(jblocker); + + (*blockers)[i] = g_strdup(blocker); + } + + return 0; +} + int qemuMonitorJSONMigrateCancel(qemuMonitor *mon) { g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("migrate_cancel", NULL); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 2759566892..e4c65e250e 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -184,6 +184,9 @@ qemuMonitorJSONMigrate(qemuMonitor *mon, unsigned int flags, const char *uri); int +qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers); +int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitor *mon, bool *spice_migrated);

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) {

On 7/20/22 12:05 PM, Eugenio Pérez wrote:

vDPA devices will be migratable soon. Since they are not migratable before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking for migration blockers, let it hardcoded in that case.

Otherwise, ask qemu about the explicit blocker.

I reworded the commit log message, but otherwise this is all fine.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> --- v3: Fix indentation --- src/qemu/qemu_migration.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2e3044289a..b554027da2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int pauseReason; size_t i; int r; + bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_MIGRATION_BLOCKED_REASONS);

/* Ask qemu if it have a migration blocker */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + if (blockedReasonsCap) { g_auto(GStrv) blockers = NULL; r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); if (r != 0) @@ -1576,7 +1578,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainNetDef *net = vm->def->nets[i]; qemuSlirp *slirp;

- if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) { + if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("vDPA devices cannot be migrated")); return false;