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

Vinzenz 'evilissimo' Feenstra posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170316102351.25056-1-vfeenstr@redhat.com
Test s390x passed
There is a newer version of this series
qga/commands-posix.c | 206 +++++++++++++++++++++++++++++++++++++++++++++++++++
qga/commands-win32.c | 105 ++++++++++++++++++++++++++
qga/qapi-schema.json |  40 ++++++++++
3 files changed, 351 insertions(+)
[Qemu-devel] [PATCH] qemu-ga: add guest-get-osinfo command
Posted by Vinzenz 'evilissimo' Feenstra 7 years, 1 month ago
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>
---
 qga/commands-posix.c | 206 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/commands-win32.c | 105 ++++++++++++++++++++++++++
 qga/qapi-schema.json |  40 ++++++++++
 3 files changed, 351 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 73d93eb..70ec3fb 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include <sys/ioctl.h>
+#include <sys/utsname.h>
 #include <sys/wait.h>
 #include <dirent.h>
 #include "qga/guest-agent-core.h"
@@ -2356,6 +2357,205 @@ 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 = strdup(codename);
+            info->version = strdup(version);
+        } else if (sscanf(value, "%[^,] %[^\"]\"", version, codename) == 2) {
+            /* eg. VERSION="12.04.5 LTS, Precise Pangolin" */
+            info->codename = strdup(codename);
+            info->version = strdup(version);
+        } else {
+            /* Just use the rest */
+            info->version = strdup(version);
+            info->codename = strdup("");
+        }
+    }
+}
+
+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 = strdup(line);
+        info->codename = strdup("");
+        info->distribution = 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 = 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 = 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 = strdup(value);
+            }
+        }
+    }
+    free(line);
+}
+
+static char *ga_stripped_strdup(char const *value)
+{
+    char *result = NULL;
+    while (value && *value == '"') {
+        ++value;
+    }
+    result = 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 && strcmp(line, "DISTRIB_CODENAME") == 0) {
+                info->codename = ga_stripped_strdup(value);
+            } else if (len == 10 && strcmp(line, "DISTRIB_ID") == 0) {
+                info->distribution = ga_stripped_strdup(value);
+            }
+        }
+    }
+}
+
+static void ga_get_linux_distribution_info(GuestOSInfo *info)
+{
+    FILE *fp = NULL;
+    fp = fopen("/etc/os-release", "r");
+    if (fp != NULL) {
+        ga_parse_os_release(fp, info);
+        goto cleanup;
+    }
+    fp = fopen("/usr/lib/os-release", "r");
+    if (fp != NULL) {
+        ga_parse_os_release(fp, info);
+        goto cleanup;
+    }
+    fp = fopen("/etc/lsb-release", "r");
+    if (fp != NULL) {
+        ga_parse_lsb_release(fp, info);
+        goto cleanup;
+    }
+    fp = fopen("/etc/redhat-release", "r");
+    if (fp != NULL) {
+        ga_parse_redhat_release(fp, info);
+        goto cleanup;
+    }
+    fp = fopen("/etc/gentoo-release", "r");
+    if (fp != NULL) {
+        ga_parse_redhat_release(fp, info);
+        goto cleanup;
+    }
+    fp = fopen("/etc/debian_version", "r");
+    if (fp != NULL) {
+        ga_parse_debian_version(fp, info);
+        goto cleanup;
+    }
+
+    if (fp == NULL) {
+        info->distribution = strdup("Unknown");
+        info->version = strdup("");
+        info->codename = strdup("");
+    }
+
+cleanup:
+    if (fp != NULL) {
+        fclose(fp);
+    }
+
+}
+
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+    GuestOSInfo *info = g_new0(GuestOSInfo, 1);
+    struct utsname kinfo;
+
+    ga_get_linux_distribution_info(info);
+
+    if (!info->codename || !info->distribution || !info->version) {
+        qapi_free_GuestOSInfo(info);
+        return NULL;
+    }
+    info->type = GUESTOS_TYPE_LINUX;
+    uname(&kinfo);
+    info->kernel = g_strdup(kinfo.release);
+    info->arch = g_strdup(kinfo.machine);
+
+    return info;
+}
+
 #else /* defined(__linux__) */
 
 void qmp_guest_suspend_disk(Error **errp)
@@ -2418,6 +2618,12 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
     return NULL;
 }
 
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
 #endif
 
 #if !defined(CONFIG_FSFREEZE)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 19d72b2..b11cfde 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1536,3 +1536,108 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
         ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
     }
 }
