Re: [Qemu-devel] [PATCH v4 3/4] migration: Convert 'status' of MigrationInfo to use an enum type

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2184 bytes --]

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>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]