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

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

[Vinzenz 'evilissimo' Feenstra]

From: Vinzenz Feenstra <vfeenstr@redhat.com>

This is a concept change over v2 of the guest-get-osinfo command.
The command now adds all information in the os-release format.

Vinzenz Feenstra (1):
  qemu-ga: add guest-get-osrelease command

 qga/commands-posix.c |  41 +++++++++++++++++
 qga/commands-win32.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/qapi-schema.json |  29 ++++++++++++
 3 files changed, 197 insertions(+)

-- 
2.9.3

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

[Vinzenz 'evilissimo' Feenstra]

From: Vinzenz Feenstra <vfeenstr@redhat.com>

Add a new 'guest-get-osrelease' command to report OS information in the
os-release format. As documented here:
https://www.freedesktop.org/software/systemd/man/os-release.html

The win32 implementation generates the information.
On POSIX systems the /etc/os-release or /usr/lib/os-release files
content is returned when available and gets extended with the fields:
- QGA_UNAME_RELEASE which is the content of `uname -r`
- QGA_UNAME_VERSION which is the content of `uname -v`
- QGA_UNAME_MACHINE which is the content of `uname -m`

Here an example for a Fedora 25 VM:

virsh # qemu-agent-command F25 '{ "execute": "guest-get-osrelease" }'
{"return":{"content":"NAME=Fedora\nVERSION=\"25 (Server Edition)\"\n
ID=fedora\nVERSION_ID=25\nPRETTY_NAME=\"Fedora 25 (Server Edition)\"\n
ANSI_COLOR=\"0;34\"\nCPE_NAME=\"cpe:/o:fedoraproject:fedora:25\"\n
HOME_URL=\"https://fedoraproject.org/\"\n
BUG_REPORT_URL=\"https://bugzilla.redhat.com/\"\n
REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n
REDHAT_BUGZILLA_PRODUCT_VERSION=25\nREDHAT_SUPPORT_PRODUCT=\"Fedora\"\n
REDHAT_SUPPORT_PRODUCT_VERSION=25\n
PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy\n
VARIANT=\"Server Edition\"\nVARIANT_ID=server\n\n
QGA_UNAME_RELEASE=\"4.8.6-300.fc25.x86_64\"\n
QGA_UNAME_VERSION=\"#1 SMP Tue Nov 1 12:36:38 UTC 2016\"\n
QGA_UNAME_MACHINE=\"x86_64\"\n"}}

And an example for a Windows 2012 R2 VM:

virsh # qemu-agent-command Win2k12r2 '{ "execute": "guest-get-osrelease" }'
{"return":{"content":"NAME=\"Microsoft Windows\"\nID=mswindows\n
VERSION=\"Microsoft Windows Server 2012 R2 (6.3)\"\nVERSION_ID=6.3\n
PRETTY_NAME=\"Windows Server 2012 R2 Datacenter\"\nVARIANT=\"server\"\n
VARIANT_ID=server\n"}}

Signed-off-by: Vinzenz Feenstra <vfeenstr@redhat.com>
---
 qga/commands-posix.c |  41 +++++++++++++++++
 qga/commands-win32.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/qapi-schema.json |  29 ++++++++++++
 3 files changed, 197 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 73d93eb..0a1fa28 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"
@@ -2418,6 +2419,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)
@@ -2515,3 +2522,37 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
     ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
 #endif
 }
