Re: [Qemu-devel] [PATCH v2] qemu-ga: add guest-get-osinfo command

[Vinzenz Feenstra <vfeenstr@redhat.com>]

> On Mar 23, 2017, at 3:58 PM, Eric Blake <eblake@redhat.com> wrote:
> 
> On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
>> From: Vinzenz Feenstra <vfeenstr@redhat.com>
>> 
>> Add a new 'guest-get-osinfo' command for reporting basic information of
>> the guest operating system (hereafter just 'OS'). This information
>> includes the type of the OS, the version, and the architecture.
>> Additionally reported would be a name, distribution type and kernel
>> version where applicable.
>> 
>> Here an example for a Fedora 25 VM:
>> 
>> $ virsh -c qemu:////system qemu-agent-command F25 \
>>    '{ "execute": "guest-get-osinfo" }'
>>  {"return":{"arch":"x86_64","codename":"Server Edition","version":"25",
>>   "kernel":"4.8.6-300.fc25.x86_64","type":"linux","distribution":"Fedora"}}
>> 
>> And an example for a Windows 2012 R2 VM:
>> 
>> $ virsh -c qemu:////system qemu-agent-command Win2k12R2 \
>>    '{ "execute": "guest-get-osinfo" }'
>>  {"return":{"arch":"x86_64","codename":"Win 2012 R2",
>>   "version":"6.3","kernel":"","type":"windows","distribution":""}}
>> 
>> Signed-off-by: Vinzenz Feenstra <vfeenstr@redhat.com>
>> ---
> 
> Let's make sure we get the interface right, before I even bother looking
> at the code.

Sure

> 
>> +++ b/qga/qapi-schema.json
>> @@ -1042,3 +1042,43 @@
>>   'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
>>                '*input-data': 'str', '*capture-output': 'bool' },
>>   'returns': 'GuestExec' }
>> +
>> +##
>> +# @GuestOSType:
>> +#
>> +# @linux:    Indicator for linux distributions
>> +# @windows:  Indicator for windows versions
>> +# @other:    Indicator for any other operating system that is not yet
>> +#            explicitly supported
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'enum': 'GuestOSType', 'data': ['linux', 'windows', 'other'] }
>> +
>> +##
>> +# @GuestOSInfo:
>> +#
>> +# @version:      OS version, e.g. 25 for FC25 etc.
>> +# @distribution: Fedora, Ubuntu, Debian, CentOS...
>> +# @codename:     Code name of the OS. e.g. Ubuntu has Xenial Xerus etc.
>> +# @arch:         Architecture of the OS e.g. x86, x86_64, ppc64, aarch64...
>> +# @type:         Specifies the type of the OS.
>> +# @kernel:       Linux kernel version (Might be used by other OS types too).
>> +#                May be empty.
> 
> Why would it be empty, instead of just omitted?
That’s a relict of v1 where this was still the case and all fields were supposed to be there all the time.
> 
>> +# Since: 2.10
>> +##
>> +{ 'struct': 'GuestOSInfo',
>> +  'data': { '*version': 'str', '*distribution': 'str', '*codename': 'str',
>> +            '*arch': 'str', 'type': 'GuestOSType', '*kernel': 'str'} }
> 
> So everything except 'type' is optional?  Is there anything a client can
> actually rely on being always present?
> 
> uname() is probably the lowest-common-denominator interface for
> describing a system (in that it is implemented on more OSs than anything
> else, and Windows information can be mapped to uname concepts).  Let's
> compare:
> 
> POSIX says:
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_utsname.h.html#tag_13_70 <http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_utsname.h.html#tag_13_70>
> 
>    The <sys/utsname.h> header shall define the structure utsname which
> shall include at least the following members:
> 
>    char  sysname[]  Name of this implementation of the operating system.
>    char  nodename[] Name of this node within the communications
>                     network to which this node is attached, if any.
>    char  release[]  Current release level of this implementation.
>    char  version[]  Current version level of this release.
>    char  machine[]  Name of the hardware type on which the system is
> running.
> 
> So you both have version; your 'arch' maps somewhat to machine, your
> 'type' maps somewhat to sysname, your 'kernel' probably matches release,
> and then you are adding 'distribution' and 'codename' which don't appear
> in uname output.  Is it worth naming your members after the uname()
> names, for less confusion?

OK So how about something like this:

{‘struct’: ‘GuestWindowsVersionInfo’,
 ‘data’: {
	‘version’: ‘str’,
        ‘name’: ‘str’,
} }

{‘struct’: ‘GuestLinuxOSRelease’,
 ‘data’: {
        ‘id’: ‘str’,
        ‘name’: ‘str'
	‘*version’: ‘str’,
        ‘*variant’: ‘str’
        ‘*codename’: ‘str’
} }

{‘enum’: ‘GuestOSVersionInfo’,
 ‘data’: {‘linux’: ‘ GuestLinuxOSRelease’,
             ‘win’: ‘ GuestWindowsVersionInfo’
}}

{ 'struct': 'GuestOSInfo’,
    'data': { 
	‘machine': 'str’,
	’sysname': 'GuestOSType’, 
	‘release': ‘str’,
	‘*version_info’: ‘GuestOSVersionInfo’,
} }

> 
>> +
>> +##
>> +# @guest-get-osinfo:
>> +#
>> +# Retrieve guest operating system information
>> +#
>> +# Returns: operating system information on success
>> +#
>> +# Since 2.10
>> +##
>> +{ 'command': 'guest-get-osinfo',
>> +  'returns': 'GuestOSInfo' }
>> 
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org <http://libvirt.org/>