qapi/migration.json: Fix documentation issue about query_colo_status

[Qemu-devel] [PATCH] qapi/migration.json: Fix documentation issue about query_colo_status

Posted by Zhang Chen 5 years ago

From: Zhang Chen <chen.zhang@intel.com>

The documentation with the wrong initial version number of last_mode field,
This patch just fix this issue.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 qapi/migration.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index cfde29acf8..798c6ac2df 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1382,7 +1382,7 @@
 #
 # @last_mode: COLO last running mode. If COLO is running, this field
 #             will return same like mode field, after failover we can
-#             use this field to get last colo mode. (since 4.1)
+#             use this field to get last colo mode. (since 4.0)
 #
 # @reason: describes the reason for the COLO exit.
 #
-- 
2.17.GIT

Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation issue about query_colo_status

Posted by Eric Blake 5 years ago

On 3/26/19 12:45 PM, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
> 
> The documentation with the wrong initial version number of last_mode field,
> This patch just fix this issue.

Grammar is a bit odd, but I'll leave it up to a maintainer if they want
to improve it. I suggest a shorter:

Fix a wrong initial version number for last_mode.

> 
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  qapi/migration.json | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

Safe for 4.0, but not a showstopper if it slips.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation issue about query_colo_status

Posted by Markus Armbruster 5 years ago

Zhang Chen <chen.zhang@intel.com > writes:

> From: Zhang Chen <chen.zhang@intel.com>
>
> The documentation with the wrong initial version number of last_mode field,
> This patch just fix this issue.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  qapi/migration.json | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index cfde29acf8..798c6ac2df 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1382,7 +1382,7 @@
>  #
>  # @last_mode: COLO last running mode. If COLO is running, this field
>  #             will return same like mode field, after failover we can
> -#             use this field to get last colo mode. (since 4.1)
> +#             use this field to get last colo mode. (since 4.0)
>  #
>  # @reason: describes the reason for the COLO exit.
>  #

What's the excuse for spelling last_mode with '_' instead of '-'?

Any objection to changing it to last-mode?

Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation issue about query_colo_status

Posted by Zhang, Chen 5 years ago

> -----Original Message-----
> From: Markus Armbruster [mailto:armbru@redhat.com]
> Sent: Tuesday, April 2, 2019 2:20 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Zhang Chen <zhangckid@gmail.com>; Dr. David Alan Gilbert
> <dgilbert@redhat.com>; Juan Quintela <quintela@redhat.com>; zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Eric Blake <eblake@redhat.com>; qemu-
> dev <qemu-devel@nongnu.org>; Zhang, Chen <chen.zhang@intel.com>
> Subject: Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation
> issue about query_colo_status
> 
> Zhang Chen <chen.zhang@intel.com > writes:
> 
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > The documentation with the wrong initial version number of last_mode
> > field, This patch just fix this issue.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  qapi/migration.json | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qapi/migration.json b/qapi/migration.json index
> > cfde29acf8..798c6ac2df 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -1382,7 +1382,7 @@
> >  #
> >  # @last_mode: COLO last running mode. If COLO is running, this field
> >  #             will return same like mode field, after failover we can
> > -#             use this field to get last colo mode. (since 4.1)
> > +#             use this field to get last colo mode. (since 4.0)
> >  #
> >  # @reason: describes the reason for the COLO exit.
> >  #
> 
> What's the excuse for spelling last_mode with '_' instead of '-'?
> 
> Any objection to changing it to last-mode?

No, I just found in migration.json have both '_' and  '-', for example "migrate_cancel" and "migrate-continue".
If you think we should use the '-' format in this migration.qapi file, I will send a patch to change all command format to the '-'.

Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation issue about query_colo_status

Posted by Markus Armbruster 5 years ago

"Zhang, Chen" <chen.zhang@intel.com> writes:

>> -----Original Message-----
>> From: Markus Armbruster [mailto:armbru@redhat.com]
>> Sent: Tuesday, April 2, 2019 2:20 PM
>> To: Zhang, Chen <chen.zhang@intel.com>
>> Cc: Zhang Chen <zhangckid@gmail.com>; Dr. David Alan Gilbert
>> <dgilbert@redhat.com>; Juan Quintela <quintela@redhat.com>; zhanghailiang
>> <zhang.zhanghailiang@huawei.com>; Eric Blake <eblake@redhat.com>; qemu-
>> dev <qemu-devel@nongnu.org>; Zhang, Chen <chen.zhang@intel.com>
>> Subject: Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation
>> issue about query_colo_status
>> 
>> Zhang Chen <chen.zhang@intel.com > writes:
>> 
>> > From: Zhang Chen <chen.zhang@intel.com>
>> >
>> > The documentation with the wrong initial version number of last_mode
>> > field, This patch just fix this issue.
>> >
>> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
>> > ---
>> >  qapi/migration.json | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/qapi/migration.json b/qapi/migration.json index
>> > cfde29acf8..798c6ac2df 100644
>> > --- a/qapi/migration.json
>> > +++ b/qapi/migration.json
>> > @@ -1382,7 +1382,7 @@
>> >  #
>> >  # @last_mode: COLO last running mode. If COLO is running, this field
>> >  #             will return same like mode field, after failover we can
>> > -#             use this field to get last colo mode. (since 4.1)
>> > +#             use this field to get last colo mode. (since 4.0)
>> >  #
>> >  # @reason: describes the reason for the COLO exit.
>> >  #
>> 
>> What's the excuse for spelling last_mode with '_' instead of '-'?
>> 
>> Any objection to changing it to last-mode?
>
> No, I just found in migration.json have both '_' and  '-', for example "migrate_cancel" and "migrate-continue".
> If you think we should use the '-' format in this migration.qapi file, I will send a patch to change all command format to the '-'.