+
+typedef struct _ga_matrix_lookup_t {
+    int major;
+    int minor;
+    char const *name;
+} ga_matrix_lookup_t;
+
+static ga_matrix_lookup_t const WIN_VERSION_MATRIX[2][8] = {
+    {
+        { 5, 0, "Win 2000"},
+        { 5, 1, "Win XP"},
+        { 6, 0, "Win Vista"},
+        { 6, 1, "Win 7"},
+
+        { 6, 2, "Win 8"},
+        { 6, 3, "Win 8.1"},
+        {10, 0, "Win 10"},
+        { 0, 0, 0}
+    },{
+        { 5, 2, "Win 2003"},
+        { 6, 0, "Win 2008"},
+        { 6, 1, "Win 2008 R2"},
+        { 6, 2, "Win 2012"},
+        { 6, 3, "Win 2012 R2"},
+        {10, 0, "Win 2016"},
+        { 0, 0, 0},
+        { 0, 0, 0}
+    }
+};
+
+static void ga_get_version(OSVERSIONINFOEXW *info)
+{
+    typedef NTSTATUS(WINAPI * rtl_get_version_t)(
+        OSVERSIONINFOEXW *os_version_info_ex);
+    HMODULE module = GetModuleHandle("ntdll");
+    PVOID fun = GetProcAddress(module, "RtlGetVersion");
+    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
+    rtl_get_version(info);
+}
+
+static char *ga_get_win_ver(void)
+{
+    OSVERSIONINFOEXW os_version;
+    ga_get_version(&os_version);
+    char buf[64] = {};
+    int major = (int)os_version.dwMajorVersion;
+    int minor = (int)os_version.dwMinorVersion;
+    sprintf(buf, "%d.%d", major, minor);
+    return strdup(buf);
+}
+
+static char *ga_get_win_name(void)
+{
+    OSVERSIONINFOEXW os_version;
+    ga_get_version(&os_version);
+    int major = (int)os_version.dwMajorVersion;
+    int minor = (int)os_version.dwMinorVersion;
+    int tbl_idx = (os_version.wProductType != VER_NT_WORKSTATION);
+    ga_matrix_lookup_t const *table = WIN_VERSION_MATRIX[tbl_idx];
+    while (table->name != NULL) {
+        if (major == table->major && minor == table->minor) {
+            return strdup(table->name);
+        }
+        ++table;
+    }
+    return strdup("N/A");
+}
+
+static char *ga_get_current_arch(void)
+{
+    SYSTEM_INFO info;
+    GetNativeSystemInfo(&info);
+    char *result = NULL;
+    switch (info.wProcessorArchitecture) {
+    case PROCESSOR_ARCHITECTURE_AMD64:
+        result = strdup("x86_64");;
+        break;
+    case PROCESSOR_ARCHITECTURE_ARM:
+        result = strdup("arm");;
+        break;
+    case PROCESSOR_ARCHITECTURE_IA64:
+        result = strdup("ia64");;
+        break;
+    case PROCESSOR_ARCHITECTURE_INTEL:
+        result = strdup("x86");;
+        break;
+    case PROCESSOR_ARCHITECTURE_UNKNOWN: default:
+        result = strdup("N/A");;
+        break;
+    }
+    return result;
+}
+
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+    GuestOSInfo *info = g_new0(GuestOSInfo, 1);
+    info->type = GUESTOS_TYPE_WINDOWS;
+    info->version = ga_get_win_ver();
+    info->codename = ga_get_win_name();
+    info->arch = ga_get_current_arch();
+    /* Not available on Windows */
+    info->kernel = strdup("");
+    info->distribution = strdup("");
+    return info;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a02dbf2..e31cb17 100644
--- a/qga/qapi-schema.json
+++ 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.8
+##
+{ '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.
+# Since: 2.8
+##
+{ 'struct': 'GuestOSInfo',
+  'data': { 'version': 'str', 'distribution': 'str', 'codename': 'str',
+            'arch': 'str', 'type': 'GuestOSType', 'kernel': 'str'} }
+
+##
+# @guest-get-osinfo:
+#
+# Retrieve guest operating system information
+#
+# Returns: operating system information on success
+#
+# Since 2.8
+##
+{ 'command': 'guest-get-osinfo',
+  'returns': 'GuestOSInfo' }
-- 
2.9.3


Re: [Qemu-devel] [PATCH] qemu-ga: add guest-get-osinfo command
Posted by Eric Blake 7 years ago
On 03/16/2017 05:23 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>
> ---


> +static void ga_get_linux_distribution_info(GuestOSInfo *info)
> +{
> +    FILE *fp = NULL;
> +    fp = fopen("/etc/os-release", "r");

Rather than read random files, I'm thinking that the uname() syscall is
a better approach.  It is portable to more guests (since uname is a
POSIX interface), even if it is somewhat more limited on the information
it provides.

> +++ b/qga/commands-win32.c
> @@ -1536,3 +1536,108 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
>          ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
>      }
>  }

