| Message ID | 1425883543-8852-4-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 03/09/2015 12:45 AM, zhanghailiang wrote: > The original 'status' is an open-coded 'str' type, convert it to use an > enum type. > This conversion is backwards compatible, better documented and > more convenient for future extensibility. > > In addition, Fix a typo for qapi-schema.json (just remove the typo) : > s/'completed'. 'comppleted' (since 1.2)/'completed' (since 1.2) > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > hmp.c | 7 ++++--- > migration/migration.c | 20 +++++--------------- > qapi-schema.json | 34 +++++++++++++++++++++++++++++----- > 3 files changed, 38 insertions(+), 23 deletions(-) > > -enum { > - MIGRATION_STATUS_FAILED = -1, > - MIGRATION_STATUS_NONE, Note that we are changing any 0-initialized struct from having STATUS_NONE... > +## > +{ 'enum': 'MigrationStatus', > + 'data': [ 'failed', 'none', 'setup', 'cancelling', 'cancelled', > + 'active', 'completed' ] } ...to now having status _FAILED. Fortunately, in migration/migration.c (the only caller prior to this conversion), I didn't spot any zero-initialization (migrate_get_current() returns a struct that is explicitly initialized to _NONE). So it looks correct. > + > ## > # @MigrationInfo > # > # Information about current migration process. > # > -# @status: #optional string describing the current migration status. > -# As of 0.14.0 this can be 'setup', 'active', 'completed', 'failed' or > -# 'cancelled'. If this field is not returned, no migration process > +# @status: #optional @MigState describing the current migration status. s/MigState/MigrationStatus/ > + 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', > '*disk': 'MigrationStats', Hmm. "Status" and "Stats" look awfully close to one another. But I can live with the two names (it's not worth a respin just for the rename). With the typo fix pointed out in the documentation, Reviewed-by: Eric Blake <eblake@redhat.com>
On 2015/3/13 3:37, Eric Blake wrote: > On 03/09/2015 12:45 AM, zhanghailiang wrote: >> The original 'status' is an open-coded 'str' type, convert it to use an >> enum type. >> This conversion is backwards compatible, better documented and >> more convenient for future extensibility. >> >> In addition, Fix a typo for qapi-schema.json (just remove the typo) : >> s/'completed'. 'comppleted' (since 1.2)/'completed' (since 1.2) >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> --- >> hmp.c | 7 ++++--- >> migration/migration.c | 20 +++++--------------- >> qapi-schema.json | 34 +++++++++++++++++++++++++++++----- >> 3 files changed, 38 insertions(+), 23 deletions(-) > >> >> -enum { >> - MIGRATION_STATUS_FAILED = -1, >> - MIGRATION_STATUS_NONE, > > Note that we are changing any 0-initialized struct from having > STATUS_NONE... > > >> +## >> +{ 'enum': 'MigrationStatus', >> + 'data': [ 'failed', 'none', 'setup', 'cancelling', 'cancelled', >> + 'active', 'completed' ] } > > ...to now having status _FAILED. Fortunately, in migration/migration.c > (the only caller prior to this conversion), I didn't spot any > zero-initialization (migrate_get_current() returns a struct that is > explicitly initialized to _NONE). So it looks correct. > Hmm, however, i think it deserves a small modification, i will adjust the place of 'failed', which i will move it to the behind of 'completed' in v5. In this way, all of these enum type macros except 'failed' will be consistent with there original value. >> + >> ## >> # @MigrationInfo >> # >> # Information about current migration process. >> # >> -# @status: #optional string describing the current migration status. >> -# As of 0.14.0 this can be 'setup', 'active', 'completed', 'failed' or >> -# 'cancelled'. If this field is not returned, no migration process >> +# @status: #optional @MigState describing the current migration status. > > s/MigState/MigrationStatus/ > fix in v5. > >> + 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', >> '*disk': 'MigrationStats', > > Hmm. "Status" and "Stats" look awfully close to one another. But I can > live with the two names (it's not worth a respin just for the rename). > With the typo fix pointed out in the documentation, > I will keep it like that. Thanks. > Reviewed-by: Eric Blake <eblake@redhat.com> >
diff --git a/hmp.c b/hmp.c index 7300ddc..cffc94f 100644 --- a/hmp.c +++ b/hmp.c @@ -162,7 +162,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) } if (info->has_status) { - monitor_printf(mon, "Migration status: %s\n", info->status); + monitor_printf(mon, "Migration status: %s\n", + MigrationStatus_lookup[info->status]); monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n", info->total_time); if (info->has_expected_downtime) { @@ -1344,8 +1345,8 @@ static void hmp_migrate_status_cb(void *opaque) MigrationInfo *info; info = qmp_query_migrate(NULL); - if (!info->has_status || strcmp(info->status, "active") == 0 || - strcmp(info->status, "setup") == 0) { + if (!info->has_status || info->status == MIGRATION_STATUS_ACTIVE || + info->status == MIGRATION_STATUS_SETUP) { if (info->has_disk) { int progress; diff --git a/migration/migration.c b/migration/migration.c index f6c998b..035e005 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -26,16 +26,6 @@ #include "qmp-commands.h" #include "trace.h" -enum { - MIGRATION_STATUS_FAILED = -1, - MIGRATION_STATUS_NONE, - MIGRATION_STATUS_SETUP, - MIGRATION_STATUS_CANCELLING, - MIGRATION_STATUS_CANCELLED, - MIGRATION_STATUS_ACTIVE, - MIGRATION_STATUS_COMPLETED, -}; - #define MAX_THROTTLE (32 << 20) /* Migration speed throttling */ /* Amount of time to allocate to each "chunk" of bandwidth-throttled @@ -189,13 +179,13 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_SETUP: info->has_status = true; - info->status = g_strdup("setup"); + 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 = g_strdup("active"); + info->status = MIGRATION_STATUS_ACTIVE; info->has_total_time = true; info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - s->total_time; @@ -231,7 +221,7 @@ MigrationInfo *qmp_query_migrate(Error **errp) get_xbzrle_cache_stats(info); info->has_status = true; - info->status = g_strdup("completed"); + info->status = MIGRATION_STATUS_COMPLETED; info->has_total_time = true; info->total_time = s->total_time; info->has_downtime = true; @@ -253,11 +243,11 @@ MigrationInfo *qmp_query_migrate(Error **errp) break; case MIGRATION_STATUS_FAILED: info->has_status = true; - info->status = g_strdup("failed"); + info->status = MIGRATION_STATUS_FAILED; break; case MIGRATION_STATUS_CANCELLED: info->has_status = true; - info->status = g_strdup("cancelled"); + info->status = MIGRATION_STATUS_CANCELLED; break; } diff --git a/qapi-schema.json b/qapi-schema.json index e16f8eb..2b93b27 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -410,19 +410,43 @@ 'cache-miss': 'int', 'cache-miss-rate': 'number', 'overflow': 'int' } } +# @MigrationStatus: +# +# An enumeration of migration status. +# +# @failed: some error occurred during migration process. +# +# @none: no migration has ever happened. +# +# @setup: migration process has been initiated. +# +# @cancelling: in the process of cancelling migration. +# +# @cancelled: cancelling migration is finished. +# +# @active: in the process of doing migration. +# +# @completed: migration is finished. +# +# Since: 2.3 +# +## +{ 'enum': 'MigrationStatus', + 'data': [ 'failed', 'none', 'setup', 'cancelling', 'cancelled', + 'active', 'completed' ] } + ## # @MigrationInfo # # Information about current migration process. # -# @status: #optional string describing the current migration status. -# As of 0.14.0 this can be 'setup', 'active', 'completed', 'failed' or -# 'cancelled'. If this field is not returned, no migration process +# @status: #optional @MigState describing the current migration status. +# If this field is not returned, no migration process # has been initiated # # @ram: #optional @MigrationStats containing detailed migration # status, only returned if status is 'active' or -# 'completed'. 'comppleted' (since 1.2) +# 'completed'(since 1.2) # # @disk: #optional @MigrationStats containing detailed disk migration # status, only returned if status is 'active' and it is a block @@ -453,7 +477,7 @@ # Since: 0.14.0 ## { 'type': 'MigrationInfo', - 'data': {'*status': 'str', '*ram': 'MigrationStats', + 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', '*disk': 'MigrationStats', '*xbzrle-cache': 'XBZRLECacheStats', '*total-time': 'int',
The original 'status' is an open-coded 'str' type, convert it to use an enum type. This conversion is backwards compatible, better documented and more convenient for future extensibility. In addition, Fix a typo for qapi-schema.json (just remove the typo) : s/'completed'. 'comppleted' (since 1.2)/'completed' (since 1.2) Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- hmp.c | 7 ++++--- migration/migration.c | 20 +++++--------------- qapi-schema.json | 34 +++++++++++++++++++++++++++++----- 3 files changed, 38 insertions(+), 23 deletions(-)