+
+GuestOSRelease *qmp_guest_get_osrelease(Error **errp)
+{
+    GuestOSRelease *info = g_new0(GuestOSRelease, 1);
+    memset(info, 0, sizeof(GuestOSRelease));
+
+    struct utsname kinfo;
+    uname(&kinfo);
+
+    if (!g_file_get_contents("/etc/os-release", &info->content, NULL,
+                             NULL)){
+        g_file_get_contents("/usr/lib/os-release", &info->content,
+                            NULL, NULL);
+    }
+
+    char *extension = g_strdup_printf(
+        "\n"
+        "QGA_UNAME_RELEASE=\"%s\"\n"
+        "QGA_UNAME_VERSION=\"%s\"\n"
+        "QGA_UNAME_MACHINE=\"%s\"\n",
+        kinfo.release,
+        kinfo.version,
+        kinfo.machine
+    );
+    if (info->content != NULL) {
+        char *previous = info->content;
+        info->content = g_strconcat(previous, extension, NULL);
+        g_free(previous);
+    } else {
+        info->content = extension;
+    }
+    return info;
+}
+
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 19d72b2..a1c217e 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1536,3 +1536,130 @@ 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, "Microsoft Windows 2000"},
+        { 5, 1, "Microsoft Windows XP"},
+        { 6, 0, "Microsoft Windows Vista"},
+        { 6, 1, "Microsoft Windows 7"},
+
+        { 6, 2, "Microsoft Windows 8"},
+        { 6, 3, "Microsoft Windows 8.1"},
+        {10, 0, "Microsoft Windows 10"},
+        { 0, 0, 0}
+    },{
+        { 5, 2, "Microsoft Windows Server 2003"},
+        { 6, 0, "Microsoft Windows Server 2008"},
+        { 6, 1, "Microsoft Windows Server 2008 R2"},
+        { 6, 2, "Microsoft Windows Server 2012"},
+        { 6, 3, "Microsoft Windows Server 2012 R2"},
+        {10, 0, "Microsoft Windows Server 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_name(OSVERSIONINFOEXW const *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 g_strdup("N/A");
+}
+
+static char *ga_get_windows_product_name(void)
+{
+    HKEY key = NULL;
+    DWORD size = 128;
+    char *result = g_malloc0(size);
+    memset(result, 0, size);
+    LONG err = ERROR_SUCCESS;
+
+    err = RegOpenKeyA(HKEY_LOCAL_MACHINE,
+                      "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion",
+                      &key);
+    if (err == ERROR_SUCCESS) {
+        err = RegQueryValueExA(key, "ProductName", NULL, NULL,
+                               (LPBYTE)result, &size);
+        if (err == ERROR_MORE_DATA) {
+            g_free(result);
+            result = NULL;
+            if (size > 0) {
+                result = g_malloc0(size);
+                memset(result, 0, size);
+            }
+            if (result != NULL && size > 0) {
+                err = RegQueryValueExA(key, "ProductName", NULL, NULL,
+                                       (LPBYTE)result, &size);
+                if (err != ERROR_SUCCESS) {
+                    g_free(result);
+                    result = NULL;
+                }
+            }
+        }
+    }
+    return result;
+}
+
+GuestOSRelease *qmp_guest_get_osrelease(Error **errp)
+{
+    OSVERSIONINFOEXW os_version;
+    ga_get_version(&os_version);
+    int major = (int)os_version.dwMajorVersion;
+    int minor = (int)os_version.dwMinorVersion;
+    bool server = os_version.wProductType != VER_NT_WORKSTATION;
+    char *product_name = ga_get_windows_product_name();
+    if (product_name == NULL) {
+        return NULL;
+    }
+
+    GuestOSRelease *info = g_new0(GuestOSRelease, 1);
+    memset(info, 0, sizeof(GuestOSRelease));
+
+    info->content = g_strdup_printf(
+        "NAME=\"Microsoft Windows\"\n"
+        "ID=mswindows\n"
+        "VERSION=\"%s (%d.%d)\"\n"
+        "VERSION_ID=%d.%d\n"
+        "PRETTY_NAME=\"%s\"\n"
+        "VARIANT=\"%s\"\n"
+        "VARIANT_ID=%s\n",
+
+        /* VERSION */
+        ga_get_win_name(&os_version), major, minor,
+        /* VERSION_ID */
+        major, minor,
+        /* PRETTY_NAME */
+        product_name,
+        /* VARIANT */
+        server ? "server" : "client",
+        /* VARIANT_ID */
+        server ? "server" : "client"
+    );
+    g_free(product_name);
+    return info;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a02dbf2..268648c 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1042,3 +1042,32 @@
   'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
                '*input-data': 'str', '*capture-output': 'bool' },
   'returns': 'GuestExec' }
+
+##
+# @GuestOSRelease:
+#
+# @content: OS information in the os-release format as specified at:
+#           https://www.freedesktop.org/software/systemd/man/os-release.html
+#           On GNU/Linux this will be content of /etc/os-release or
+#           /usr/lib/os-release if either exists.
+#           On Windows, the data will be generated in the os-release format.
+#           Additionally on POSIX systems the os-release format will be
+#           extended with 3 fields QGA_UNAME_{VERSION,RELEASE,MACHINE} which
+#           are the retrieved through the uname syscall.
+#
+# Since: 2.10
+##
+{ 'struct': 'GuestOSRelease',
+  'data': { 'content': 'str' } }
+
+##
+# @guest-get-osrelease:
+#
+# Retrieve guest operating system information
+#
+# Returns: operating system information on success
+#
+# Since 2.10
+##
+{ 'command': 'guest-get-osrelease',
+  'returns': 'GuestOSRelease' }
-- 
2.9.3

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Eric Blake]

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