> +static char *ga_get_win_ver(void)
> +{
> +    OSVERSIONINFOEXW os_version;
> +    ga_get_version(&os_version);
> +    char buf[64] = {};
> +    int major = (int)os_version.dwMajorVersion;
> +    int minor = (int)os_version.dwMinorVersion;
> +    sprintf(buf, "%d.%d", major, minor);
> +    return strdup(buf);

Instead of using strdup() (which uses malloc()), you should be using
g_strdup_printf() (which uses g_malloc().  In general, we prefer
g_malloc/g_free over raw malloc.  And once you use g_strdup_printf, you
no longer have to worry about whether buf is allocated large enough to
avoid overflow with sprintf.


> +++ 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.8

You've missed 2.8 by a long shot. In fact, you've missed hard freeze for
2.9, so this needs to be 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.

Rather than printing an empty string, would it be better to make the
fields optional, and omit them when there is nothing useful to supply?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Re: [Qemu-devel] [PATCH] qemu-ga: add guest-get-osinfo command
Posted by Vinzenz Feenstra 7 years ago
> On Mar 21, 2017, at 2:39 PM, Eric Blake <eblake@redhat.com> wrote:
> 
> On 03/16/2017 05:23 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>
>> ---
> 
> 
>> +static void ga_get_linux_distribution_info(GuestOSInfo *info)
>> +{
>> +    FILE *fp = NULL;
>> +    fp = fopen("/etc/os-release", "r");
> 
> Rather than read random files, I'm thinking that the uname() syscall is
> a better approach.  It is portable to more guests (since uname is a
> POSIX interface), even if it is somewhat more limited on the information
> it provides.

How about using it as a fallback option only but still use the information available
on the other places. I think it’s useful information what can be found there.

> 
>> +++ b/qga/commands-win32.c
>> @@ -1536,3 +1536,108 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
>>         ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
>>     }
>> }
> 
>> +static char *ga_get_win_ver(void)
>> +{
>> +    OSVERSIONINFOEXW os_version;
>> +    ga_get_version(&os_version);
>> +    char buf[64] = {};
>> +    int major = (int)os_version.dwMajorVersion;
>> +    int minor = (int)os_version.dwMinorVersion;
>> +    sprintf(buf, "%d.%d", major, minor);
>> +    return strdup(buf);
> 
> Instead of using strdup() (which uses malloc()), you should be using
> g_strdup_printf() (which uses g_malloc().  In general, we prefer
> g_malloc/g_free over raw malloc.  And once you use g_strdup_printf, you
> no longer have to worry about whether buf is allocated large enough to
> avoid overflow with sprintf.
Will do with the next version. Thanks for that hint with sprintf I didn’t know about that.

> 
> 
>> +++ 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.8
> 
> You've missed 2.8 by a long shot. In fact, you've missed hard freeze for
> 2.9, so this needs to be 2.10.

I don’t even know where I was looking for finding that number.

> 
>> +##
>> +{ '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.
> 
> Rather than printing an empty string, would it be better to make the
> fields optional, and omit them when there is nothing useful to supply?

I tried to keep it as stable as possible, but optional would work for me too, if it is preferred.

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org <http://libvirt.org/>
[Qemu-devel] (no subject)
Posted by Vinzenz 'evilissimo' Feenstra 7 years, 1 month ago
In this version:

- Changed the use of strdup to g_strdup and the use of sprintf with a local
  buffer to use g_strdup_printf instead.
- Made the majority of fields in the GuestOSInfo optional to allow 0 values
- Used the right target version in the schema (2.10 vs 2.8 before)
- Refactored the code for deciding which release/version file to use to use a
  configuration struct and a while loop to iterate over the options.

I was looking into the usage of uname, as suggested by eric, however after
looking into this I realized that there's no additional information to be
gained from this. Therefore I decided that this is still a feasible approach.
In most cases the code will break out of the loop after accessing the second
file. For older systems there are some supported fallbacks available, but
/etc/os-release and /usr/lib/os-release are already quite established.


Re: [Qemu-devel] (no subject)
Posted by Eric Blake 7 years ago
On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
> In this version:

When sending a v2, it's best to send it as a new top-level thread
instead of burying it in-reply-to an older thread. Also, don't forget
the subject line on the header message.

> 
> - Changed the use of strdup to g_strdup and the use of sprintf with a local
>   buffer to use g_strdup_printf instead.
> - Made the majority of fields in the GuestOSInfo optional to allow 0 values
> - Used the right target version in the schema (2.10 vs 2.8 before)
> - Refactored the code for deciding which release/version file to use to use a
>   configuration struct and a while loop to iterate over the options.
> 
> I was looking into the usage of uname, as suggested by eric, however after
> looking into this I realized that there's no additional information to be
> gained from this. Therefore I decided that this is still a feasible approach.
> In most cases the code will break out of the loop after accessing the second
> file. For older systems there are some supported fallbacks available, but
> /etc/os-release and /usr/lib/os-release are already quite established.
> 
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[Qemu-devel] [PATCH v2] qemu-ga: add guest-get-osinfo command
Posted by Vinzenz 'evilissimo' Feenstra 7 years, 1 month ago
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>
---
 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 <sys/ioctl.h>
+#include <sys/utsname.h>
 #include <sys/wait.h>
 #include <dirent.h>
 #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 && strcmp(line, "DISTRIB_CODENAME") == 0) {
+                info->codename = ga_stripped_strdup(value);
+            } else if (len == 10 && strcmp(line, "DISTRIB_ID") == 0) {
+                info->distribution = ga_stripped_strdup(value);
+            }
+        }
+    }
+}
+
+typedef struct _ga_distribution_info_t {
+    char const *file_path;
+    void(*implementation)(FILE *fp, GuestOSInfo *info);
+} ga_distribution_info_t;
+
+static ga_distribution_info_t const GA_DISTIBUTION_INFO[] = {
+    {"/etc/os-release", ga_parse_os_release},
+    {"/usr/lib/os-release", ga_parse_os_release},
+    {"/etc/lsb-release", ga_parse_lsb_release},
+    {"/etc/redhat-release", ga_parse_redhat_release},
+    {"/etc/gentoo-release", ga_parse_redhat_release},
+    {"/etc/debian_version", ga_parse_debian_version},
+    {0, 0}
+};
+
+static void ga_get_linux_distribution_info(GuestOSInfo *info)
+{
+    FILE *fp = NULL;
+    ga_distribution_info_t const *distrib = GA_DISTIBUTION_INFO;
+    while (distrib->file_path) {
+        fp = fopen(distrib->file_path, "r");
+        if (fp != NULL) {
+            distrib->implementation(fp, info);
+            fclose(fp);
+            break;
+        }
+        ++distrib;
+    }
+}
+
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+    GuestOSInfo *info = g_new0(GuestOSInfo, 1);
+    memset(info, 0, sizeof(GuestOSInfo));
+    struct utsname kinfo;
+
+    ga_get_linux_distribution_info(info);
+
+    if (!info->codename || !info->distribution || !info->version) {
+        qapi_free_GuestOSInfo(info);
+        return NULL;
+    }
+    info->type = GUESTOS_TYPE_LINUX;
+    uname(&kinfo);
+    info->kernel = g_strdup(kinfo.release);
+    info->arch = g_strdup(kinfo.machine);
+
+    return info;
+}
+
 #else /* defined(__linux__) */
 
 void qmp_guest_suspend_disk(Error **errp)
