Re: [Qemu-devel] [RFC PATCH 1/2] qga: Introduce guest-get-os-version command with stubs

[Eric Blake <eblake@redhat.com>]

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

On 12/16/2014 12:30 AM, zhanghailiang wrote:
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---

Might be nice to show an example (intended) usage of the new command in
the commit message.

> +++ b/qga/qapi-schema.json
> @@ -738,3 +738,29 @@
>  ##
>  { 'command': 'guest-get-fsinfo',
>    'returns': ['GuestFilesystemInfo'] }
> +##

Blank line between commands.

> +# @GuestOSVersion:
> +#
> +# @name: system version.
> +#
> +# @:type: 64-bit or 32-bit.

s/@:/@/

'type' feels like the wrong name for word-width.  Maybe 'word-width' is
a better name?

> +#
> +# Since: 2.3
> +##
> +{ 'type': 'GuestOSVersion',
> +  'data': {'name': 'str', 'type': 'int'} }

'name' feels a bit vague; it looks like you are intending to use the
string as a free-form text field, where the guest can supply arbitrary
strings.  Maybe name it 'info' instead? Is it worth being any stricter,
such as having actual enums of known values?  On the other hand, tying
to known enums means we have to update qemu-ga every time a new guest
gains support for running the agent, while free-form string leaves us a
bit more flexible.  Or maybe you want both, as in:

{ 'enum': 'GuestOSFamily', 'data': ['Windows', 'Linux', 'other'] }
{ 'type': 'GuestOSVersion',
  'data': { 'info': 'str', 'family': 'GuestOSFamily',
    'word-width': 'int' } }

> +
> +##
> +# @guest-get-os-version:
> +#
> +# Get the guest's operating system version and bit.

s/bit/word width/

> +#
> +# This is a read-only operation.
> +#
> +# Returns: version
> +#
> +# Since: 2.3
> +##
> +{ 'command': 'guest-get-os-version',
> +  'returns': 'GuestOSVersion' }

with my suggestions, a usage might be:
=> { "execute": "guest-get-os-version" }
<= { "return": { "family": "Windows",
     "info": "Microsoft Windows Server 2012 R2",
     "word-width": 64 } }

-- 
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: 539 bytes --]