On 03/31/2017 05:19 AM, Vinzenz 'evilissimo' Feenstra wrote:
> From: Vinzenz Feenstra <vfeenstr@redhat.com>
> 
> Add a new 'guest-get-osrelease' command to report OS information in the
> os-release format. As documented here:
> https://www.freedesktop.org/software/systemd/man/os-release.html
> 
> The win32 implementation generates the information.
> On POSIX systems the /etc/os-release or /usr/lib/os-release files
> content is returned when available and gets extended with the fields:
> - QGA_UNAME_RELEASE which is the content of `uname -r`
> - QGA_UNAME_VERSION which is the content of `uname -v`
> - QGA_UNAME_MACHINE which is the content of `uname -m`
> 
> Here an example for a Fedora 25 VM:
> 
> virsh # qemu-agent-command F25 '{ "execute": "guest-get-osrelease" }'
> {"return":{"content":"NAME=Fedora\nVERSION=\"25 (Server Edition)\"\n
> ID=fedora\nVERSION_ID=25\nPRETTY_NAME=\"Fedora 25 (Server Edition)\"\n
> ANSI_COLOR=\"0;34\"\nCPE_NAME=\"cpe:/o:fedoraproject:fedora:25\"\n
> HOME_URL=\"https://fedoraproject.org/\"\n
> BUG_REPORT_URL=\"https://bugzilla.redhat.com/\"\n
> REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n
> REDHAT_BUGZILLA_PRODUCT_VERSION=25\nREDHAT_SUPPORT_PRODUCT=\"Fedora\"\n
> REDHAT_SUPPORT_PRODUCT_VERSION=25\n
> PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy\n
> VARIANT=\"Server Edition\"\nVARIANT_ID=server\n\n
> QGA_UNAME_RELEASE=\"4.8.6-300.fc25.x86_64\"\n
> QGA_UNAME_VERSION=\"#1 SMP Tue Nov 1 12:36:38 UTC 2016\"\n
> QGA_UNAME_MACHINE=\"x86_64\"\n"}}

Uggh. This is a step backwards.  Now you are requiring the end user to
parse a raw string, instead of giving them the information already
broken out as a JSON dictionary.

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

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Marc-André Lureau]

Hi

On Fri, Mar 31, 2017 at 3:41 PM Eric Blake <eblake@redhat.com> wrote:

> On 03/31/2017 05:19 AM, Vinzenz 'evilissimo' Feenstra wrote:
> > From: Vinzenz Feenstra <vfeenstr@redhat.com>
> >
> > Add a new 'guest-get-osrelease' command to report OS information in the
> > os-release format. As documented here:
> > https://www.freedesktop.org/software/systemd/man/os-release.html
> >
> > The win32 implementation generates the information.
> > On POSIX systems the /etc/os-release or /usr/lib/os-release files
> > content is returned when available and gets extended with the fields:
> > - QGA_UNAME_RELEASE which is the content of `uname -r`
> > - QGA_UNAME_VERSION which is the content of `uname -v`
> > - QGA_UNAME_MACHINE which is the content of `uname -m`
> >
> > Here an example for a Fedora 25 VM:
> >
> > virsh # qemu-agent-command F25 '{ "execute": "guest-get-osrelease" }'
> > {"return":{"content":"NAME=Fedora\nVERSION=\"25 (Server Edition)\"\n
> > ID=fedora\nVERSION_ID=25\nPRETTY_NAME=\"Fedora 25 (Server Edition)\"\n
> > ANSI_COLOR=\"0;34\"\nCPE_NAME=\"cpe:/o:fedoraproject:fedora:25\"\n
> > HOME_URL=\"https://fedoraproject.org/\"\n
> > BUG_REPORT_URL=\"https://bugzilla.redhat.com/\"\n
> > REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n
> > REDHAT_BUGZILLA_PRODUCT_VERSION=25\nREDHAT_SUPPORT_PRODUCT=\"Fedora\"\n
> > REDHAT_SUPPORT_PRODUCT_VERSION=25\n
> > PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy\n
> > VARIANT=\"Server Edition\"\nVARIANT_ID=server\n\n
> > QGA_UNAME_RELEASE=\"4.8.6-300.fc25.x86_64\"\n
> > QGA_UNAME_VERSION=\"#1 SMP Tue Nov 1 12:36:38 UTC 2016\"\n
> > QGA_UNAME_MACHINE=\"x86_64\"\n"}}
>
> Uggh. This is a step backwards.  Now you are requiring the end user to
> parse a raw string, instead of giving them the information already
> broken out as a JSON dictionary.
>

yes otoh, it uses an existing standard to retrieve various guest os release
informations, which existing tool may know how to handle.

(I feel partially guilty about it since I suggested it, mainly in a
discussion over irc and Vinzenz adopted it)

The format is fairly straightforward to parse, but perhaps it should be
sent as a JSON dict instead? However, that would mean that the list of keys
is limited by what QGA protocol defines, and the agent would have to parse
the file himself. And we would have to duplicate the documentation etc..

I would rely on the XDG format instead, given its simplicity, extensibility
and documentation that fits the job nicely imho.

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Vinzenz Feenstra]

Hi Michael,

It’d be really great to get your opinion on this matter. Thanks!

> On Apr 3, 2017, at 5:17 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> 
> Hi
> 
> On Fri, Mar 31, 2017 at 3:41 PM Eric Blake <eblake@redhat.com <mailto:eblake@redhat.com>> wrote:
> On 03/31/2017 05:19 AM, Vinzenz 'evilissimo' Feenstra wrote:
> > From: Vinzenz Feenstra <vfeenstr@redhat.com <mailto:vfeenstr@redhat.com>>
> >
> > Add a new 'guest-get-osrelease' command to report OS information in the
> > os-release format. As documented here:
> > https://www.freedesktop.org/software/systemd/man/os-release.html <https://www.freedesktop.org/software/systemd/man/os-release.html>
> >
> > The win32 implementation generates the information.
> > On POSIX systems the /etc/os-release or /usr/lib/os-release files
> > content is returned when available and gets extended with the fields:
> > - QGA_UNAME_RELEASE which is the content of `uname -r`
> > - QGA_UNAME_VERSION which is the content of `uname -v`
> > - QGA_UNAME_MACHINE which is the content of `uname -m`
> >
> > Here an example for a Fedora 25 VM:
> >
> > virsh # qemu-agent-command F25 '{ "execute": "guest-get-osrelease" }'
> > {"return":{"content":"NAME=Fedora\nVERSION=\"25 (Server Edition)\"\n
> > ID=fedora\nVERSION_ID=25\nPRETTY_NAME=\"Fedora 25 (Server Edition)\"\n
> > ANSI_COLOR=\"0;34\"\nCPE_NAME=\"cpe:/o:fedoraproject:fedora:25\"\n
> > HOME_URL=\"https://fedoraproject.org/\ <https://fedoraproject.org/%5C>"\n
> > BUG_REPORT_URL=\"https://bugzilla.redhat.com/\ <https://bugzilla.redhat.com/%5C>"\n
> > REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n
> > REDHAT_BUGZILLA_PRODUCT_VERSION=25\nREDHAT_SUPPORT_PRODUCT=\"Fedora\"\n
> > REDHAT_SUPPORT_PRODUCT_VERSION=25\n
> > PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy\n <https://fedoraproject.org/wiki/Legal:PrivacyPolicy%5Cn>
> > VARIANT=\"Server Edition\"\nVARIANT_ID=server\n\n
> > QGA_UNAME_RELEASE=\"4.8.6-300.fc25.x86_64\"\n
> > QGA_UNAME_VERSION=\"#1 SMP Tue Nov 1 12:36:38 UTC 2016\"\n
> > QGA_UNAME_MACHINE=\"x86_64\"\n"}}
> 
> Uggh. This is a step backwards.  Now you are requiring the end user to
> parse a raw string, instead of giving them the information already
> broken out as a JSON dictionary.
> 
> yes otoh, it uses an existing standard to retrieve various guest os release informations, which existing tool may know how to handle.
> 
> (I feel partially guilty about it since I suggested it, mainly in a discussion over irc and Vinzenz adopted it)
> 
> The format is fairly straightforward to parse, but perhaps it should be sent as a JSON dict instead? However, that would mean that the list of keys is limited by what QGA protocol defines, and the agent would have to parse the file himself. And we would have to duplicate the documentation etc..
> 
> I would rely on the XDG format instead, given its simplicity, extensibility and documentation that fits the job nicely imho.
>  
> -- 
> Marc-André Lureau

--
Vinzenz Feenstra
Senior Software Developer
Red Hat Czech

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Michael Roth]

