Re: [libvirt] [PATCH] qemu: Check QEMU error on failed migration

On Thu, Oct 12, 2017 at 03:48:29PM +0200, Jiri Denemark wrote: > 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 <jdenemar(a)redhat.com&gt;

> - if (qemuMonitorJSONGetMigrationStats(qemuMonitorTestGetMonitor(test), &stats) < 0) > + if (qemuMonitorJSONGetMigrationStats(qemuMonitorTestGetMonitor(test), > + &stats, &error) < 0) > goto cleanup; > > - if (memcmp(&stats, &expectedStats, sizeof(stats)) != 0) { > + if (memcmp(&stats, &expectedStats, sizeof(stats)) != 0 || error) { > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - "Invalid migration status"); > + "Invalid migration statistics"); > + goto cleanup; > + } Do we need to pass the "&error" for the first call of qemuMonitorJSONGetMigrationStats() since we know the answer?

> + > + memset(&stats, 0, sizeof(stats)); > + if (qemuMonitorJSONGetMigrationStats(qemuMonitorTestGetMonitor(test), > + &stats, &error) < 0) > + goto cleanup; > + > + if (stats.status != QEMU_MONITOR_MIGRATION_STATUS_ERROR || > + STRNEQ_NULLABLE(error, "It's broken")) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + "Invalid failed migration status"); > goto cleanup; > } > > ret = 0; > cleanup: > qemuMonitorTestFree(test); > + VIR_FREE(error); > return ret; > } Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt;