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

2017-03-28 Thread Marc-André Lureau 
Hi

On Tue, Mar 28, 2017 at 7:15 AM Vinzenz Feenstra 
wrote:

> Eric, what you think of this below way, instead?
>
> >
> > 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’
> > }}
> >
>

I think this will be rejected. You could instead use a flat 'union' with a
'type' (see SocketAddressFlat for ex).

Otherwise, I like the approach better if the various os-specific details
are needed.

Wouldn't it be nice instead to return xdg os-release content? On OS that
don't have it, it could be crafted manually. Just an idea..

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

-- 
Marc-André Lureau

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

2017-03-27 Thread Vinzenz Feenstra 

> On Mar 24, 2017, at 10:48 AM, Vinzenz Feenstra  wrote:
> 
>> 
>> On Mar 23, 2017, at 3:58 PM, Eric Blake > > wrote:
>> 
>> On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
>>> From: Vinzenz Feenstra mailto:vfeen...@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 >> >
>>> ---
>> 
>> 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
>>  
>> 
>> 
>>The  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?

Eric, what you think of this below way, instead?

> 
> 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’,
> } }
> 
>

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

2017-03-24 Thread Vinzenz Feenstra 

> On Mar 23, 2017, at 3:58 PM, Eric Blake  wrote:
> 
> On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
>> From: Vinzenz Feenstra 
>> 
>> 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 
>> ---
> 
> 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
>  
> 
> 
>The  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

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

2017-03-23 Thread Eric Blake 
On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
> From: Vinzenz Feenstra 
> 
> 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 
> ---

Let's make sure we get the interface right, before I even bother looking
at the code.

> +++ 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?

> +# 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

The  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?

> +
> +##
> +# @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



signature.asc
Description: OpenPGP digital signature

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

2017-03-22 Thread Vinzenz 'evilissimo' Feenstra 
From: Vinzenz Feenstra 

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 
---
 qga/commands-posix.c | 189 +++
 qga/commands-win32.c | 104 
 qga/qapi-schema.json |  40 +++
 3 files changed, 333 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 73d93eb..381c01a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include 
+#include 
 #include 
 #include 
 #include "qga/guest-agent-core.h"
@@ -2356,6 +2357,188 @@ GuestMemoryBlockInfo 
*qmp_guest_get_memory_block_info(Error **errp)
 return info;
 }
 
+static void ga_strip_end(char *value)
+{
+size_t value_length = strlen(value);
+while (value_length > 0) {
+switch (value[value_length - 1]) {
+default:
+value_length = 0;
+break;
+case ' ': case '\n': case '\t': case '\'': case '"':
+value[value_length - 1] = 0;
+--value_length;
+break;
+}
+}
+}
+
+static void ga_parse_version_id(char const *value, GuestOSInfo *info)
+{
+if (strlen(value) < 128) {
+char codename[128];
+char version[128];
+
+if (*value == '"') {
+++value;
+}
+
+if (sscanf(value, "%[^(] (%[^)])", version, codename) == 2) {
+/* eg. VERSION="16.04.1 LTS (Xenial Xerus)" */
+info->codename = g_strdup(codename);
+info->version = g_strdup(version);
+} else if (sscanf(value, "%[^,] %[^\"]\"", version, codename) == 2) {
+/* eg. VERSION="12.04.5 LTS, Precise Pangolin" */
+info->codename = g_strdup(codename);
+info->version = g_strdup(version);
+} else {
+/* Just use the rest */
+info->version = g_strdup(version);
+}
+}
+}
+
+static void ga_parse_debian_version(FILE *fp, GuestOSInfo *info)
+{
+char *line = NULL;
+size_t n = 0;
+
+if (getline(&line, &n, fp) != -1) {
+ga_strip_end(line);
+info->version = g_strdup(line);
+info->distribution = g_strdup("Debian GNU/Linux");
+}
+free(line);
+}
+
+static void ga_parse_redhat_release(FILE *fp, GuestOSInfo *info)
+{
+char *line = NULL;
+size_t n = 0;
+
+if (getline(&line, &n, fp) != -1) {
+char *value = strstr(line, " release ");
+if (value != NULL) {
+*value = 0;
+info->distribution = g_strdup(line);
+value += 9;
+ga_strip_end(value);
+ga_parse_version_id(value, info);
+}
+}
+free(line);
+}
+
+static void ga_parse_os_release(FILE *fp, GuestOSInfo *info)
+{
+char *line = NULL;
+size_t n = 0;
+
+while (getline(&line, &n, fp) != -1) {
+char *value = strstr(line, "=");
+if (value != NULL) {
+*value = 0;
+++value;
+ga_strip_end(value);
+
+size_t len = strlen(line);
+if (len == 9 && strcmp(line, "VERSION_ID") == 0) {
+info->version = g_strdup(value);
+} else if (len == 7 && strcmp(line, "VERSION") == 0) {
+ga_parse_version_id(value, info);
+} else if (len == 4 && strcmp(line, "NAME") == 0) {
+info->distribution = g_strdup(value);
+}
+}
+}
+free(line);
+}
+
+static char *ga_stripped_strdup(char const *value)
+{
+char *result = NULL;
+while (value && *value == '"') {
+++value;
+}
+result = g_strdup(value);
+ga_strip_end(result);
+return result;
+}
+
+static void ga_parse_lsb_release(FILE *fp, GuestOSInfo *info)
+{
+char *line = NULL;
+size_t n = 0;
+
+while (getline(&line, &n, fp) != -1) {
+char *value = strstr(line, "=");
+if (value != NULL) {
+*value = 0;
+++value;
+ga_strip_end(value);
+
+size_t len = strlen(line);
+if (len == 15 && strcmp(line, "DISTRIB_RELEASE") == 0) {
+info->version = ga_stripped_strdup(value);
+} else if (len == 16 &&