Re: [Qemu-devel] [PATCH V9 12/20] qapi: Add new command to query colo status

[Markus Armbruster]

Zhang Chen <address@hidden> writes:

> On Tue, Jul 3, 2018 at 7:09 PM, Markus Armbruster <address@hidden> wrote:
>
>> Zhang Chen <address@hidden> writes:
>>
>> > Libvirt or other high level software can use this command query colo
>> status.
>> > You can test this command like that:
>> > {'execute':'query-colo-status'}
>> >
>> > Signed-off-by: Zhang Chen <address@hidden>
[...]
>> > diff --git a/qapi/migration.json b/qapi/migration.json
>> > index c24f114104..73c64686ec 100644
>> > --- a/qapi/migration.json
>> > +++ b/qapi/migration.json
>> > @@ -1248,6 +1248,40 @@
>> >  ##
>> >  { 'command': 'xen-colo-do-checkpoint' }
>> >
>> > +##
>> > +# @COLOStatus:
>> > +#
>> > +# The result format for 'query-colo-status'.
>> > +#
>> > +# @mode: COLO running mode. If COLO is running, this field will return
>> > +#        'primary' or 'secondary'.
>>
>> Please mention that @mode is "none" when COLO is not running.
>>
>
> OK.
>
>
>>
>> > +#
>> > +# @active: true if COLO is active.
>>
>> Please use consistent terminology: pick one of "COLO is running", "COLO
>> is active" and stick to it.  v8 had that here, v9 regressed.  Also use
>> this wording for the comment improvement I requested for PATCH 11.
>>
>> However, isn't @active redundant with @mode?
>>
>>
> I re-think about this and review the codes.
> The 'none' mode can cover this field, I will remove the @active in next
> version.

I like it.

> By the way, can you review the diagram patch as your comments?
> (have some action not related to qmp, so I just drew a general diagram.)

I need to find some time for it.  Do not hold the series until I do.