From e1ca8ecb46c1504a8552338aa65ac95cf4a87e08 Mon Sep 17 00:00:00 2001 From: Jiri Denemark Date: Thu, 12 Oct 2017 15:19:19 +0200 Subject: [PATCH] qemu: Check QEMU error on failed migration When migration fails, QEMU may provide a description of the error in the reply to query-migrate QMP command. We can fetch this error and use it instead of the generic "unexpectedly failed" message. Signed-off-by: Jiri Denemark Reviewed-by: Pavel Hrdina --- src/qemu/qemu_driver.c | 3 ++- src/qemu/qemu_migration.c | 36 +++++++++++++++++++++++------------- src/qemu/qemu_migration.h | 3 ++- src/qemu/qemu_monitor.c | 8 ++++++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 18 ++++++++++++++---- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 29 ++++++++++++++++++++++++++--- 8 files changed, 77 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f889003ba6..7c8ad32675 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13117,7 +13117,8 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) { if (events && jobInfo->status != QEMU_DOMAIN_JOB_STATUS_ACTIVE && - qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_NONE, jobInfo) < 0) + qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_NONE, + jobInfo, NULL) < 0) goto cleanup; if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE && diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index dd60071bfd..b286d68061 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1382,7 +1382,8 @@ int qemuMigrationFetchStats(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, - qemuDomainJobInfoPtr jobInfo) + qemuDomainJobInfoPtr jobInfo, + char **error) { qemuDomainObjPrivatePtr priv = vm->privateData; qemuMonitorMigrationStats stats; @@ -1391,7 +1392,7 @@ qemuMigrationFetchStats(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - rv = qemuMonitorGetMigrationStats(priv->mon, &stats); + rv = qemuMonitorGetMigrationStats(priv->mon, &stats, error); if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) return -1; @@ -1427,12 +1428,15 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainJobInfoPtr jobInfo = priv->job.current; - + char *error = NULL; bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); + int ret = -1; - if (!events && - qemuMigrationFetchStats(driver, vm, asyncJob, jobInfo) < 0) - return -1; + if (!events || + jobInfo->stats.status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { + if (qemuMigrationFetchStats(driver, vm, asyncJob, jobInfo, &error) < 0) + return -1; + } qemuMigrationUpdateJobType(jobInfo); @@ -1440,17 +1444,18 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver, case QEMU_DOMAIN_JOB_STATUS_NONE: virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"), qemuMigrationJobName(vm), _("is not active")); - return -1; + goto cleanup; case QEMU_DOMAIN_JOB_STATUS_FAILED: virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"), - qemuMigrationJobName(vm), _("unexpectedly failed")); - return -1; + qemuMigrationJobName(vm), + error ? error : _("unexpectedly failed")); + goto cleanup; case QEMU_DOMAIN_JOB_STATUS_CANCELED: virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"), qemuMigrationJobName(vm), _("canceled by client")); - return -1; + goto cleanup; case QEMU_DOMAIN_JOB_STATUS_COMPLETED: case QEMU_DOMAIN_JOB_STATUS_ACTIVE: @@ -1459,7 +1464,12 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver, case QEMU_DOMAIN_JOB_STATUS_POSTCOPY: break; } - return 0; + + ret = 0; + + cleanup: + VIR_FREE(error); + return ret; } @@ -1577,7 +1587,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, } if (events) - ignore_value(qemuMigrationFetchStats(driver, vm, asyncJob, jobInfo)); + ignore_value(qemuMigrationFetchStats(driver, vm, asyncJob, jobInfo, NULL)); qemuDomainJobInfoUpdateTime(jobInfo); qemuDomainJobInfoUpdateDowntime(jobInfo); @@ -3177,7 +3187,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && reason == VIR_DOMAIN_PAUSED_POSTCOPY && qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, - jobInfo) < 0) + jobInfo, NULL) < 0) VIR_WARN("Could not refresh migration statistics"); qemuDomainJobInfoUpdateTime(jobInfo); diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 57c747934d..63a4325624 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -282,7 +282,8 @@ int qemuMigrationFetchStats(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, - qemuDomainJobInfoPtr jobInfo); + qemuDomainJobInfoPtr jobInfo, + char **error); int qemuMigrationErrorInit(virQEMUDriverPtr driver); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aac318f787..8ffce5a35d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2632,12 +2632,16 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon, int qemuMonitorGetMigrationStats(qemuMonitorPtr mon, - qemuMonitorMigrationStatsPtr stats) + qemuMonitorMigrationStatsPtr stats, + char **error) { QEMU_CHECK_MONITOR(mon); + if (error) + *error = NULL; + if (mon->json) - return qemuMonitorJSONGetMigrationStats(mon, stats); + return qemuMonitorJSONGetMigrationStats(mon, stats, error); else return qemuMonitorTextGetMigrationStats(mon, stats); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 40d90d0da6..57893c61c6 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -696,7 +696,8 @@ struct _qemuMonitorMigrationStats { }; int qemuMonitorGetMigrationStats(qemuMonitorPtr mon, - qemuMonitorMigrationStatsPtr stats); + qemuMonitorMigrationStatsPtr stats, + char **error); typedef enum { QEMU_MONITOR_MIGRATION_CAPS_XBZRLE, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5546d1aa1f..171cdf1b7c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2792,7 +2792,8 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, static int qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, - qemuMonitorMigrationStatsPtr stats) + qemuMonitorMigrationStatsPtr stats, + char **error) { virJSONValuePtr ret; virJSONValuePtr ram; @@ -2801,6 +2802,7 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, const char *statusstr; int rc; double mbps; + const char *tmp; ret = virJSONValueObjectGetObject(reply, "return"); @@ -2839,11 +2841,18 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, switch ((qemuMonitorMigrationStatus) stats->status) { case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: case QEMU_MONITOR_MIGRATION_STATUS_SETUP: - case QEMU_MONITOR_MIGRATION_STATUS_ERROR: case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED: case QEMU_MONITOR_MIGRATION_STATUS_LAST: break; + case QEMU_MONITOR_MIGRATION_STATUS_ERROR: + if (error) { + tmp = virJSONValueObjectGetString(ret, "error-desc"); + if (tmp && VIR_STRDUP(*error, tmp) < 0) + return -1; + } + break; + case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY: case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: @@ -2989,7 +2998,8 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, int qemuMonitorJSONGetMigrationStats(qemuMonitorPtr mon, - qemuMonitorMigrationStatsPtr stats) + qemuMonitorMigrationStatsPtr stats, + char **error) { int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-migrate", @@ -3007,7 +3017,7 @@ int qemuMonitorJSONGetMigrationStats(qemuMonitorPtr mon, if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - if (qemuMonitorJSONGetMigrationStatsReply(reply, stats) < 0) + if (qemuMonitorJSONGetMigrationStatsReply(reply, stats, error) < 0) goto cleanup; ret = 0; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index f418c74264..7c45be6725 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -141,7 +141,8 @@ int qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, qemuMonitorMigrationParamsPtr params); int qemuMonitorJSONGetMigrationStats(qemuMonitorPtr mon, - qemuMonitorMigrationStatsPtr stats); + qemuMonitorMigrationStatsPtr stats, + char **error); int qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon, char ***capabilities); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index df3ef0a932..475fd270e1 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1907,6 +1907,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationStats(const void *data) qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); int ret = -1; qemuMonitorMigrationStats stats, expectedStats; + char *error = NULL; if (!test) return -1; @@ -1931,21 +1932,43 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationStats(const void *data) " }" " }," " \"id\": \"libvirt-13\"" + "}") < 0 || + qemuMonitorTestAddItem(test, "query-migrate", + "{" + " \"return\": {" + " \"status\": \"failed\"," + " \"error-desc\": \"It's broken\"" + " }," + " \"id\": \"libvirt-14\"" "}") < 0) goto cleanup; - if (qemuMonitorJSONGetMigrationStats(qemuMonitorTestGetMonitor(test), &stats) < 0) + if (qemuMonitorJSONGetMigrationStats(qemuMonitorTestGetMonitor(test), + &stats, &error) < 0) + goto cleanup; + + if (memcmp(&stats, &expectedStats, sizeof(stats)) != 0 || error) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Invalid migration statistics"); + goto cleanup; + } + + memset(&stats, 0, sizeof(stats)); + if (qemuMonitorJSONGetMigrationStats(qemuMonitorTestGetMonitor(test), + &stats, &error) < 0) goto cleanup; - if (memcmp(&stats, &expectedStats, sizeof(stats)) != 0) { + if (stats.status != QEMU_MONITOR_MIGRATION_STATUS_ERROR || + STRNEQ_NULLABLE(error, "It's broken")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - "Invalid migration status"); + "Invalid failed migration status"); goto cleanup; } ret = 0; cleanup: qemuMonitorTestFree(test); + VIR_FREE(error); return ret; } -- GitLab