On 04/03/2017 10:17 AM, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Mar 31, 2017 at 3:41 PM Eric Blake <eblake@redhat.com> wrote:
> 
>> On 03/31/2017 05:19 AM, Vinzenz 'evilissimo' Feenstra wrote:
>>> From: Vinzenz Feenstra <vfeenstr@redhat.com>
>>>
>>> Add a new 'guest-get-osrelease' command to report OS information in
>>> the
>>> os-release format. As documented here:
>>> https://www.freedesktop.org/software/systemd/man/os-release.html
>>>
>>> The win32 implementation generates the information.
>>> On POSIX systems the /etc/os-release or /usr/lib/os-release files
>>> content is returned when available and gets extended with the
>>> fields:
>>> - QGA_UNAME_RELEASE which is the content of `uname -r`
>>> - QGA_UNAME_VERSION which is the content of `uname -v`
>>> - QGA_UNAME_MACHINE which is the content of `uname -m`
>>>
>>> Here an example for a Fedora 25 VM:
>>>
>>> virsh # qemu-agent-command F25 '{ "execute": "guest-get-osrelease"
>>> }'
>>> {"return":{"content":"NAME=Fedora\nVERSION=\"25 (Server Edition)\"\n
>>> ID=fedora\nVERSION_ID=25\nPRETTY_NAME=\"Fedora 25 (Server
>>> Edition)\"\n
>>> ANSI_COLOR=\"0;34\"\nCPE_NAME=\"cpe:/o:fedoraproject:fedora:25\"\n
>>> HOME_URL=\"https://fedoraproject.org/\"\n
>>> BUG_REPORT_URL=\"https://bugzilla.redhat.com/\"\n
>>> REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n
>>> REDHAT_BUGZILLA_PRODUCT_VERSION=25\nREDHAT_SUPPORT_PRODUCT=\"Fedora\"\n
>>> REDHAT_SUPPORT_PRODUCT_VERSION=25\n
>>> PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy\n
>>> VARIANT=\"Server Edition\"\nVARIANT_ID=server\n\n
>>> QGA_UNAME_RELEASE=\"4.8.6-300.fc25.x86_64\"\n
>>> QGA_UNAME_VERSION=\"#1 SMP Tue Nov 1 12:36:38 UTC 2016\"\n
>>> QGA_UNAME_MACHINE=\"x86_64\"\n"}}
>>
>> Uggh. This is a step backwards.  Now you are requiring the end user
>> to
>> parse a raw string, instead of giving them the information already
>> broken out as a JSON dictionary.
>>
> 
> yes otoh, it uses an existing standard to retrieve various guest os
> release
> informations, which existing tool may know how to handle.
> 
> (I feel partially guilty about it since I suggested it, mainly in a
> discussion over irc and Vinzenz adopted it)
> 
> The format is fairly straightforward to parse, but perhaps it should
> be
> sent as a JSON dict instead? However, that would mean that the list of
> keys
> is limited by what QGA protocol defines, and the agent would have to
> parse
> the file himself. And we would have to duplicate the documentation
> etc..
> 
> I would rely on the XDG format instead, given its simplicity,
> extensibility
> and documentation that fits the job nicely imho.

I like the idea of using an existing standard, but if they really want
to get at a raw dump of /etc/os-release to use with existing tools then
I think guest-file-open/read are the more appropriate interfaces.

Knowing that they *can* get at information like that for a particular
guest, or do things like execute 'uname -m' via guest-exec, is where I
think an interface like this has it's place.

So I think a more curated/limited set of identifiers is sufficient, and
still flexible enough to enable to more os-specific use-cases.

But I also don't like the idea of re-defining what terms like
"version_id", "variant", "varient_id", etc mean, so I think it's still
a good idea to use the os-release-documented fields as the basis for
the fields we decide to return in our dictionary, and note that
explicitly in the schema documentation.

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Tomáš Golembiovský]

On Wed, 12 Apr 2017 15:05:02 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 04/03/2017 10:17 AM, Marc-André Lureau wrote:
> > Hi
> > 
> > On Fri, Mar 31, 2017 at 3:41 PM Eric Blake <eblake@redhat.com> wrote:
> >   
> >> On 03/31/2017 05:19 AM, Vinzenz 'evilissimo' Feenstra wrote:  
> >>> From: Vinzenz Feenstra <vfeenstr@redhat.com>
> >>>
> >>> Add a new 'guest-get-osrelease' command to report OS information in
> >>> the
> >>> os-release format. As documented here:
> >>> https://www.freedesktop.org/software/systemd/man/os-release.html
> >>>
> >>> The win32 implementation generates the information.
> >>> On POSIX systems the /etc/os-release or /usr/lib/os-release files
> >>> content is returned when available and gets extended with the
> >>> fields:
> >>> - QGA_UNAME_RELEASE which is the content of `uname -r`
> >>> - QGA_UNAME_VERSION which is the content of `uname -v`
> >>> - QGA_UNAME_MACHINE which is the content of `uname -m`
> >>>
> >>> Here an example for a Fedora 25 VM:
> >>>
> >>> virsh # qemu-agent-command F25 '{ "execute": "guest-get-osrelease"
> >>> }'
> >>> {"return":{"content":"NAME=Fedora\nVERSION=\"25 (Server Edition)\"\n
> >>> ID=fedora\nVERSION_ID=25\nPRETTY_NAME=\"Fedora 25 (Server
> >>> Edition)\"\n
> >>> ANSI_COLOR=\"0;34\"\nCPE_NAME=\"cpe:/o:fedoraproject:fedora:25\"\n
> >>> HOME_URL=\"https://fedoraproject.org/\"\n
> >>> BUG_REPORT_URL=\"https://bugzilla.redhat.com/\"\n
> >>> REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n
> >>> REDHAT_BUGZILLA_PRODUCT_VERSION=25\nREDHAT_SUPPORT_PRODUCT=\"Fedora\"\n
> >>> REDHAT_SUPPORT_PRODUCT_VERSION=25\n
> >>> PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy\n
> >>> VARIANT=\"Server Edition\"\nVARIANT_ID=server\n\n
> >>> QGA_UNAME_RELEASE=\"4.8.6-300.fc25.x86_64\"\n
> >>> QGA_UNAME_VERSION=\"#1 SMP Tue Nov 1 12:36:38 UTC 2016\"\n
> >>> QGA_UNAME_MACHINE=\"x86_64\"\n"}}  
> >>
> >> Uggh. This is a step backwards.  Now you are requiring the end user
> >> to
> >> parse a raw string, instead of giving them the information already
> >> broken out as a JSON dictionary.
> >>  
> > 
> > yes otoh, it uses an existing standard to retrieve various guest os
> > release
> > informations, which existing tool may know how to handle.
> > 
> > (I feel partially guilty about it since I suggested it, mainly in a
> > discussion over irc and Vinzenz adopted it)
> > 
> > The format is fairly straightforward to parse, but perhaps it should
> > be
> > sent as a JSON dict instead? However, that would mean that the list of
> > keys
> > is limited by what QGA protocol defines, and the agent would have to
> > parse
> > the file himself. And we would have to duplicate the documentation
> > etc..
> > 
> > I would rely on the XDG format instead, given its simplicity,
> > extensibility
> > and documentation that fits the job nicely imho.  
> 
> I like the idea of using an existing standard, but if they really want
> to get at a raw dump of /etc/os-release to use with existing tools then
> I think guest-file-open/read are the more appropriate interfaces.
> 
> Knowing that they *can* get at information like that for a particular
> guest, or do things like execute 'uname -m' via guest-exec, is where I
> think an interface like this has it's place.
> 
> So I think a more curated/limited set of identifiers is sufficient, and
> still flexible enough to enable to more os-specific use-cases.
> 
> But I also don't like the idea of re-defining what terms like
> "version_id", "variant", "varient_id", etc mean, so I think it's still
> a good idea to use the os-release-documented fields as the basis for
> the fields we decide to return in our dictionary, and note that
> explicitly in the schema documentation.
> 

So what about the following, would that be acceptable?


##
# @GuestOSRelease:
#
# @content:
#           POSIX systems the @kernel_version, @kernel_release and
#           @machine_hardware correspond to the values release, version and
#           machine returned by uname(2). On Windows, they correspond to the
#           version number, build number and architecture.
#
#           On Linux-based system where os-release info is available either
#           from /etc/os-release or from /usr/lib/os-release, the fields @id,
#           @name, @pretty_name, @version, @version_codename, @variant,
#           correspond to the fields of the same name defined in os-release(5).
#           On Windows, the data is generated based on the available
#           inforamtion.
#
# Since: 2.10
##
{ 'struct': 'GuestOSRelease',
  'data': {
      'kernel_release': 'str',
      'kernel_version': 'str',
      'machine_hardware': 'str'
      'id': '*str',
      'name': '*str',
      'pretty_name': '*str',
      'version': '*str',
      'version_codename': '*str',
      'variant': '*str',
  } }


    Tomas


