[libvirt] [PATCH v3 4/4] migration: Expose 'cancelling' status to user
'cancelling' status is introduced by commit 51cf4c1a, which is mainly avoid possible starting a new migration process while the previous one still exist. But we don't expose this status to user, instead by return a 'active' state. Here, we expose it to the user (such as libvirt), 'cancelling' status only occurs for a short window before the migration aborts, so for users, if they cancell a migration process, it will observe 'cancelling' status occasionally. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Cc: libvir-list@redhat.com --- migration/migration.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 035e005..a57928d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -179,13 +179,11 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_SETUP: info->has_status = true; - info->status = MIGRATION_STATUS_SETUP; info->has_total_time = false; break; case MIGRATION_STATUS_ACTIVE: case MIGRATION_STATUS_CANCELLING: info->has_status = true; - info->status = MIGRATION_STATUS_ACTIVE; info->has_total_time = true; info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s->total_time; @@ -221,7 +219,6 @@ MigrationInfo *qmp_query_migrate(Error **errp) get_xbzrle_cache_stats(info); info->has_status = true; - info->status = MIGRATION_STATUS_COMPLETED; info->has_total_time = true; info->total_time = s->total_time; info->has_downtime = true; @@ -243,13 +240,12 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_FAILED: info->has_status = true; - info->status = MIGRATION_STATUS_FAILED; break; case MIGRATION_STATUS_CANCELLED: info->has_status = true; - info->status = MIGRATION_STATUS_CANCELLED; break; } + info->status = s->state; return info; } -- 1.7.12.4
On 03/04/2015 07:09 AM, zhanghailiang wrote:
'cancelling' status is introduced by commit 51cf4c1a, which is mainly avoid
s/is introduced/was introduced/ s/which is mainly avoid/mainly to avoid a/
possible starting a new migration process while the previous one still exist.
s/starting a new/start of a new/ s/exist/exists/
But we don't expose this status to user, instead by return a 'active' state.
s/don't/didn't/ s/by return a/we returned the/
Here, we expose it to the user (such as libvirt), 'cancelling' status only occurs for a short window before the migration aborts, so for users, if they cancell a migration process, it will observe 'cancelling' status
s/cancell/cancel/
occasionally.
Add: Testing revealed that with older libvirt (anything 1.2.13 or less) will print an odd error message if the state is seen, but that the migration is still properly cancelled. Newer libvirt will be patched to recognize the new state without the odd error message.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Cc: libvir-list@redhat.com --- migration/migration.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
With the grammar in the commit message fixed, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2015/3/7 0:24, Eric Blake wrote:
On 03/04/2015 07:09 AM, zhanghailiang wrote:
'cancelling' status is introduced by commit 51cf4c1a, which is mainly avoid
s/is introduced/was introduced/ s/which is mainly avoid/mainly to avoid a/
possible starting a new migration process while the previous one still exist.
s/starting a new/start of a new/ s/exist/exists/
But we don't expose this status to user, instead by return a 'active' state.
s/don't/didn't/ s/by return a/we returned the/
Here, we expose it to the user (such as libvirt), 'cancelling' status only occurs for a short window before the migration aborts, so for users, if they cancell a migration process, it will observe 'cancelling' status
s/cancell/cancel/
occasionally.
Add: Testing revealed that with older libvirt (anything 1.2.13 or less) will print an odd error message if the state is seen, but that the migration is still properly cancelled. Newer libvirt will be patched to recognize the new state without the odd error message.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Cc: libvir-list@redhat.com --- migration/migration.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
With the grammar in the commit message fixed, Reviewed-by: Eric Blake <eblake@redhat.com>
OK, will fix that in v4, thanks.
In qemu 2.3, the migration status will include 'cancelling' in the window between when an asynchronous cancel has been requested and when the migration is actually halted. Previously, qemu hid this state and reported 'active'. Libvirt manages the sequence okay even when the string is unrecognized (that is, it will report an unknown state: Migration: [ 69 %]^Cerror: internal error: unexpected migration status in cancelling. but the migration is still cancelled), but recognizing the string makes for a smoother user experience. * src/qemu/qemu_monitor.h (QEMU_MONITOR_MIGRATION_STATUS_CANCELLING): Add enum. * src/qemu/qemu_monitor.c (qemuMonitorMigrationStatus): Map it. * src/qemu/qemu_migration.c (qemuMigrationUpdateJobStatus): Adjust clients. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONGetMigrationStatusReply): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_migration.c | 3 ++- src/qemu/qemu_monitor.c | 5 +++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 3 ++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20e40aa..c44b19b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1,7 +1,7 @@ /* * qemu_migration.c: QEMU migration handling * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -2297,6 +2297,7 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver, /* fall through */ case QEMU_MONITOR_MIGRATION_STATUS_SETUP: case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: + case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING: ret = 0; break; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 94495cd..d587606 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1,7 +1,7 @@ /* * qemu_monitor.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -117,7 +117,8 @@ VIR_ONCE_GLOBAL_INIT(qemuMonitor) VIR_ENUM_IMPL(qemuMonitorMigrationStatus, QEMU_MONITOR_MIGRATION_STATUS_LAST, - "inactive", "active", "completed", "failed", "cancelled", "setup") + "inactive", "active", "completed", "failed", "cancelling", + "cancelled", "setup") VIR_ENUM_IMPL(qemuMonitorMigrationCaps, QEMU_MONITOR_MIGRATION_CAPS_LAST, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 133d42d..9311926 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1,7 +1,7 @@ /* * qemu_monitor.h: interaction with QEMU monitor console * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -456,6 +456,7 @@ enum { QEMU_MONITOR_MIGRATION_STATUS_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_COMPLETED, QEMU_MONITOR_MIGRATION_STATUS_ERROR, + QEMU_MONITOR_MIGRATION_STATUS_CANCELLING, QEMU_MONITOR_MIGRATION_STATUS_CANCELLED, QEMU_MONITOR_MIGRATION_STATUS_SETUP, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 832f589..a4e02dc 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1,7 +1,7 @@ /* * qemu_monitor_json.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -2695,6 +2695,7 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, status->setup_time_set = true; if (status->status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE || + status->status == QEMU_MONITOR_MIGRATION_STATUS_CANCELLING || status->status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { virJSONValuePtr ram = virJSONValueObjectGet(ret, "ram"); if (!ram) { -- 2.1.0
On Fri, Mar 06, 2015 at 09:34:59 -0700, Eric Blake wrote:
In qemu 2.3, the migration status will include 'cancelling' in the window between when an asynchronous cancel has been requested and when the migration is actually halted. Previously, qemu hid this state and reported 'active'. Libvirt manages the sequence okay even when the string is unrecognized (that is, it will report an unknown state:
Migration: [ 69 %]^Cerror: internal error: unexpected migration status in cancelling.
but the migration is still cancelled), but recognizing the string makes for a smoother user experience.
ACK once the change is merged to QEMU. Jirka
On 03/09/2015 03:07 AM, Jiri Denemark wrote:
On Fri, Mar 06, 2015 at 09:34:59 -0700, Eric Blake wrote:
In qemu 2.3, the migration status will include 'cancelling' in the window between when an asynchronous cancel has been requested and when the migration is actually halted. Previously, qemu hid this state and reported 'active'. Libvirt manages the sequence okay even when the string is unrecognized (that is, it will report an unknown state:
Migration: [ 69 %]^Cerror: internal error: unexpected migration status in cancelling.
but the migration is still cancelled), but recognizing the string makes for a smoother user experience.
ACK once the change is merged to QEMU.
qemu commit cde63fbe, so I'm now pushing this to libvirt. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake -
Jiri Denemark -
zhanghailiang