@@ -2418,6 +2601,12 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
     return NULL;
 }
 
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
 #endif
 
 #if !defined(CONFIG_FSFREEZE)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 19d72b2..97bba7c 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1536,3 +1536,107 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
         ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
     }
 }
+
+typedef struct _ga_matrix_lookup_t {
+    int major;
+    int minor;
+    char const *name;
+} ga_matrix_lookup_t;
+
+static ga_matrix_lookup_t const WIN_VERSION_MATRIX[2][8] = {
+    {
+        { 5, 0, "Win 2000"},
+        { 5, 1, "Win XP"},
+        { 6, 0, "Win Vista"},
+        { 6, 1, "Win 7"},
+
+        { 6, 2, "Win 8"},
+        { 6, 3, "Win 8.1"},
+        {10, 0, "Win 10"},
+        { 0, 0, 0}
+    },{
+        { 5, 2, "Win 2003"},
+        { 6, 0, "Win 2008"},
+        { 6, 1, "Win 2008 R2"},
+        { 6, 2, "Win 2012"},
+        { 6, 3, "Win 2012 R2"},
+        {10, 0, "Win 2016"},
+        { 0, 0, 0},
+        { 0, 0, 0}
+    }
+};
+
+static void ga_get_version(OSVERSIONINFOEXW *info)
+{
+    typedef NTSTATUS(WINAPI * rtl_get_version_t)(
+        OSVERSIONINFOEXW *os_version_info_ex);
+    HMODULE module = GetModuleHandle("ntdll");
+    PVOID fun = GetProcAddress(module, "RtlGetVersion");
+    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
+    rtl_get_version(info);
+}
+
+static char *ga_get_win_ver(void)
+{
+    OSVERSIONINFOEXW os_version;
+    ga_get_version(&os_version);
+    int major = (int)os_version.dwMajorVersion;
+    int minor = (int)os_version.dwMinorVersion;
+    return g_strdup_printf("%d.%d", major, minor);
+}
+
+static char *ga_get_win_name(void)
+{
+    OSVERSIONINFOEXW os_version;
+    ga_get_version(&os_version);
+    int major = (int)os_version.dwMajorVersion;
+    int minor = (int)os_version.dwMinorVersion;
+    int tbl_idx = (os_version.wProductType != VER_NT_WORKSTATION);
+    ga_matrix_lookup_t const *table = WIN_VERSION_MATRIX[tbl_idx];
+    while (table->name != NULL) {
+        if (major == table->major && minor == table->minor) {
+            return g_strdup(table->name);
+        }
+        ++table;
+    }
+    return NULL;
+}
+
+static char *ga_get_current_arch(void)
+{
+    SYSTEM_INFO info;
+    GetNativeSystemInfo(&info);
+    char *result = NULL;
+    switch (info.wProcessorArchitecture) {
+    case PROCESSOR_ARCHITECTURE_AMD64:
+        result = g_strdup("x86_64");
+        break;
+    case PROCESSOR_ARCHITECTURE_ARM:
+        result = g_strdup("arm");
+        break;
+    case PROCESSOR_ARCHITECTURE_IA64:
+        result = g_strdup("ia64");
+        break;
+    case PROCESSOR_ARCHITECTURE_INTEL:
+        result = g_strdup("x86");
+        break;
+    case PROCESSOR_ARCHITECTURE_UNKNOWN:
+    default:
+        /* Stays NULL in unknown cases */
+        break;
+    }
+    return result;
+}
+
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+    GuestOSInfo *info = g_new0(GuestOSInfo, 1);
+    info->type = GUESTOS_TYPE_WINDOWS;
+    info->version = ga_get_win_ver();
+    info->codename = ga_get_win_name();
+    info->arch = ga_get_current_arch();
+    /* Not available on Windows */
+    info->kernel = NULL;
+    info->distribution = NULL;
+    return info;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a02dbf2..c5019a4 100644
--- a/qga/qapi-schema.json
+++ 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.
+# Since: 2.10
+##
+{ 'struct': 'GuestOSInfo',
+  'data': { '*version': 'str', '*distribution': 'str', '*codename': 'str',
+            '*arch': 'str', 'type': 'GuestOSType', '*kernel': 'str'} }
+
+##
+# @guest-get-osinfo:
+#
+# Retrieve guest operating system information
+#
+# Returns: operating system information on success
+#
+# Since 2.10
+##
+{ 'command': 'guest-get-osinfo',
+  'returns': 'GuestOSInfo' }
-- 
2.9.3