-- 
Tomáš Golembiovský <tgolembi@redhat.com>

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Tomáš Golembiovský]

On Wed, 24 May 2017 23:51:55 +0200
Tomáš Golembiovský <tgolembi@redhat.com> wrote:

> On Wed, 12 Apr 2017 15:05:02 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 
> > On 04/03/2017 10:17 AM, Marc-André Lureau wrote:  
> > > Hi
> > > 
> > > On Fri, Mar 31, 2017 at 3:41 PM Eric Blake <eblake@redhat.com> wrote:
> > >     
> > >> On 03/31/2017 05:19 AM, Vinzenz 'evilissimo' Feenstra wrote:    
> > >>> From: Vinzenz Feenstra <vfeenstr@redhat.com>
> > >>>
> > >>> Add a new 'guest-get-osrelease' command to report OS information in
> > >>> the
> > >>> os-release format. As documented here:
> > >>> https://www.freedesktop.org/software/systemd/man/os-release.html
> > >>>
> > >>> The win32 implementation generates the information.
> > >>> On POSIX systems the /etc/os-release or /usr/lib/os-release files
> > >>> content is returned when available and gets extended with the
> > >>> fields:
> > >>> - QGA_UNAME_RELEASE which is the content of `uname -r`
> > >>> - QGA_UNAME_VERSION which is the content of `uname -v`
> > >>> - QGA_UNAME_MACHINE which is the content of `uname -m`
> > >>>
> > >>> Here an example for a Fedora 25 VM:
> > >>>
> > >>> virsh # qemu-agent-command F25 '{ "execute": "guest-get-osrelease"
> > >>> }'
> > >>> {"return":{"content":"NAME=Fedora\nVERSION=\"25 (Server Edition)\"\n
> > >>> ID=fedora\nVERSION_ID=25\nPRETTY_NAME=\"Fedora 25 (Server
> > >>> Edition)\"\n
> > >>> ANSI_COLOR=\"0;34\"\nCPE_NAME=\"cpe:/o:fedoraproject:fedora:25\"\n
> > >>> HOME_URL=\"https://fedoraproject.org/\"\n
> > >>> BUG_REPORT_URL=\"https://bugzilla.redhat.com/\"\n
> > >>> REDHAT_BUGZILLA_PRODUCT=\"Fedora\"\n
> > >>> REDHAT_BUGZILLA_PRODUCT_VERSION=25\nREDHAT_SUPPORT_PRODUCT=\"Fedora\"\n
> > >>> REDHAT_SUPPORT_PRODUCT_VERSION=25\n
> > >>> PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy\n
> > >>> VARIANT=\"Server Edition\"\nVARIANT_ID=server\n\n
> > >>> QGA_UNAME_RELEASE=\"4.8.6-300.fc25.x86_64\"\n
> > >>> QGA_UNAME_VERSION=\"#1 SMP Tue Nov 1 12:36:38 UTC 2016\"\n
> > >>> QGA_UNAME_MACHINE=\"x86_64\"\n"}}    
> > >>
> > >> Uggh. This is a step backwards.  Now you are requiring the end user
> > >> to
> > >> parse a raw string, instead of giving them the information already
> > >> broken out as a JSON dictionary.
> > >>    
> > > 
> > > yes otoh, it uses an existing standard to retrieve various guest os
> > > release
> > > informations, which existing tool may know how to handle.
> > > 
> > > (I feel partially guilty about it since I suggested it, mainly in a
> > > discussion over irc and Vinzenz adopted it)
> > > 
> > > The format is fairly straightforward to parse, but perhaps it should
> > > be
> > > sent as a JSON dict instead? However, that would mean that the list of
> > > keys
> > > is limited by what QGA protocol defines, and the agent would have to
> > > parse
> > > the file himself. And we would have to duplicate the documentation
> > > etc..
> > > 
> > > I would rely on the XDG format instead, given its simplicity,
> > > extensibility
> > > and documentation that fits the job nicely imho.    
> > 
> > I like the idea of using an existing standard, but if they really want
> > to get at a raw dump of /etc/os-release to use with existing tools then
> > I think guest-file-open/read are the more appropriate interfaces.
> > 
> > Knowing that they *can* get at information like that for a particular
> > guest, or do things like execute 'uname -m' via guest-exec, is where I
> > think an interface like this has it's place.
> > 
> > So I think a more curated/limited set of identifiers is sufficient, and
> > still flexible enough to enable to more os-specific use-cases.
> > 
> > But I also don't like the idea of re-defining what terms like
> > "version_id", "variant", "varient_id", etc mean, so I think it's still
> > a good idea to use the os-release-documented fields as the basis for
> > the fields we decide to return in our dictionary, and note that
> > explicitly in the schema documentation.
> >   
> 
> So what about the following, would that be acceptable?
> 
> 
> ##
> # @GuestOSRelease:
> #
> # @content:
> #           POSIX systems the @kernel_version, @kernel_release and
> #           @machine_hardware correspond to the values release, version and
> #           machine returned by uname(2). On Windows, they correspond to the
> #           version number, build number and architecture.
> #
> #           On Linux-based system where os-release info is available either
> #           from /etc/os-release or from /usr/lib/os-release, the fields @id,
> #           @name, @pretty_name, @version, @version_codename, @variant,
> #           correspond to the fields of the same name defined in os-release(5).
> #           On Windows, the data is generated based on the available
> #           inforamtion.
> #
> # Since: 2.10
> ##
> { 'struct': 'GuestOSRelease',
>   'data': {
>       'kernel_release': 'str',
>       'kernel_version': 'str',
>       'machine_hardware': 'str'
>       'id': '*str',
>       'name': '*str',
>       'pretty_name': '*str',
>       'version': '*str',
>       'version_codename': '*str',

I forgot version_id here, maybe it can go in place of version_codename... or do we want both?

>       'variant': '*str',

And here I forgot variant_id.

    Tomas

>   } }
> 
> 
>     Tomas
> 
> 
> -- 
> Tomáš Golembiovský <tgolembi@redhat.com>


