* [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
@ 2019-09-25 21:00 Tomáš Golembiovský
2019-09-26 7:13 ` Marc-André Lureau
2019-09-27 1:12 ` no-reply
0 siblings, 2 replies; 6+ messages in thread
From: Tomáš Golembiovský @ 2019-09-25 21:00 UTC (permalink / raw)
To: qemu-devel
Cc: Marc-André Lureau, Tomáš Golembiovský,
Michael Roth
Add command for reporting devices on Windows guest. The intent is not so
much to report the devices but more importantly the driver (and its
version) that is assigned to the device. This gives caller the
information whether VirtIO drivers are installed and/or whether
inadequate driver is used on a device (e.g. QXL device with base VGA
driver).
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
qga/commands-posix.c | 9 ++
qga/commands-win32.c | 204 ++++++++++++++++++++++++++++++++++++++++++-
qga/qapi-schema.json | 32 +++++++
3 files changed, 244 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index dfc05f5b8a..58e93feef9 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2757,6 +2757,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
#endif
+ blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
+
return blacklist;
}
@@ -2977,3 +2979,10 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
return info;
}
+
+GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
+{
+ error_setg(errp, QERR_UNSUPPORTED);
+
+ return NULL;
+}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 6b67f16faf..139dbd7c9a 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -21,10 +21,11 @@
#ifdef CONFIG_QGA_NTDDSCSI
#include <winioctl.h>
#include <ntddscsi.h>
+#endif
#include <setupapi.h>
#include <cfgmgr32.h>
#include <initguid.h>
-#endif
+#include <devpropdef.h>
#include <lm.h>
#include <wtsapi32.h>
#include <wininet.h>
@@ -38,6 +39,36 @@
#include "qemu/host-utils.h"
#include "qemu/base64.h"
+/*
+ * The following should be in devpkey.h, but it isn't. The key names were
+ * prefixed to avoid (future) name clashes. Once the definitions get into
+ * mingw the following lines can be removed.
+ */
+DEFINE_DEVPROPKEY(qga_DEVPKEY_NAME, 0xb725f130, 0x47ef, 0x101a, 0xa5,
+ 0xf1, 0x02, 0x60, 0x8c, 0x9e, 0xeb, 0xac, 10);
+ /* DEVPROP_TYPE_STRING */
+DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_HardwareIds, 0xa45c254e, 0xdf1c,
+ 0x4efd, 0x80, 0x20, 0x67, 0xd1, 0x46, 0xa8, 0x50, 0xe0, 3);
+ /* DEVPROP_TYPE_STRING_LIST */
+DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverDate, 0xa8b865dd, 0x2e3d,
+ 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 2);
+ /* DEVPROP_TYPE_FILETIME */
+DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverVersion, 0xa8b865dd, 0x2e3d,
+ 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 3);
+ /* DEVPROP_TYPE_STRING */
+/* The following shoud be in cfgmgr32.h, but it isn't */
+#ifndef CM_Get_DevNode_Property
+CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
+ DEVINST dnDevInst,
+ CONST DEVPROPKEY * PropertyKey,
+ DEVPROPTYPE * PropertyType,
+ PBYTE PropertyBuffer,
+ PULONG PropertyBufferSize,
+ ULONG ulFlags
+);
+#define CM_Get_DevNode_Property CM_Get_DevNode_PropertyW
+#endif
+
#ifndef SHTDN_REASON_FLAG_PLANNED
#define SHTDN_REASON_FLAG_PLANNED 0x80000000
#endif
@@ -92,6 +123,8 @@ static OpenFlags guest_file_open_modes[] = {
g_free(suffix); \
} while (0)
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(GuestDeviceInfo, qapi_free_GuestDeviceInfo)
+
static OpenFlags *find_open_flag(const char *mode_str)
{
int mode;
@@ -2234,3 +2267,172 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
return info;
}
+
+/*
+ * Safely get device property. Returned strings are using wide characters.
+ * Caller is responsible for freeing the buffer.
+ */
+static LPBYTE cm_get_property(DEVINST devInst, const DEVPROPKEY *propName,
+ PDEVPROPTYPE propType)
+{
+ CONFIGRET cr;
+ g_autofree LPBYTE buffer = NULL;
+ ULONG buffer_len = 0;
+
+ /* First query for needed space */
+ cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
+ buffer, &buffer_len, 0);
+ if (cr != CR_SUCCESS && cr != CR_BUFFER_SMALL) {
+
+ slog("failed to get property size, error=0x%lx", cr);
+ return NULL;
+ }
+ buffer = g_new0(BYTE, buffer_len + 1);
+ cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
+ buffer, &buffer_len, 0);
+ if (cr != CR_SUCCESS) {
+ slog("failed to get device property, error=0x%lx", cr);
+ return NULL;
+ }
+ return g_steal_pointer(&buffer);
+}
+
+static GStrv ga_get_hardware_ids(DEVINST devInstance)
+{
+ GStrv hw_ids = NULL;
+ GArray *values = NULL;
+ DEVPROPTYPE cm_type;
+ LPWSTR id;
+ g_autofree LPWSTR property = (LPWSTR)cm_get_property(devInstance,
+ &qga_DEVPKEY_Device_HardwareIds, &cm_type);
+ if (property == NULL) {
+ slog("failed to get hardware IDs");
+ return NULL;
+ }
+ if (*property == '\0') {
+ /* empty list */
+ return NULL;
+ }
+ values = g_array_new(TRUE, TRUE, sizeof(gchar*));
+ for (id = property; '\0' != *id; id += lstrlenW(id) + 1) {
+ gchar* id8 = g_utf16_to_utf8(id, -1, NULL, NULL, NULL);
+ g_array_append_val(values, id8);
+ }
+ hw_ids = (GStrv)g_array_free(values, FALSE);
+ values = NULL;
+ return hw_ids;
+}
+
+/*
+ * https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices
+ */
+#define DEVICE_PCI_RE "PCI\\\\VEN_(1AF4|1B36)&DEV_([0-9A-B]{4})(&|$)"
+
+GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
+{
+ GuestDeviceInfoList *head = NULL, *cur_item = NULL, *item = NULL;
+ HDEVINFO dev_info = INVALID_HANDLE_VALUE;
+ SP_DEVINFO_DATA dev_info_data;
+ int i, j;
+ GError *gerr = NULL;
+ g_autoptr(GRegex) device_pci_re = NULL;
+ DEVPROPTYPE cm_type;
+
+ device_pci_re = g_regex_new(DEVICE_PCI_RE,
+ G_REGEX_ANCHORED | G_REGEX_OPTIMIZE, 0,
+ &gerr);
+ g_assert(device_pci_re != NULL);
+
+ dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
+ dev_info = SetupDiGetClassDevs(0, 0, 0, DIGCF_PRESENT | DIGCF_ALLCLASSES);
+ if (dev_info == INVALID_HANDLE_VALUE) {
+ error_setg(errp, "failed to get device tree");
+ return NULL;
+ }
+
+ slog("enumerating devices");
+ for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
+ bool skip = true;
+ SYSTEMTIME utc_date;
+ g_autofree LPWSTR name = NULL;
+ g_autofree LPFILETIME date = NULL;
+ g_autofree LPWSTR version = NULL;
+ g_auto(GStrv) hw_ids = NULL;
+ g_autoptr(GuestDeviceInfo) device = g_new0(GuestDeviceInfo, 1);
+ g_autofree char *vendor_id = NULL;
+ g_autofree char *device_id = NULL;
+
+ name = (LPWSTR)cm_get_property(dev_info_data.DevInst,
+ &qga_DEVPKEY_NAME, &cm_type);
+ if (name == NULL) {
+ slog("failed to get device description");
+ continue;
+ }
+ device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
+ if (device->driver_name == NULL) {
+ error_setg(errp, "conversion to utf8 failed (driver name)");
+ goto out;
+ }
+ slog("querying device: %s", device->driver_name);
+ hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
+ if (hw_ids == NULL) {
+ continue;
+ }
+ for (j = 0; hw_ids[j] != NULL; j++) {
+ GMatchInfo *match_info;
+ if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
+ continue;
+ }
+ skip = false;
+ vendor_id = g_match_info_fetch(match_info, 1);
+ device_id = g_match_info_fetch(match_info, 2);
+ device->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+ device->device_id = g_ascii_strtoull(device_id, NULL, 16);
+ g_match_info_free(match_info);
+ }
+ if (skip) {
+ continue;
+ }
+
+ version = (LPWSTR)cm_get_property(dev_info_data.DevInst,
+ &qga_DEVPKEY_Device_DriverVersion, &cm_type);
+ if (version == NULL) {
+ slog("failed to get driver version");
+ continue;
+ }
+ device->driver_version = g_utf16_to_utf8(version, -1, NULL,
+ NULL, NULL);
+ if (device->driver_version == NULL) {
+ error_setg(errp, "conversion to utf8 failed (driver version)");
+ goto out;
+ }
+
+ date = (LPFILETIME)cm_get_property(dev_info_data.DevInst,
+ &qga_DEVPKEY_Device_DriverDate, &cm_type);
+ if (date == NULL) {
+ slog("failed to get driver date");
+ continue;
+ }
+ FileTimeToSystemTime(date, &utc_date);
+ device->driver_date = g_strdup_printf("%04d-%02d-%02d",
+ utc_date.wYear, utc_date.wMonth, utc_date.wDay);
+
+ slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,
+ device->driver_date, device->driver_version);
+ item = g_new0(GuestDeviceInfoList, 1);
+ item->value = g_steal_pointer(&device);
+ if (!cur_item) {
+ head = cur_item = item;
+ } else {
+ cur_item->next = item;
+ cur_item = item;
+ }
+ continue;
+ }
+
+out:
+ if (dev_info != INVALID_HANDLE_VALUE) {
+ SetupDiDestroyDeviceInfoList(dev_info);
+ }
+ return head;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index fb4605cc19..fe04ff2f39 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1242,3 +1242,35 @@
##
{ 'command': 'guest-get-osinfo',
'returns': 'GuestOSInfo' }
+
+##
+# @GuestDeviceInfo:
+#
+# @vendor-id: vendor ID
+# @device-id: device ID
+# @driver-name: name of the associated driver
+# @driver-date: driver release date in format YYYY-MM-DD
+# @driver-version: driver version
+#
+# Since: 4.2
+##
+{ 'struct': 'GuestDeviceInfo',
+ 'data': {
+ 'vendor-id': 'uint16',
+ 'device-id': 'uint16',
+ 'driver-name': 'str',
+ 'driver-date': 'str',
+ 'driver-version': 'str'
+ } }
+
+##
+# @guest-get-devices:
+#
+# Retrieve information about device drivers in Windows guest
+#
+# Returns: @GuestDeviceInfo
+#
+# Since: 4.2
+##
+{ 'command': 'guest-get-devices',
+ 'returns': ['GuestDeviceInfo'] }
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
2019-09-25 21:00 [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices Tomáš Golembiovský
@ 2019-09-26 7:13 ` Marc-André Lureau
2019-09-26 10:16 ` Tomáš Golembiovský
2019-09-27 1:12 ` no-reply
1 sibling, 1 reply; 6+ messages in thread
From: Marc-André Lureau @ 2019-09-26 7:13 UTC (permalink / raw)
To: Tomáš Golembiovský; +Cc: QEMU, Michael Roth
Hi
On Thu, Sep 26, 2019 at 1:01 AM Tomáš Golembiovský <tgolembi@redhat.com> wrote:
>
> Add command for reporting devices on Windows guest. The intent is not so
> much to report the devices but more importantly the driver (and its
> version) that is assigned to the device. This gives caller the
> information whether VirtIO drivers are installed and/or whether
> inadequate driver is used on a device (e.g. QXL device with base VGA
> driver).
>
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
> qga/commands-posix.c | 9 ++
> qga/commands-win32.c | 204 ++++++++++++++++++++++++++++++++++++++++++-
> qga/qapi-schema.json | 32 +++++++
> 3 files changed, 244 insertions(+), 1 deletion(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index dfc05f5b8a..58e93feef9 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2757,6 +2757,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
> blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
> #endif
>
> + blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
> +
> return blacklist;
> }
>
> @@ -2977,3 +2979,10 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
>
> return info;
> }
> +
> +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> +{
> + error_setg(errp, QERR_UNSUPPORTED);
> +
> + return NULL;
> +}
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 6b67f16faf..139dbd7c9a 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -21,10 +21,11 @@
> #ifdef CONFIG_QGA_NTDDSCSI
> #include <winioctl.h>
> #include <ntddscsi.h>
> +#endif
> #include <setupapi.h>
> #include <cfgmgr32.h>
> #include <initguid.h>
> -#endif
> +#include <devpropdef.h>
> #include <lm.h>
> #include <wtsapi32.h>
> #include <wininet.h>
> @@ -38,6 +39,36 @@
> #include "qemu/host-utils.h"
> #include "qemu/base64.h"
>
> +/*
> + * The following should be in devpkey.h, but it isn't. The key names were
> + * prefixed to avoid (future) name clashes. Once the definitions get into
> + * mingw the following lines can be removed.
> + */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_NAME, 0xb725f130, 0x47ef, 0x101a, 0xa5,
> + 0xf1, 0x02, 0x60, 0x8c, 0x9e, 0xeb, 0xac, 10);
> + /* DEVPROP_TYPE_STRING */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_HardwareIds, 0xa45c254e, 0xdf1c,
> + 0x4efd, 0x80, 0x20, 0x67, 0xd1, 0x46, 0xa8, 0x50, 0xe0, 3);
> + /* DEVPROP_TYPE_STRING_LIST */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverDate, 0xa8b865dd, 0x2e3d,
> + 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 2);
> + /* DEVPROP_TYPE_FILETIME */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverVersion, 0xa8b865dd, 0x2e3d,
> + 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 3);
> + /* DEVPROP_TYPE_STRING */
> +/* The following shoud be in cfgmgr32.h, but it isn't */
> +#ifndef CM_Get_DevNode_Property
> +CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
> + DEVINST dnDevInst,
> + CONST DEVPROPKEY * PropertyKey,
> + DEVPROPTYPE * PropertyType,
> + PBYTE PropertyBuffer,
> + PULONG PropertyBufferSize,
> + ULONG ulFlags
> +);
> +#define CM_Get_DevNode_Property CM_Get_DevNode_PropertyW
> +#endif
> +
> #ifndef SHTDN_REASON_FLAG_PLANNED
> #define SHTDN_REASON_FLAG_PLANNED 0x80000000
> #endif
> @@ -92,6 +123,8 @@ static OpenFlags guest_file_open_modes[] = {
> g_free(suffix); \
> } while (0)
>
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(GuestDeviceInfo, qapi_free_GuestDeviceInfo)
> +
> static OpenFlags *find_open_flag(const char *mode_str)
> {
> int mode;
> @@ -2234,3 +2267,172 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
>
> return info;
> }
> +
> +/*
> + * Safely get device property. Returned strings are using wide characters.
> + * Caller is responsible for freeing the buffer.
> + */
> +static LPBYTE cm_get_property(DEVINST devInst, const DEVPROPKEY *propName,
> + PDEVPROPTYPE propType)
> +{
> + CONFIGRET cr;
> + g_autofree LPBYTE buffer = NULL;
> + ULONG buffer_len = 0;
> +
> + /* First query for needed space */
> + cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> + buffer, &buffer_len, 0);
> + if (cr != CR_SUCCESS && cr != CR_BUFFER_SMALL) {
> +
> + slog("failed to get property size, error=0x%lx", cr);
> + return NULL;
> + }
> + buffer = g_new0(BYTE, buffer_len + 1);
> + cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> + buffer, &buffer_len, 0);
> + if (cr != CR_SUCCESS) {
> + slog("failed to get device property, error=0x%lx", cr);
> + return NULL;
> + }
> + return g_steal_pointer(&buffer);
> +}
> +
> +static GStrv ga_get_hardware_ids(DEVINST devInstance)
> +{
> + GStrv hw_ids = NULL;
> + GArray *values = NULL;
> + DEVPROPTYPE cm_type;
> + LPWSTR id;
> + g_autofree LPWSTR property = (LPWSTR)cm_get_property(devInstance,
> + &qga_DEVPKEY_Device_HardwareIds, &cm_type);
> + if (property == NULL) {
> + slog("failed to get hardware IDs");
> + return NULL;
> + }
> + if (*property == '\0') {
> + /* empty list */
> + return NULL;
> + }
> + values = g_array_new(TRUE, TRUE, sizeof(gchar*));
> + for (id = property; '\0' != *id; id += lstrlenW(id) + 1) {
> + gchar* id8 = g_utf16_to_utf8(id, -1, NULL, NULL, NULL);
> + g_array_append_val(values, id8);
> + }
> + hw_ids = (GStrv)g_array_free(values, FALSE);
> + values = NULL;
> + return hw_ids;
Why not return g_array_free(values, FALSE) directly?
> +}
> +
> +/*
> + * https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices
> + */
> +#define DEVICE_PCI_RE "PCI\\\\VEN_(1AF4|1B36)&DEV_([0-9A-B]{4})(&|$)"
> +
> +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> +{
> + GuestDeviceInfoList *head = NULL, *cur_item = NULL, *item = NULL;
> + HDEVINFO dev_info = INVALID_HANDLE_VALUE;
> + SP_DEVINFO_DATA dev_info_data;
> + int i, j;
> + GError *gerr = NULL;
> + g_autoptr(GRegex) device_pci_re = NULL;
> + DEVPROPTYPE cm_type;
> +
> + device_pci_re = g_regex_new(DEVICE_PCI_RE,
> + G_REGEX_ANCHORED | G_REGEX_OPTIMIZE, 0,
> + &gerr);
> + g_assert(device_pci_re != NULL);
> +
> + dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
> + dev_info = SetupDiGetClassDevs(0, 0, 0, DIGCF_PRESENT | DIGCF_ALLCLASSES);
> + if (dev_info == INVALID_HANDLE_VALUE) {
> + error_setg(errp, "failed to get device tree");
> + return NULL;
> + }
> +
> + slog("enumerating devices");
> + for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
> + bool skip = true;
> + SYSTEMTIME utc_date;
> + g_autofree LPWSTR name = NULL;
> + g_autofree LPFILETIME date = NULL;
> + g_autofree LPWSTR version = NULL;
> + g_auto(GStrv) hw_ids = NULL;
> + g_autoptr(GuestDeviceInfo) device = g_new0(GuestDeviceInfo, 1);
> + g_autofree char *vendor_id = NULL;
> + g_autofree char *device_id = NULL;
> +
> + name = (LPWSTR)cm_get_property(dev_info_data.DevInst,
> + &qga_DEVPKEY_NAME, &cm_type);
> + if (name == NULL) {
> + slog("failed to get device description");
> + continue;
> + }
> + device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
> + if (device->driver_name == NULL) {
> + error_setg(errp, "conversion to utf8 failed (driver name)");
> + goto out;
> + }
> + slog("querying device: %s", device->driver_name);
> + hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
> + if (hw_ids == NULL) {
> + continue;
> + }
> + for (j = 0; hw_ids[j] != NULL; j++) {
> + GMatchInfo *match_info;
> + if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
> + continue;
> + }
> + skip = false;
> + vendor_id = g_match_info_fetch(match_info, 1);
> + device_id = g_match_info_fetch(match_info, 2);
> + device->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> + device->device_id = g_ascii_strtoull(device_id, NULL, 16);
> + g_match_info_free(match_info);
> + }
> + if (skip) {
> + continue;
> + }
> +
> + version = (LPWSTR)cm_get_property(dev_info_data.DevInst,
> + &qga_DEVPKEY_Device_DriverVersion, &cm_type);
> + if (version == NULL) {
> + slog("failed to get driver version");
> + continue;
> + }
> + device->driver_version = g_utf16_to_utf8(version, -1, NULL,
> + NULL, NULL);
> + if (device->driver_version == NULL) {
> + error_setg(errp, "conversion to utf8 failed (driver version)");
> + goto out;
> + }
> +
> + date = (LPFILETIME)cm_get_property(dev_info_data.DevInst,
> + &qga_DEVPKEY_Device_DriverDate, &cm_type);
> + if (date == NULL) {
> + slog("failed to get driver date");
> + continue;
> + }
> + FileTimeToSystemTime(date, &utc_date);
> + device->driver_date = g_strdup_printf("%04d-%02d-%02d",
> + utc_date.wYear, utc_date.wMonth, utc_date.wDay);
> +
> + slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,
> + device->driver_date, device->driver_version);
> + item = g_new0(GuestDeviceInfoList, 1);
> + item->value = g_steal_pointer(&device);
> + if (!cur_item) {
> + head = cur_item = item;
> + } else {
> + cur_item->next = item;
> + cur_item = item;
> + }
> + continue;
> + }
> +
> +out:
> + if (dev_info != INVALID_HANDLE_VALUE) {
> + SetupDiDestroyDeviceInfoList(dev_info);
> + }
> + return head;
> +}
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index fb4605cc19..fe04ff2f39 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1242,3 +1242,35 @@
> ##
> { 'command': 'guest-get-osinfo',
> 'returns': 'GuestOSInfo' }
> +
> +##
> +# @GuestDeviceInfo:
> +#
> +# @vendor-id: vendor ID
> +# @device-id: device ID
> +# @driver-name: name of the associated driver
> +# @driver-date: driver release date in format YYYY-MM-DD
> +# @driver-version: driver version
> +#
> +# Since: 4.2
> +##
> +{ 'struct': 'GuestDeviceInfo',
> + 'data': {
> + 'vendor-id': 'uint16',
> + 'device-id': 'uint16',
> + 'driver-name': 'str',
> + 'driver-date': 'str',
> + 'driver-version': 'str'
> + } }
> +
> +##
> +# @guest-get-devices:
> +#
> +# Retrieve information about device drivers in Windows guest
> +#
> +# Returns: @GuestDeviceInfo
> +#
> +# Since: 4.2
> +##
> +{ 'command': 'guest-get-devices',
> + 'returns': ['GuestDeviceInfo'] }
> --
> 2.23.0
>
>
other than that,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
2019-09-26 7:13 ` Marc-André Lureau
@ 2019-09-26 10:16 ` Tomáš Golembiovský
2019-09-26 10:22 ` Tomáš Golembiovský
0 siblings, 1 reply; 6+ messages in thread
From: Tomáš Golembiovský @ 2019-09-26 10:16 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: QEMU, Michael Roth
On Thu, Sep 26, 2019 at 11:13:32AM +0400, Marc-André Lureau wrote:
> Hi
>
> On Thu, Sep 26, 2019 at 1:01 AM Tomáš Golembiovský <tgolembi@redhat.com> wrote:
> >
> > Add command for reporting devices on Windows guest. The intent is not so
> > much to report the devices but more importantly the driver (and its
> > version) that is assigned to the device. This gives caller the
> > information whether VirtIO drivers are installed and/or whether
> > inadequate driver is used on a device (e.g. QXL device with base VGA
> > driver).
> >
> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > ---
> > qga/commands-posix.c | 9 ++
> > qga/commands-win32.c | 204 ++++++++++++++++++++++++++++++++++++++++++-
> > qga/qapi-schema.json | 32 +++++++
> > 3 files changed, 244 insertions(+), 1 deletion(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index dfc05f5b8a..58e93feef9 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -2757,6 +2757,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
> > blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
> > #endif
> >
> > + blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
> > +
> > return blacklist;
> > }
> >
> > @@ -2977,3 +2979,10 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> >
> > return info;
> > }
> > +
> > +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> > +{
> > + error_setg(errp, QERR_UNSUPPORTED);
> > +
> > + return NULL;
> > +}
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 6b67f16faf..139dbd7c9a 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -21,10 +21,11 @@
> > #ifdef CONFIG_QGA_NTDDSCSI
> > #include <winioctl.h>
> > #include <ntddscsi.h>
> > +#endif
> > #include <setupapi.h>
> > #include <cfgmgr32.h>
> > #include <initguid.h>
> > -#endif
> > +#include <devpropdef.h>
> > #include <lm.h>
> > #include <wtsapi32.h>
> > #include <wininet.h>
> > @@ -38,6 +39,36 @@
> > #include "qemu/host-utils.h"
> > #include "qemu/base64.h"
> >
> > +/*
> > + * The following should be in devpkey.h, but it isn't. The key names were
> > + * prefixed to avoid (future) name clashes. Once the definitions get into
> > + * mingw the following lines can be removed.
> > + */
> > +DEFINE_DEVPROPKEY(qga_DEVPKEY_NAME, 0xb725f130, 0x47ef, 0x101a, 0xa5,
> > + 0xf1, 0x02, 0x60, 0x8c, 0x9e, 0xeb, 0xac, 10);
> > + /* DEVPROP_TYPE_STRING */
> > +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_HardwareIds, 0xa45c254e, 0xdf1c,
> > + 0x4efd, 0x80, 0x20, 0x67, 0xd1, 0x46, 0xa8, 0x50, 0xe0, 3);
> > + /* DEVPROP_TYPE_STRING_LIST */
> > +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverDate, 0xa8b865dd, 0x2e3d,
> > + 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 2);
> > + /* DEVPROP_TYPE_FILETIME */
> > +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverVersion, 0xa8b865dd, 0x2e3d,
> > + 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 3);
> > + /* DEVPROP_TYPE_STRING */
> > +/* The following shoud be in cfgmgr32.h, but it isn't */
> > +#ifndef CM_Get_DevNode_Property
> > +CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
> > + DEVINST dnDevInst,
> > + CONST DEVPROPKEY * PropertyKey,
> > + DEVPROPTYPE * PropertyType,
> > + PBYTE PropertyBuffer,
> > + PULONG PropertyBufferSize,
> > + ULONG ulFlags
> > +);
> > +#define CM_Get_DevNode_Property CM_Get_DevNode_PropertyW
> > +#endif
> > +
> > #ifndef SHTDN_REASON_FLAG_PLANNED
> > #define SHTDN_REASON_FLAG_PLANNED 0x80000000
> > #endif
> > @@ -92,6 +123,8 @@ static OpenFlags guest_file_open_modes[] = {
> > g_free(suffix); \
> > } while (0)
> >
> > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(GuestDeviceInfo, qapi_free_GuestDeviceInfo)
> > +
> > static OpenFlags *find_open_flag(const char *mode_str)
> > {
> > int mode;
> > @@ -2234,3 +2267,172 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> >
> > return info;
> > }
> > +
> > +/*
> > + * Safely get device property. Returned strings are using wide characters.
> > + * Caller is responsible for freeing the buffer.
> > + */
> > +static LPBYTE cm_get_property(DEVINST devInst, const DEVPROPKEY *propName,
> > + PDEVPROPTYPE propType)
> > +{
> > + CONFIGRET cr;
> > + g_autofree LPBYTE buffer = NULL;
> > + ULONG buffer_len = 0;
> > +
> > + /* First query for needed space */
> > + cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> > + buffer, &buffer_len, 0);
> > + if (cr != CR_SUCCESS && cr != CR_BUFFER_SMALL) {
> > +
> > + slog("failed to get property size, error=0x%lx", cr);
> > + return NULL;
> > + }
> > + buffer = g_new0(BYTE, buffer_len + 1);
> > + cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> > + buffer, &buffer_len, 0);
> > + if (cr != CR_SUCCESS) {
> > + slog("failed to get device property, error=0x%lx", cr);
> > + return NULL;
> > + }
> > + return g_steal_pointer(&buffer);
> > +}
> > +
> > +static GStrv ga_get_hardware_ids(DEVINST devInstance)
> > +{
> > + GStrv hw_ids = NULL;
> > + GArray *values = NULL;
> > + DEVPROPTYPE cm_type;
> > + LPWSTR id;
> > + g_autofree LPWSTR property = (LPWSTR)cm_get_property(devInstance,
> > + &qga_DEVPKEY_Device_HardwareIds, &cm_type);
> > + if (property == NULL) {
> > + slog("failed to get hardware IDs");
> > + return NULL;
> > + }
> > + if (*property == '\0') {
> > + /* empty list */
> > + return NULL;
> > + }
> > + values = g_array_new(TRUE, TRUE, sizeof(gchar*));
> > + for (id = property; '\0' != *id; id += lstrlenW(id) + 1) {
> > + gchar* id8 = g_utf16_to_utf8(id, -1, NULL, NULL, NULL);
> > + g_array_append_val(values, id8);
> > + }
> > + hw_ids = (GStrv)g_array_free(values, FALSE);
> > + values = NULL;
> > + return hw_ids;
>
> Why not return g_array_free(values, FALSE) directly?
Yeah, you're right. This is a relic from before when I used g_autoptr
for it. Could be more straightforward now as you say.
Tomas
>
> > +}
> > +
> > +/*
> > + * https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices
> > + */
> > +#define DEVICE_PCI_RE "PCI\\\\VEN_(1AF4|1B36)&DEV_([0-9A-B]{4})(&|$)"
> > +
> > +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> > +{
> > + GuestDeviceInfoList *head = NULL, *cur_item = NULL, *item = NULL;
> > + HDEVINFO dev_info = INVALID_HANDLE_VALUE;
> > + SP_DEVINFO_DATA dev_info_data;
> > + int i, j;
> > + GError *gerr = NULL;
> > + g_autoptr(GRegex) device_pci_re = NULL;
> > + DEVPROPTYPE cm_type;
> > +
> > + device_pci_re = g_regex_new(DEVICE_PCI_RE,
> > + G_REGEX_ANCHORED | G_REGEX_OPTIMIZE, 0,
> > + &gerr);
> > + g_assert(device_pci_re != NULL);
> > +
> > + dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
> > + dev_info = SetupDiGetClassDevs(0, 0, 0, DIGCF_PRESENT | DIGCF_ALLCLASSES);
> > + if (dev_info == INVALID_HANDLE_VALUE) {
> > + error_setg(errp, "failed to get device tree");
> > + return NULL;
> > + }
> > +
> > + slog("enumerating devices");
> > + for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
> > + bool skip = true;
> > + SYSTEMTIME utc_date;
> > + g_autofree LPWSTR name = NULL;
> > + g_autofree LPFILETIME date = NULL;
> > + g_autofree LPWSTR version = NULL;
> > + g_auto(GStrv) hw_ids = NULL;
> > + g_autoptr(GuestDeviceInfo) device = g_new0(GuestDeviceInfo, 1);
> > + g_autofree char *vendor_id = NULL;
> > + g_autofree char *device_id = NULL;
> > +
> > + name = (LPWSTR)cm_get_property(dev_info_data.DevInst,
> > + &qga_DEVPKEY_NAME, &cm_type);
> > + if (name == NULL) {
> > + slog("failed to get device description");
> > + continue;
> > + }
> > + device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
> > + if (device->driver_name == NULL) {
> > + error_setg(errp, "conversion to utf8 failed (driver name)");
> > + goto out;
> > + }
> > + slog("querying device: %s", device->driver_name);
> > + hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
> > + if (hw_ids == NULL) {
> > + continue;
> > + }
> > + for (j = 0; hw_ids[j] != NULL; j++) {
> > + GMatchInfo *match_info;
> > + if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
> > + continue;
> > + }
> > + skip = false;
> > + vendor_id = g_match_info_fetch(match_info, 1);
> > + device_id = g_match_info_fetch(match_info, 2);
> > + device->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> > + device->device_id = g_ascii_strtoull(device_id, NULL, 16);
> > + g_match_info_free(match_info);
> > + }
> > + if (skip) {
> > + continue;
> > + }
> > +
> > + version = (LPWSTR)cm_get_property(dev_info_data.DevInst,
> > + &qga_DEVPKEY_Device_DriverVersion, &cm_type);
> > + if (version == NULL) {
> > + slog("failed to get driver version");
> > + continue;
> > + }
> > + device->driver_version = g_utf16_to_utf8(version, -1, NULL,
> > + NULL, NULL);
> > + if (device->driver_version == NULL) {
> > + error_setg(errp, "conversion to utf8 failed (driver version)");
> > + goto out;
> > + }
> > +
> > + date = (LPFILETIME)cm_get_property(dev_info_data.DevInst,
> > + &qga_DEVPKEY_Device_DriverDate, &cm_type);
> > + if (date == NULL) {
> > + slog("failed to get driver date");
> > + continue;
> > + }
> > + FileTimeToSystemTime(date, &utc_date);
> > + device->driver_date = g_strdup_printf("%04d-%02d-%02d",
> > + utc_date.wYear, utc_date.wMonth, utc_date.wDay);
> > +
> > + slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,
> > + device->driver_date, device->driver_version);
> > + item = g_new0(GuestDeviceInfoList, 1);
> > + item->value = g_steal_pointer(&device);
> > + if (!cur_item) {
> > + head = cur_item = item;
> > + } else {
> > + cur_item->next = item;
> > + cur_item = item;
> > + }
> > + continue;
> > + }
> > +
> > +out:
> > + if (dev_info != INVALID_HANDLE_VALUE) {
> > + SetupDiDestroyDeviceInfoList(dev_info);
> > + }
> > + return head;
> > +}
> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index fb4605cc19..fe04ff2f39 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -1242,3 +1242,35 @@
> > ##
> > { 'command': 'guest-get-osinfo',
> > 'returns': 'GuestOSInfo' }
> > +
> > +##
> > +# @GuestDeviceInfo:
> > +#
> > +# @vendor-id: vendor ID
> > +# @device-id: device ID
> > +# @driver-name: name of the associated driver
> > +# @driver-date: driver release date in format YYYY-MM-DD
> > +# @driver-version: driver version
> > +#
> > +# Since: 4.2
> > +##
> > +{ 'struct': 'GuestDeviceInfo',
> > + 'data': {
> > + 'vendor-id': 'uint16',
> > + 'device-id': 'uint16',
> > + 'driver-name': 'str',
> > + 'driver-date': 'str',
> > + 'driver-version': 'str'
> > + } }
> > +
> > +##
> > +# @guest-get-devices:
> > +#
> > +# Retrieve information about device drivers in Windows guest
> > +#
> > +# Returns: @GuestDeviceInfo
> > +#
> > +# Since: 4.2
> > +##
> > +{ 'command': 'guest-get-devices',
> > + 'returns': ['GuestDeviceInfo'] }
> > --
> > 2.23.0
> >
> >
>
>
> other than that,
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> --
> Marc-André Lureau
--
Tomáš Golembiovský <tgolembi@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
2019-09-26 10:16 ` Tomáš Golembiovský
@ 2019-09-26 10:22 ` Tomáš Golembiovský
0 siblings, 0 replies; 6+ messages in thread
From: Tomáš Golembiovský @ 2019-09-26 10:22 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: QEMU, Michael Roth
On Thu, Sep 26, 2019 at 12:16:38PM +0200, Tomáš Golembiovský wrote:
> On Thu, Sep 26, 2019 at 11:13:32AM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Sep 26, 2019 at 1:01 AM Tomáš Golembiovský <tgolembi@redhat.com> wrote:
> > >
> > > Add command for reporting devices on Windows guest. The intent is not so
> > > much to report the devices but more importantly the driver (and its
> > > version) that is assigned to the device. This gives caller the
> > > information whether VirtIO drivers are installed and/or whether
> > > inadequate driver is used on a device (e.g. QXL device with base VGA
> > > driver).
> > >
> > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > > ---
> > > qga/commands-posix.c | 9 ++
> > > qga/commands-win32.c | 204 ++++++++++++++++++++++++++++++++++++++++++-
> > > qga/qapi-schema.json | 32 +++++++
> > > 3 files changed, 244 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > > index dfc05f5b8a..58e93feef9 100644
> > > --- a/qga/commands-posix.c
> > > +++ b/qga/commands-posix.c
> > > @@ -2757,6 +2757,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
> > > blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
> > > #endif
> > >
> > > + blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
> > > +
> > > return blacklist;
> > > }
> > >
> > > @@ -2977,3 +2979,10 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> > >
> > > return info;
> > > }
> > > +
> > > +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> > > +{
> > > + error_setg(errp, QERR_UNSUPPORTED);
> > > +
> > > + return NULL;
> > > +}
> > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > > index 6b67f16faf..139dbd7c9a 100644
> > > --- a/qga/commands-win32.c
> > > +++ b/qga/commands-win32.c
> > > @@ -21,10 +21,11 @@
> > > #ifdef CONFIG_QGA_NTDDSCSI
> > > #include <winioctl.h>
> > > #include <ntddscsi.h>
> > > +#endif
> > > #include <setupapi.h>
> > > #include <cfgmgr32.h>
> > > #include <initguid.h>
> > > -#endif
> > > +#include <devpropdef.h>
> > > #include <lm.h>
> > > #include <wtsapi32.h>
> > > #include <wininet.h>
> > > @@ -38,6 +39,36 @@
> > > #include "qemu/host-utils.h"
> > > #include "qemu/base64.h"
> > >
> > > +/*
> > > + * The following should be in devpkey.h, but it isn't. The key names were
> > > + * prefixed to avoid (future) name clashes. Once the definitions get into
> > > + * mingw the following lines can be removed.
> > > + */
> > > +DEFINE_DEVPROPKEY(qga_DEVPKEY_NAME, 0xb725f130, 0x47ef, 0x101a, 0xa5,
> > > + 0xf1, 0x02, 0x60, 0x8c, 0x9e, 0xeb, 0xac, 10);
> > > + /* DEVPROP_TYPE_STRING */
> > > +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_HardwareIds, 0xa45c254e, 0xdf1c,
> > > + 0x4efd, 0x80, 0x20, 0x67, 0xd1, 0x46, 0xa8, 0x50, 0xe0, 3);
> > > + /* DEVPROP_TYPE_STRING_LIST */
> > > +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverDate, 0xa8b865dd, 0x2e3d,
> > > + 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 2);
> > > + /* DEVPROP_TYPE_FILETIME */
> > > +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverVersion, 0xa8b865dd, 0x2e3d,
> > > + 0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 3);
> > > + /* DEVPROP_TYPE_STRING */
> > > +/* The following shoud be in cfgmgr32.h, but it isn't */
> > > +#ifndef CM_Get_DevNode_Property
> > > +CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
> > > + DEVINST dnDevInst,
> > > + CONST DEVPROPKEY * PropertyKey,
> > > + DEVPROPTYPE * PropertyType,
> > > + PBYTE PropertyBuffer,
> > > + PULONG PropertyBufferSize,
> > > + ULONG ulFlags
> > > +);
> > > +#define CM_Get_DevNode_Property CM_Get_DevNode_PropertyW
> > > +#endif
> > > +
> > > #ifndef SHTDN_REASON_FLAG_PLANNED
> > > #define SHTDN_REASON_FLAG_PLANNED 0x80000000
> > > #endif
> > > @@ -92,6 +123,8 @@ static OpenFlags guest_file_open_modes[] = {
> > > g_free(suffix); \
> > > } while (0)
> > >
> > > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(GuestDeviceInfo, qapi_free_GuestDeviceInfo)
> > > +
> > > static OpenFlags *find_open_flag(const char *mode_str)
> > > {
> > > int mode;
> > > @@ -2234,3 +2267,172 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> > >
> > > return info;
> > > }
> > > +
> > > +/*
> > > + * Safely get device property. Returned strings are using wide characters.
> > > + * Caller is responsible for freeing the buffer.
> > > + */
> > > +static LPBYTE cm_get_property(DEVINST devInst, const DEVPROPKEY *propName,
> > > + PDEVPROPTYPE propType)
> > > +{
> > > + CONFIGRET cr;
> > > + g_autofree LPBYTE buffer = NULL;
> > > + ULONG buffer_len = 0;
> > > +
> > > + /* First query for needed space */
> > > + cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> > > + buffer, &buffer_len, 0);
> > > + if (cr != CR_SUCCESS && cr != CR_BUFFER_SMALL) {
> > > +
> > > + slog("failed to get property size, error=0x%lx", cr);
> > > + return NULL;
> > > + }
> > > + buffer = g_new0(BYTE, buffer_len + 1);
> > > + cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> > > + buffer, &buffer_len, 0);
> > > + if (cr != CR_SUCCESS) {
> > > + slog("failed to get device property, error=0x%lx", cr);
> > > + return NULL;
> > > + }
> > > + return g_steal_pointer(&buffer);
> > > +}
> > > +
> > > +static GStrv ga_get_hardware_ids(DEVINST devInstance)
> > > +{
> > > + GStrv hw_ids = NULL;
> > > + GArray *values = NULL;
> > > + DEVPROPTYPE cm_type;
> > > + LPWSTR id;
> > > + g_autofree LPWSTR property = (LPWSTR)cm_get_property(devInstance,
> > > + &qga_DEVPKEY_Device_HardwareIds, &cm_type);
> > > + if (property == NULL) {
> > > + slog("failed to get hardware IDs");
> > > + return NULL;
> > > + }
> > > + if (*property == '\0') {
> > > + /* empty list */
> > > + return NULL;
> > > + }
> > > + values = g_array_new(TRUE, TRUE, sizeof(gchar*));
> > > + for (id = property; '\0' != *id; id += lstrlenW(id) + 1) {
> > > + gchar* id8 = g_utf16_to_utf8(id, -1, NULL, NULL, NULL);
> > > + g_array_append_val(values, id8);
> > > + }
> > > + hw_ids = (GStrv)g_array_free(values, FALSE);
> > > + values = NULL;
> > > + return hw_ids;
> >
> > Why not return g_array_free(values, FALSE) directly?
>
> Yeah, you're right. This is a relic from before when I used g_autoptr
> for it. Could be more straightforward now as you say.
>
> Tomas
Do you want me to send new version?
>
> >
> > > +}
> > > +
> > > +/*
> > > + * https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices
> > > + */
> > > +#define DEVICE_PCI_RE "PCI\\\\VEN_(1AF4|1B36)&DEV_([0-9A-B]{4})(&|$)"
> > > +
> > > +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> > > +{
> > > + GuestDeviceInfoList *head = NULL, *cur_item = NULL, *item = NULL;
> > > + HDEVINFO dev_info = INVALID_HANDLE_VALUE;
> > > + SP_DEVINFO_DATA dev_info_data;
> > > + int i, j;
> > > + GError *gerr = NULL;
> > > + g_autoptr(GRegex) device_pci_re = NULL;
> > > + DEVPROPTYPE cm_type;
> > > +
> > > + device_pci_re = g_regex_new(DEVICE_PCI_RE,
> > > + G_REGEX_ANCHORED | G_REGEX_OPTIMIZE, 0,
> > > + &gerr);
> > > + g_assert(device_pci_re != NULL);
> > > +
> > > + dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
> > > + dev_info = SetupDiGetClassDevs(0, 0, 0, DIGCF_PRESENT | DIGCF_ALLCLASSES);
> > > + if (dev_info == INVALID_HANDLE_VALUE) {
> > > + error_setg(errp, "failed to get device tree");
> > > + return NULL;
> > > + }
> > > +
> > > + slog("enumerating devices");
> > > + for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
> > > + bool skip = true;
> > > + SYSTEMTIME utc_date;
> > > + g_autofree LPWSTR name = NULL;
> > > + g_autofree LPFILETIME date = NULL;
> > > + g_autofree LPWSTR version = NULL;
> > > + g_auto(GStrv) hw_ids = NULL;
> > > + g_autoptr(GuestDeviceInfo) device = g_new0(GuestDeviceInfo, 1);
> > > + g_autofree char *vendor_id = NULL;
> > > + g_autofree char *device_id = NULL;
> > > +
> > > + name = (LPWSTR)cm_get_property(dev_info_data.DevInst,
> > > + &qga_DEVPKEY_NAME, &cm_type);
> > > + if (name == NULL) {
> > > + slog("failed to get device description");
> > > + continue;
> > > + }
> > > + device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
> > > + if (device->driver_name == NULL) {
> > > + error_setg(errp, "conversion to utf8 failed (driver name)");
> > > + goto out;
> > > + }
> > > + slog("querying device: %s", device->driver_name);
> > > + hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
> > > + if (hw_ids == NULL) {
> > > + continue;
> > > + }
> > > + for (j = 0; hw_ids[j] != NULL; j++) {
> > > + GMatchInfo *match_info;
> > > + if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
> > > + continue;
> > > + }
> > > + skip = false;
> > > + vendor_id = g_match_info_fetch(match_info, 1);
> > > + device_id = g_match_info_fetch(match_info, 2);
> > > + device->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> > > + device->device_id = g_ascii_strtoull(device_id, NULL, 16);
> > > + g_match_info_free(match_info);
> > > + }
> > > + if (skip) {
> > > + continue;
> > > + }
> > > +
> > > + version = (LPWSTR)cm_get_property(dev_info_data.DevInst,
> > > + &qga_DEVPKEY_Device_DriverVersion, &cm_type);
> > > + if (version == NULL) {
> > > + slog("failed to get driver version");
> > > + continue;
> > > + }
> > > + device->driver_version = g_utf16_to_utf8(version, -1, NULL,
> > > + NULL, NULL);
> > > + if (device->driver_version == NULL) {
> > > + error_setg(errp, "conversion to utf8 failed (driver version)");
> > > + goto out;
> > > + }
> > > +
> > > + date = (LPFILETIME)cm_get_property(dev_info_data.DevInst,
> > > + &qga_DEVPKEY_Device_DriverDate, &cm_type);
> > > + if (date == NULL) {
> > > + slog("failed to get driver date");
> > > + continue;
> > > + }
> > > + FileTimeToSystemTime(date, &utc_date);
> > > + device->driver_date = g_strdup_printf("%04d-%02d-%02d",
> > > + utc_date.wYear, utc_date.wMonth, utc_date.wDay);
> > > +
> > > + slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,
> > > + device->driver_date, device->driver_version);
> > > + item = g_new0(GuestDeviceInfoList, 1);
> > > + item->value = g_steal_pointer(&device);
> > > + if (!cur_item) {
> > > + head = cur_item = item;
> > > + } else {
> > > + cur_item->next = item;
> > > + cur_item = item;
> > > + }
> > > + continue;
> > > + }
> > > +
> > > +out:
> > > + if (dev_info != INVALID_HANDLE_VALUE) {
> > > + SetupDiDestroyDeviceInfoList(dev_info);
> > > + }
> > > + return head;
> > > +}
> > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > > index fb4605cc19..fe04ff2f39 100644
> > > --- a/qga/qapi-schema.json
> > > +++ b/qga/qapi-schema.json
> > > @@ -1242,3 +1242,35 @@
> > > ##
> > > { 'command': 'guest-get-osinfo',
> > > 'returns': 'GuestOSInfo' }
> > > +
> > > +##
> > > +# @GuestDeviceInfo:
> > > +#
> > > +# @vendor-id: vendor ID
> > > +# @device-id: device ID
> > > +# @driver-name: name of the associated driver
> > > +# @driver-date: driver release date in format YYYY-MM-DD
> > > +# @driver-version: driver version
> > > +#
> > > +# Since: 4.2
> > > +##
> > > +{ 'struct': 'GuestDeviceInfo',
> > > + 'data': {
> > > + 'vendor-id': 'uint16',
> > > + 'device-id': 'uint16',
> > > + 'driver-name': 'str',
> > > + 'driver-date': 'str',
> > > + 'driver-version': 'str'
> > > + } }
> > > +
> > > +##
> > > +# @guest-get-devices:
> > > +#
> > > +# Retrieve information about device drivers in Windows guest
> > > +#
> > > +# Returns: @GuestDeviceInfo
> > > +#
> > > +# Since: 4.2
> > > +##
> > > +{ 'command': 'guest-get-devices',
> > > + 'returns': ['GuestDeviceInfo'] }
> > > --
> > > 2.23.0
> > >
> > >
> >
> >
> > other than that,
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > --
> > Marc-André Lureau
>
> --
> Tomáš Golembiovský <tgolembi@redhat.com>
--
Tomáš Golembiovský <tgolembi@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
2019-09-25 21:00 [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices Tomáš Golembiovský
2019-09-26 7:13 ` Marc-André Lureau
@ 2019-09-27 1:12 ` no-reply
2019-09-27 9:09 ` Tomáš Golembiovský
1 sibling, 1 reply; 6+ messages in thread
From: no-reply @ 2019-09-27 1:12 UTC (permalink / raw)
To: tgolembi; +Cc: marcandre.lureau, tgolembi, qemu-devel, mdroth
Patchew URL: https://patchew.org/QEMU/919bbd6e0557d2fe2d9c17de394cc0b4c6fa4426.1569445204.git.tgolembi@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 919bbd6e0557d2fe2d9c17de394cc0b4c6fa4426.1569445204.git.tgolembi@redhat.com
Subject: [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
42c2919 qga: add command guest-get-devices for reporting VirtIO devices
=== OUTPUT BEGIN ===
ERROR: "(foo*)" should be "(foo *)"
#155: FILE: qga/commands-win32.c:2316:
+ values = g_array_new(TRUE, TRUE, sizeof(gchar*));
ERROR: "foo* bar" should be "foo *bar"
#157: FILE: qga/commands-win32.c:2318:
+ gchar* id8 = g_utf16_to_utf8(id, -1, NULL, NULL, NULL);
total: 2 errors, 0 warnings, 281 lines checked
Commit 42c2919fb935 (qga: add command guest-get-devices for reporting VirtIO devices) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/919bbd6e0557d2fe2d9c17de394cc0b4c6fa4426.1569445204.git.tgolembi@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
2019-09-27 1:12 ` no-reply
@ 2019-09-27 9:09 ` Tomáš Golembiovský
0 siblings, 0 replies; 6+ messages in thread
From: Tomáš Golembiovský @ 2019-09-27 9:09 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, mdroth
already fixed in v4
On Thu, Sep 26, 2019 at 06:12:26PM -0700, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/919bbd6e0557d2fe2d9c17de394cc0b4c6fa4426.1569445204.git.tgolembi@redhat.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 919bbd6e0557d2fe2d9c17de394cc0b4c6fa4426.1569445204.git.tgolembi@redhat.com
> Subject: [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> Switched to a new branch 'test'
> 42c2919 qga: add command guest-get-devices for reporting VirtIO devices
>
> === OUTPUT BEGIN ===
> ERROR: "(foo*)" should be "(foo *)"
> #155: FILE: qga/commands-win32.c:2316:
> + values = g_array_new(TRUE, TRUE, sizeof(gchar*));
>
> ERROR: "foo* bar" should be "foo *bar"
> #157: FILE: qga/commands-win32.c:2318:
> + gchar* id8 = g_utf16_to_utf8(id, -1, NULL, NULL, NULL);
>
> total: 2 errors, 0 warnings, 281 lines checked
>
> Commit 42c2919fb935 (qga: add command guest-get-devices for reporting VirtIO devices) has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> The full log is available at
> http://patchew.org/logs/919bbd6e0557d2fe2d9c17de394cc0b4c6fa4426.1569445204.git.tgolembi@redhat.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
--
Tomáš Golembiovský <tgolembi@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-27 9:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-25 21:00 [PATCH v3] qga: add command guest-get-devices for reporting VirtIO devices Tomáš Golembiovský
2019-09-26 7:13 ` Marc-André Lureau
2019-09-26 10:16 ` Tomáš Golembiovský
2019-09-26 10:22 ` Tomáš Golembiovský
2019-09-27 1:12 ` no-reply
2019-09-27 9:09 ` Tomáš Golembiovský
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).