Quote docs/devel/qapi-code-gen.txt:

    Command names, and member names within a type, should be all lower
    case with words separated by a hyphen.  However, some existing older
    commands and complex types use underscore; when extending such
    expressions, consistency is preferred over blindly avoiding
    underscore.

The consistency argument doesn't apply here.

I'd prefer to have this cleaned up, and I'd prefer to have it done in
-rc2.  If I see a patch from you before I do my pull request, I'll use
it, otherweise I'll patch it myself.

Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation issue about query_colo_status

Posted by Zhang, Chen 5 years ago


> -----Original Message-----
> From: Markus Armbruster [mailto:armbru@redhat.com]
> Sent: Tuesday, April 2, 2019 4:37 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Markus Armbruster <armbru@redhat.com>; zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Juan Quintela <quintela@redhat.com>;
> qemu-dev <qemu-devel@nongnu.org>; Dr. David Alan Gilbert
> <dgilbert@redhat.com>; Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix documentation
> issue about query_colo_status
> 
> "Zhang, Chen" <chen.zhang@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Markus Armbruster [mailto:armbru@redhat.com]
> >> Sent: Tuesday, April 2, 2019 2:20 PM
> >> To: Zhang, Chen <chen.zhang@intel.com>
> >> Cc: Zhang Chen <zhangckid@gmail.com>; Dr. David Alan Gilbert
> >> <dgilbert@redhat.com>; Juan Quintela <quintela@redhat.com>;
> >> zhanghailiang <zhang.zhanghailiang@huawei.com>; Eric Blake
> >> <eblake@redhat.com>; qemu- dev <qemu-devel@nongnu.org>; Zhang, Chen
> >> <chen.zhang@intel.com>
> >> Subject: Re: [Qemu-devel] [PATCH] qapi/migration.json: Fix
> >> documentation issue about query_colo_status
> >>
> >> Zhang Chen <chen.zhang@intel.com > writes:
> >>
> >> > From: Zhang Chen <chen.zhang@intel.com>
> >> >
> >> > The documentation with the wrong initial version number of
> >> > last_mode field, This patch just fix this issue.
> >> >
> >> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> >> > ---
> >> >  qapi/migration.json | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/qapi/migration.json b/qapi/migration.json index
> >> > cfde29acf8..798c6ac2df 100644
> >> > --- a/qapi/migration.json
> >> > +++ b/qapi/migration.json
> >> > @@ -1382,7 +1382,7 @@
> >> >  #
> >> >  # @last_mode: COLO last running mode. If COLO is running, this field
> >> >  #             will return same like mode field, after failover we can
> >> > -#             use this field to get last colo mode. (since 4.1)
> >> > +#             use this field to get last colo mode. (since 4.0)
> >> >  #
> >> >  # @reason: describes the reason for the COLO exit.
> >> >  #
> >>
> >> What's the excuse for spelling last_mode with '_' instead of '-'?
> >>
> >> Any objection to changing it to last-mode?
> >
> > No, I just found in migration.json have both '_' and  '-', for example
> "migrate_cancel" and "migrate-continue".
> > If you think we should use the '-' format in this migration.qapi file, I will send
> a patch to change all command format to the '-'.
> 
> Quote docs/devel/qapi-code-gen.txt:
> 
>     Command names, and member names within a type, should be all lower
>     case with words separated by a hyphen.  However, some existing older
>     commands and complex types use underscore; when extending such
>     expressions, consistency is preferred over blindly avoiding
>     underscore.
> 
> The consistency argument doesn't apply here.
> 
> I'd prefer to have this cleaned up, and I'd prefer to have it done in -rc2.  If I see
> a patch from you before I do my pull request, I'll use it, otherweise I'll patch it
> myself.

Hi Markus,

I have sent a patch for this issue, please pick up it.
[PATCH] qapi/migration.json: Clean up for COLOStatus

Thanks
Zhang Chen