-- 
Tomáš Golembiovský <tgolembi@redhat.com>

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Eric Blake]

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

On 05/24/2017 04:51 PM, Tomáš Golembiovský wrote:
> So what about the following, would that be acceptable?
> 
> 
> ##
> # @GuestOSRelease:
> #
> # @content:
> #           POSIX systems the @kernel_version, @kernel_release and
> #           @machine_hardware correspond to the values release, version and
> #           machine returned by uname(2). On Windows, they correspond to the
> #           version number, build number and architecture.

You'll have to actually document each field, not just a catch-all
@content.  You can list per-OS on which field is likely to be present or
absent.

> #
> #           On Linux-based system where os-release info is available either
> #           from /etc/os-release or from /usr/lib/os-release, the fields @id,
> #           @name, @pretty_name, @version, @version_codename, @variant,
> #           correspond to the fields of the same name defined in os-release(5).
> #           On Windows, the data is generated based on the available
> #           inforamtion.

s/inforamtion/information/

> #
> # Since: 2.10
> ##
> { 'struct': 'GuestOSRelease',
>   'data': {
>       'kernel_release': 'str',

Please name this 'kernel-release'; new interfaces should use '-' rather
than '_'.

>       'kernel_version': 'str',

Etc.

>       'machine_hardware': 'str'
>       'id': '*str',

If a field doesn't make sense for all guests, then it should be marked
optional (for example, a Linux guest that does not have
/etc/os-release).  Marking a field optional is done as '*id':'str' (you
got it backwards).


>       'name': '*str',
>       'pretty_name': '*str',
>       'version': '*str',
>       'version_codename': '*str',
>       'variant': '*str',
>   } }
> 
> 

It's probably a reasonable start at the interface, though.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v3] qemu-ga: add guest-get-osrelease command

[Eric Blake]

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

On 03/31/2017 05:19 AM, Vinzenz 'evilissimo' Feenstra wrote:
> From: Vinzenz Feenstra <vfeenstr@redhat.com>
> 
> This is a concept change over v2 of the guest-get-osinfo command.
> The command now adds all information in the os-release format.

Cover letters are not necessary (but don't hurt) for one-patch series -
but if you DO use a cover letter, please also turn on numbering, so that
the subject lines can be distinguished between the 0/1 cover letter and
1/1 patch.  'git config format.coverletter auto' will make the creation
of cover letters happen only when you send multi-patch series.

> 
> Vinzenz Feenstra (1):
>   qemu-ga: add guest-get-osrelease command
> 
>  qga/commands-posix.c |  41 +++++++++++++++++
>  qga/commands-win32.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qga/qapi-schema.json |  29 ++++++++++++
>  3 files changed, 197 insertions(+)
> 

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