Re: [Qemu-devel] [PATCH RFC] migration: Convert 'status' of MigrationInfo to use an enum type

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

On 2015/2/26 23:24, Eric Blake wrote:
> On 02/26/2015 12:58 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: comppleted -> completed
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>
>
>> +++ b/qapi-schema.json
>> @@ -411,18 +411,42 @@
>>              'overflow': 'int' } }
>>
>>   ##
>> +# @MigState:
>> +#
>> +# An enumeration of migration status.
>> +#
>> +# @failed: some error occurred during migration process. (since 0.14.0)
>> +#
>> +# @none: no migration has happened ever.
>
> s/happened ever/ever happened/
>

>> +#
>> +# @setup: migration process has been initiated. (since 0.14.0)
>> +#
>> +# @cancelling: in the process of cancelling migration. (since 2.0)
>
> It's weird to advertise this enum value when the code goes to lengths to
> hide it behind 'active', but I guess it's okay.
>

Hmm, yes, this state is only used internally, and will return 'active' to user
instead, it is a problem left over by history, which introduced by 51cf4c1a commit.
It is no side effect, maybe a NOTE should be added.

>> +#
>> +# @cancelled: cancelling migration is finished. (since 0.14.0)
>> +#
>> +# @active: in the process of doing migration. (since 0.14.0)
>> +#
>> +# @completed: migration is finished. (since 0.14.0)
>> +#
>> +# Since: 2.3
>
> It's weird to see various enum values stating (since xyz) that is older
> than 2.3.  Generally, we only use that for an enum value that is newer
> than the enumeration.  I'd be just fine if you got rid of all of the
> per-value '(since xyz)' strings.
>

OK, will fix in v2.

> Otherwise, looks good to me.
>

Thanks.
zhanghailiang