Re: [Qemu-devel] [PATCH v2] qemu-ga: add guest-get-osinfo command
Posted by Eric Blake 7 years ago
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.

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

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

Re: [Qemu-devel] [PATCH v2] qemu-ga: add guest-get-osinfo command
Posted by Vinzenz Feenstra 7 years ago
> 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/>
Re: [Qemu-devel] [PATCH v2] qemu-ga: add guest-get-osinfo command
Posted by Vinzenz Feenstra 7 years ago
> On Mar 24, 2017, at 10:48 AM, Vinzenz Feenstra <vfeenstr@redhat.com> wrote:
> 
>> 
>> On Mar 23, 2017, at 3:58 PM, Eric Blake <eblake@redhat.com <mailto:eblake@redhat.com>> wrote:
>> 
>> On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
>>> From: Vinzenz Feenstra <vfeenstr@redhat.com <mailto: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:////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:////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 <mailto: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?

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’,
> } }
> 
>> 
>>> +
>>> +##
>>> +# @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/>
Re: [Qemu-devel] [PATCH v2] qemu-ga: add guest-get-osinfo command
Posted by Marc-André Lureau 7 years ago
Hi

On Tue, Mar 28, 2017 at 7:15 AM Vinzenz Feenstra <vfeenstr@redhat.com>
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