On Wed, Jul 08, 2015 at 15:22:50 +0200, Jiri Denemark wrote:
> If QEMU fails during incoming migration, the domain disappears including
> a possibly useful error message read from QEMU log file. Let's remember
> the error in virQEMUDriver so that Finish can report more than just "no
> such domain".
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> src/qemu/qemu_conf.h | 3 +++
> src/qemu/qemu_driver.c | 31 +++++++++++++++++++-------
> src/qemu/qemu_migration.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-
> src/qemu/qemu_migration.h | 7 ++++++
> src/qemu/qemu_monitor.c | 19 ++++++++++++++++
> src/qemu/qemu_monitor.h | 2 ++
> src/qemu/qemu_process.c | 4 ++++
> 7 files changed, 112 insertions(+), 9 deletions(-)
>
...
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index a57a177..82069a1 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
...
> @@ -6051,3 +6054,53 @@ qemuMigrationJobFinish(virQEMUDriverPtr driver, virDomainObjPtr vm)
> {
> qemuDomainObjEndAsyncJob(driver, vm);
> }
> +
> +
> +static void
> +qemuMigrationErrorFree(void *data,
> + const void *name ATTRIBUTE_UNUSED)
> +{
> + virErrorPtr err = data;
> + virFreeError(err);
> +}
> +
> +int
> +qemuMigrationErrorInit(virQEMUDriverPtr driver)
> +{
> + driver->migrationErrors = virHashLockableNew(64, qemuMigrationErrorFree);
> + if (driver->migrationErrors)
> + return 0;
> + else
> + return -1;
> +}
> +
This function consumes @err. A comment noting that would be helpful.
> +void
> +qemuMigrationErrorSave(virQEMUDriverPtr driver,
> + const char *name,
> + virErrorPtr err)
> +{
> + if (!err)
> + return;
> +
> + VIR_DEBUG("Saving incoming migration error for domain %s: %s",
> + name, err->message);
> + if (virHashLockableUpdate(driver->migrationErrors, name, err) < 0) {
> + VIR_WARN("Failed to save migration error for domain '%s'", name);
> + virFreeError(err);
> + }
> +}
> +
> +void
> +qemuMigrationErrorReport(virQEMUDriverPtr driver,
> + const char *name)
> +{
> + virErrorPtr err;
> +
> + if (!(err = virHashLockableSteal(driver->migrationErrors, name)))
> + return;
> +
> + VIR_DEBUG("Restoring saved incoming migration error for domain %s: %s",
> + name, err->message);
> + virSetError(err);
> + virFreeError(err);
> +}
...
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 896d9fd..9db05c5 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1057,6 +1057,25 @@ qemuMonitorSend(qemuMonitorPtr mon,
> }
>
Telling that the user is responsible for freeing the value would be
helpful here.
>
> +virErrorPtr
> +qemuMonitorLastError(qemuMonitorPtr mon)
> +{
> + virErrorPtr old;
> + virErrorPtr err;
> +
> + if (mon->lastError.code == VIR_ERR_OK)
> + return NULL;
> +
> + old = virSaveLastError();
> + virSetError(&mon->lastError);
> + err = virSaveLastError();
> + virSetError(old);
> + virFreeError(old);
Ummm, how about exporting virCopyError rather than using this rather
opaque and ugly way to copy the error?
> +
> + return err;
> +}
> +
> +
> virJSONValuePtr
> qemuMonitorGetOptions(qemuMonitorPtr mon)
> {
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list