Re: Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats'

[zhenwei pi]

On 7/6/22 15:20, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jul 6, 2022 at 7:09 AM zhenwei pi <pizhenwei@bytedance.com 
> <mailto:pizhenwei@bytedance.com>> wrote:
>
>     On 7/4/22 16:00, zhenwei pi wrote:
>      >
>      >
>      >>     +##
>      >>     +# @GuestOsType:
>      >>     +#
>      >>     +# An enumeration of OS type
>      >>     +#
>      >>     +# Since: 7.1
>      >>     +##
>      >>     +{ 'enum': 'GuestOsType',
>      >>     +  'data': [ 'linuxos', 'windowsos' ] }
>      >>
>      >>
>      >> I would rather keep this enum specific to GuestCpuStats,
>      >> "GuestLinuxCpuStatsType"?
>      >>
>      >
>      > Hi,
>      >
>      > 'GuestOsType' may be re-used in the future, not only for the CPU
>      > statistics case.
>      >
>      >> I would also drop the "os" suffix
>      >>
>      > I'm afraid we can not drop "os" suffix, build this without "os"
>     suffix:
>      > qga/qga-qapi-types.h:948:28: error: expected member name or ';'
>     after
>      > declaration specifiers
>      >          GuestLinuxCpuStats linux;
>      >          ~~~~~~~~~~~~~~~~~~ ^
>      > <built-in>:336:15: note: expanded from here
>      > #define linux 1
>      >
>
>     Hi, Marc
>
>     Could you please give any hint about this issue?
>
>
> Yes, it looks like we need to add "linux" to the "polluted_words":

OK, I'll fix this in the next versoin.

By the way, 'GuestCpuStatsType' seems to be used for CPU statistics 
only, but 'data': [ 'linux', 'windows' ] } is quite common, it may be 
used for other OS specified commands in the future. Should I use 
'GuestCpuStatsType' instead of 'GuestOsType'?

> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 489273574aee..737b059e6291 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -114,7 +114,7 @@ def c_name(name: str, protect: bool = True) -> str:
>                        'and', 'and_eq', 'bitand', 'bitor', 'compl', 'not',
>                        'not_eq', 'or', 'or_eq', 'xor', 'xor_eq'])
>       # namespace pollution:
> -    polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386'])
> +    polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386', 
> 'linux'])
>
>
>      >>     +
>      >>     +
>      >>
>      >>
>      >>
>      >> Looks good to me otherwise.
>      >> thanks
>      >>
>      >> --
>      >> Marc-André Lureau
>      >
>
>     -- 
>     zhenwei pi
>
>
>
> --
> Marc-André Lureau

--
zhenwei pi