Message ID | 20170316102351.25056-1-vfeenstr@redhat.com |
---|---|
State | New |
Headers | show |
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.
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?
> 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/>
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. > >
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' }