All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices
@ 2020-01-09 12:39 Tomáš Golembiovský
  2020-01-09 13:47 ` Marc-André Lureau
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Tomáš Golembiovský @ 2020-01-09 12:39 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Tomáš Golembiovský,
	Daniel P. Berrangé,
	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).

Example:
[
    {
      "driver-date": "2019-08-12",
      "driver-name": "Red Hat VirtIO SCSI controller",
      "driver-version": "100.80.104.17300",
      "address": {
        "type": "pci",
        "data": {
          "device-id": 4162,
          "vendor-id": 6900
        }
      }
    },
    ...
]

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---

Changes in v9: fixed compilation errors

 qga/commands-posix.c |   9 ++
 qga/commands-win32.c | 212 ++++++++++++++++++++++++++++++++++++++++++-
 qga/qapi-schema.json |  51 +++++++++++
 3 files changed, 271 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 93474ff770..bffee8ce48 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2771,6 +2771,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;
 }
 
@@ -2991,3 +2993,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 2461fd19bf..b18d89d7ad 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,180 @@ 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)
+{
+    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);
+    }
+    return (GStrv)g_array_free(values, FALSE);
+}
+
+/*
+ * 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)");
+            continue;
+        }
+        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;
+            GuestDeviceAddressPCI *address;
+            if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
+                continue;
+            }
+            skip = false;
+
+            address = g_new0(GuestDeviceAddressPCI, 1);
+            vendor_id = g_match_info_fetch(match_info, 1);
+            device_id = g_match_info_fetch(match_info, 2);
+            address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+            address->device_id = g_ascii_strtoull(device_id, NULL, 16);
+
+            device->address = g_new0(GuestDeviceAddress, 1);
+            device->has_address = true;
+            device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
+            device->address->u.pci.data = address;
+
+            g_match_info_free(match_info);
+            break;
+        }
+        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)");
+            continue;
+        }
+        device->has_driver_version = true;
+
+        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);
+        device->has_driver_date = true;
+
+        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;
+    }
+
+    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..92ed76c419 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1242,3 +1242,54 @@
 ##
 { 'command': 'guest-get-osinfo',
   'returns': 'GuestOSInfo' }
+
+##
+# @GuestDeviceAddressPCI:
+#
+# @vendor-id: vendor ID
+# @device-id: device ID
+#
+# Since: 5.0
+##
+{ 'struct': 'GuestDeviceAddressPCI',
+  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
+
+##
+# @GuestDeviceAddress:
+#
+# Address of the device
+# - @pci: address of PCI device, since: 5.0
+#
+# Since: 5.0
+##
+{ 'union': 'GuestDeviceAddress',
+  'data': { 'pci': 'GuestDeviceAddressPCI' } }
+
+##
+# @GuestDeviceInfo:
+#
+# @driver-name: name of the associated driver
+# @driver-date: driver release date in format YYYY-MM-DD
+# @driver-version: driver version
+#
+# Since: 5.0
+##
+{ 'struct': 'GuestDeviceInfo',
+  'data': {
+      'driver-name': 'str',
+      '*driver-date': 'str',
+      '*driver-version': 'str',
+      '*address': 'GuestDeviceAddress'
+  } }
+
+##
+# @guest-get-devices:
+#
+# Retrieve information about device drivers in Windows guest
+#
+# Returns: @GuestDeviceInfo
+#
+# Since: 5.0
+##
+{ 'command': 'guest-get-devices',
+  'returns': ['GuestDeviceInfo'] }
-- 
2.24.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices
  2020-01-09 12:39 [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices Tomáš Golembiovský
@ 2020-01-09 13:47 ` Marc-André Lureau
  2020-02-26 17:01 ` Philippe Mathieu-Daudé
  2020-07-21  8:01 ` Tomáš Golembiovský
  2 siblings, 0 replies; 7+ messages in thread
From: Marc-André Lureau @ 2020-01-09 13:47 UTC (permalink / raw)
  To: Tomáš Golembiovský
  Cc: Daniel P. Berrangé, QEMU, Michael Roth

On Thu, Jan 9, 2020 at 4:40 PM 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).
>
> Example:
> [
>     {
>       "driver-date": "2019-08-12",
>       "driver-name": "Red Hat VirtIO SCSI controller",
>       "driver-version": "100.80.104.17300",
>       "address": {
>         "type": "pci",
>         "data": {
>           "device-id": 4162,
>           "vendor-id": 6900
>         }
>       }
>     },
>     ...
> ]
>
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

(please keep my r-b unless you do significant change, thanks)

> ---
>
> Changes in v9: fixed compilation errors
>
>  qga/commands-posix.c |   9 ++
>  qga/commands-win32.c | 212 ++++++++++++++++++++++++++++++++++++++++++-
>  qga/qapi-schema.json |  51 +++++++++++
>  3 files changed, 271 insertions(+), 1 deletion(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 93474ff770..bffee8ce48 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2771,6 +2771,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;
>  }
>
> @@ -2991,3 +2993,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 2461fd19bf..b18d89d7ad 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,180 @@ 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)
> +{
> +    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);
> +    }
> +    return (GStrv)g_array_free(values, FALSE);
> +}
> +
> +/*
> + * 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)");
> +            continue;
> +        }
> +        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;
> +            GuestDeviceAddressPCI *address;
> +            if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
> +                continue;
> +            }
> +            skip = false;
> +
> +            address = g_new0(GuestDeviceAddressPCI, 1);
> +            vendor_id = g_match_info_fetch(match_info, 1);
> +            device_id = g_match_info_fetch(match_info, 2);
> +            address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> +            address->device_id = g_ascii_strtoull(device_id, NULL, 16);
> +
> +            device->address = g_new0(GuestDeviceAddress, 1);
> +            device->has_address = true;
> +            device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
> +            device->address->u.pci.data = address;
> +
> +            g_match_info_free(match_info);
> +            break;
> +        }
> +        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)");
> +            continue;
> +        }
> +        device->has_driver_version = true;
> +
> +        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);
> +        device->has_driver_date = true;
> +
> +        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;
> +    }
> +
> +    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..92ed76c419 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1242,3 +1242,54 @@
>  ##
>  { 'command': 'guest-get-osinfo',
>    'returns': 'GuestOSInfo' }
> +
> +##
> +# @GuestDeviceAddressPCI:
> +#
> +# @vendor-id: vendor ID
> +# @device-id: device ID
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'GuestDeviceAddressPCI',
> +  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
> +
> +##
> +# @GuestDeviceAddress:
> +#
> +# Address of the device
> +# - @pci: address of PCI device, since: 5.0
> +#
> +# Since: 5.0
> +##
> +{ 'union': 'GuestDeviceAddress',
> +  'data': { 'pci': 'GuestDeviceAddressPCI' } }
> +
> +##
> +# @GuestDeviceInfo:
> +#
> +# @driver-name: name of the associated driver
> +# @driver-date: driver release date in format YYYY-MM-DD
> +# @driver-version: driver version
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'GuestDeviceInfo',
> +  'data': {
> +      'driver-name': 'str',
> +      '*driver-date': 'str',
> +      '*driver-version': 'str',
> +      '*address': 'GuestDeviceAddress'
> +  } }
> +
> +##
> +# @guest-get-devices:
> +#
> +# Retrieve information about device drivers in Windows guest
> +#
> +# Returns: @GuestDeviceInfo
> +#
> +# Since: 5.0
> +##
> +{ 'command': 'guest-get-devices',
> +  'returns': ['GuestDeviceInfo'] }
> --
> 2.24.1
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices
  2020-01-09 12:39 [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices Tomáš Golembiovský
  2020-01-09 13:47 ` Marc-André Lureau
@ 2020-02-26 17:01 ` Philippe Mathieu-Daudé
  2020-07-21  8:01 ` Tomáš Golembiovský
  2 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-26 17:01 UTC (permalink / raw)
  To: Tomáš Golembiovský, qemu-devel
  Cc: Marc-André Lureau, Daniel P. Berrangé,
	Michael Roth, Paolo Bonzini

Paolo, can you take this patch via your misc-tree?

On 1/9/20 1:39 PM, Tomáš Golembiovský 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).
> 
> Example:
> [
>      {
>        "driver-date": "2019-08-12",
>        "driver-name": "Red Hat VirtIO SCSI controller",
>        "driver-version": "100.80.104.17300",
>        "address": {
>          "type": "pci",
>          "data": {
>            "device-id": 4162,
>            "vendor-id": 6900
>          }
>        }
>      },
>      ...
> ]
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
> 
> Changes in v9: fixed compilation errors
> 
>   qga/commands-posix.c |   9 ++
>   qga/commands-win32.c | 212 ++++++++++++++++++++++++++++++++++++++++++-
>   qga/qapi-schema.json |  51 +++++++++++
>   3 files changed, 271 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 93474ff770..bffee8ce48 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2771,6 +2771,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;
>   }
>   
> @@ -2991,3 +2993,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 2461fd19bf..b18d89d7ad 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,180 @@ 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)
> +{
> +    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);
> +    }
> +    return (GStrv)g_array_free(values, FALSE);
> +}
> +
> +/*
> + * 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)");
> +            continue;
> +        }
> +        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;
> +            GuestDeviceAddressPCI *address;
> +            if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
> +                continue;
> +            }
> +            skip = false;
> +
> +            address = g_new0(GuestDeviceAddressPCI, 1);
> +            vendor_id = g_match_info_fetch(match_info, 1);
> +            device_id = g_match_info_fetch(match_info, 2);
> +            address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> +            address->device_id = g_ascii_strtoull(device_id, NULL, 16);
> +
> +            device->address = g_new0(GuestDeviceAddress, 1);
> +            device->has_address = true;
> +            device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
> +            device->address->u.pci.data = address;
> +
> +            g_match_info_free(match_info);
> +            break;
> +        }
> +        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)");
> +            continue;
> +        }
> +        device->has_driver_version = true;
> +
> +        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);
> +        device->has_driver_date = true;
> +
> +        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;
> +    }
> +
> +    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..92ed76c419 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1242,3 +1242,54 @@
>   ##
>   { 'command': 'guest-get-osinfo',
>     'returns': 'GuestOSInfo' }
> +
> +##
> +# @GuestDeviceAddressPCI:
> +#
> +# @vendor-id: vendor ID
> +# @device-id: device ID
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'GuestDeviceAddressPCI',
> +  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
> +
> +##
> +# @GuestDeviceAddress:
> +#
> +# Address of the device
> +# - @pci: address of PCI device, since: 5.0
> +#
> +# Since: 5.0
> +##
> +{ 'union': 'GuestDeviceAddress',
> +  'data': { 'pci': 'GuestDeviceAddressPCI' } }
> +
> +##
> +# @GuestDeviceInfo:
> +#
> +# @driver-name: name of the associated driver
> +# @driver-date: driver release date in format YYYY-MM-DD
> +# @driver-version: driver version
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'GuestDeviceInfo',
> +  'data': {
> +      'driver-name': 'str',
> +      '*driver-date': 'str',
> +      '*driver-version': 'str',
> +      '*address': 'GuestDeviceAddress'
> +  } }
> +
> +##
> +# @guest-get-devices:
> +#
> +# Retrieve information about device drivers in Windows guest
> +#
> +# Returns: @GuestDeviceInfo
> +#
> +# Since: 5.0
> +##
> +{ 'command': 'guest-get-devices',
> +  'returns': ['GuestDeviceInfo'] }
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices
  2020-01-09 12:39 [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices Tomáš Golembiovský
  2020-01-09 13:47 ` Marc-André Lureau
  2020-02-26 17:01 ` Philippe Mathieu-Daudé
@ 2020-07-21  8:01 ` Tomáš Golembiovský
  2020-07-21 11:31   ` Marc-André Lureau
  2 siblings, 1 reply; 7+ messages in thread
From: Tomáš Golembiovský @ 2020-07-21  8:01 UTC (permalink / raw)
  To: qemu-devel, Michael Roth; +Cc: Marc-André Lureau, Daniel P. Berrangé

Ping. Can we get this merged finally?

Thanks,

    Tomas

On Thu,  9 Jan 2020 13:39:36 +0100
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).
> 
> Example:
> [
>     {
>       "driver-date": "2019-08-12",
>       "driver-name": "Red Hat VirtIO SCSI controller",
>       "driver-version": "100.80.104.17300",
>       "address": {
>         "type": "pci",
>         "data": {
>           "device-id": 4162,
>           "vendor-id": 6900
>         }
>       }
>     },
>     ...
> ]
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
> 
> Changes in v9: fixed compilation errors
> 
>  qga/commands-posix.c |   9 ++
>  qga/commands-win32.c | 212 ++++++++++++++++++++++++++++++++++++++++++-
>  qga/qapi-schema.json |  51 +++++++++++
>  3 files changed, 271 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 93474ff770..bffee8ce48 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2771,6 +2771,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;
>  }
>  
> @@ -2991,3 +2993,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 2461fd19bf..b18d89d7ad 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,180 @@ 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)
> +{
> +    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);
> +    }
> +    return (GStrv)g_array_free(values, FALSE);
> +}
> +
> +/*
> + * 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)");
> +            continue;
> +        }
> +        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;
> +            GuestDeviceAddressPCI *address;
> +            if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
> +                continue;
> +            }
> +            skip = false;
> +
> +            address = g_new0(GuestDeviceAddressPCI, 1);
> +            vendor_id = g_match_info_fetch(match_info, 1);
> +            device_id = g_match_info_fetch(match_info, 2);
> +            address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> +            address->device_id = g_ascii_strtoull(device_id, NULL, 16);
> +
> +            device->address = g_new0(GuestDeviceAddress, 1);
> +            device->has_address = true;
> +            device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
> +            device->address->u.pci.data = address;
> +
> +            g_match_info_free(match_info);
> +            break;
> +        }
> +        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)");
> +            continue;
> +        }
> +        device->has_driver_version = true;
> +
> +        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);
> +        device->has_driver_date = true;
> +
> +        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;
> +    }
> +
> +    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..92ed76c419 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1242,3 +1242,54 @@
>  ##
>  { 'command': 'guest-get-osinfo',
>    'returns': 'GuestOSInfo' }
> +
> +##
> +# @GuestDeviceAddressPCI:
> +#
> +# @vendor-id: vendor ID
> +# @device-id: device ID
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'GuestDeviceAddressPCI',
> +  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
> +
> +##
> +# @GuestDeviceAddress:
> +#
> +# Address of the device
> +# - @pci: address of PCI device, since: 5.0
> +#
> +# Since: 5.0
> +##
> +{ 'union': 'GuestDeviceAddress',
> +  'data': { 'pci': 'GuestDeviceAddressPCI' } }
> +
> +##
> +# @GuestDeviceInfo:
> +#
> +# @driver-name: name of the associated driver
> +# @driver-date: driver release date in format YYYY-MM-DD
> +# @driver-version: driver version
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'GuestDeviceInfo',
> +  'data': {
> +      'driver-name': 'str',
> +      '*driver-date': 'str',
> +      '*driver-version': 'str',
> +      '*address': 'GuestDeviceAddress'
> +  } }
> +
> +##
> +# @guest-get-devices:
> +#
> +# Retrieve information about device drivers in Windows guest
> +#
> +# Returns: @GuestDeviceInfo
> +#
> +# Since: 5.0
> +##
> +{ 'command': 'guest-get-devices',
> +  'returns': ['GuestDeviceInfo'] }
> -- 
> 2.24.1
> 


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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices
  2020-07-21  8:01 ` Tomáš Golembiovský
@ 2020-07-21 11:31   ` Marc-André Lureau
  2020-07-21 11:43     ` Daniel P. Berrangé
  0 siblings, 1 reply; 7+ messages in thread
From: Marc-André Lureau @ 2020-07-21 11:31 UTC (permalink / raw)
  To: Tomáš Golembiovský
  Cc: Daniel P. Berrangé, QEMU, Michael Roth

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

Hi

On Tue, Jul 21, 2020 at 12:03 PM Tomáš Golembiovský <tgolembi@redhat.com>
wrote:

> Ping. Can we get this merged finally?
>

We missed the feature deadline by a week:
https://wiki.qemu.org/Planning/5.1


> Thanks,
>
>     Tomas
>
> On Thu,  9 Jan 2020 13:39:36 +0100
> 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).
> >
> > Example:
> > [
> >     {
> >       "driver-date": "2019-08-12",
> >       "driver-name": "Red Hat VirtIO SCSI controller",
> >       "driver-version": "100.80.104.17300",
> >       "address": {
> >         "type": "pci",
> >         "data": {
> >           "device-id": 4162,
> >           "vendor-id": 6900
> >         }
> >       }
> >     },
> >     ...
> > ]
> >
> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > ---
> >
> > Changes in v9: fixed compilation errors
> >
> >  qga/commands-posix.c |   9 ++
> >  qga/commands-win32.c | 212 ++++++++++++++++++++++++++++++++++++++++++-
> >  qga/qapi-schema.json |  51 +++++++++++
> >  3 files changed, 271 insertions(+), 1 deletion(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index 93474ff770..bffee8ce48 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -2771,6 +2771,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;
> >  }
> >
> > @@ -2991,3 +2993,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 2461fd19bf..b18d89d7ad 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,180 @@ 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)
> > +{
> > +    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);
> > +    }
> > +    return (GStrv)g_array_free(values, FALSE);
> > +}
> > +
> > +/*
> > + *
> 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)");
> > +            continue;
> > +        }
> > +        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;
> > +            GuestDeviceAddressPCI *address;
> > +            if (!g_regex_match(device_pci_re, hw_ids[j], 0,
> &match_info)) {
> > +                continue;
> > +            }
> > +            skip = false;
> > +
> > +            address = g_new0(GuestDeviceAddressPCI, 1);
> > +            vendor_id = g_match_info_fetch(match_info, 1);
> > +            device_id = g_match_info_fetch(match_info, 2);
> > +            address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> > +            address->device_id = g_ascii_strtoull(device_id, NULL, 16);
> > +
> > +            device->address = g_new0(GuestDeviceAddress, 1);
> > +            device->has_address = true;
> > +            device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
> > +            device->address->u.pci.data = address;
> > +
> > +            g_match_info_free(match_info);
> > +            break;
> > +        }
> > +        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)");
> > +            continue;
> > +        }
> > +        device->has_driver_version = true;
> > +
> > +        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);
> > +        device->has_driver_date = true;
> > +
> > +        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;
> > +    }
> > +
> > +    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..92ed76c419 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -1242,3 +1242,54 @@
> >  ##
> >  { 'command': 'guest-get-osinfo',
> >    'returns': 'GuestOSInfo' }
> > +
> > +##
> > +# @GuestDeviceAddressPCI:
> > +#
> > +# @vendor-id: vendor ID
> > +# @device-id: device ID
> > +#
> > +# Since: 5.0
> > +##
> > +{ 'struct': 'GuestDeviceAddressPCI',
> > +  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
> > +
> > +##
> > +# @GuestDeviceAddress:
> > +#
> > +# Address of the device
> > +# - @pci: address of PCI device, since: 5.0
> > +#
> > +# Since: 5.0
> > +##
> > +{ 'union': 'GuestDeviceAddress',
> > +  'data': { 'pci': 'GuestDeviceAddressPCI' } }
> > +
> > +##
> > +# @GuestDeviceInfo:
> > +#
> > +# @driver-name: name of the associated driver
> > +# @driver-date: driver release date in format YYYY-MM-DD
> > +# @driver-version: driver version
> > +#
> > +# Since: 5.0
> > +##
> > +{ 'struct': 'GuestDeviceInfo',
> > +  'data': {
> > +      'driver-name': 'str',
> > +      '*driver-date': 'str',
> > +      '*driver-version': 'str',
> > +      '*address': 'GuestDeviceAddress'
> > +  } }
> > +
> > +##
> > +# @guest-get-devices:
> > +#
> > +# Retrieve information about device drivers in Windows guest
> > +#
> > +# Returns: @GuestDeviceInfo
> > +#
> > +# Since: 5.0
> > +##
> > +{ 'command': 'guest-get-devices',
> > +  'returns': ['GuestDeviceInfo'] }
> > --
> > 2.24.1
> >
>
>
> --
> Tomáš Golembiovský <tgolembi@redhat.com>
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 17825 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices
  2020-07-21 11:31   ` Marc-André Lureau
@ 2020-07-21 11:43     ` Daniel P. Berrangé
  2020-07-21 15:28       ` Tomáš Golembiovský
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrangé @ 2020-07-21 11:43 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Tomáš Golembiovský, QEMU, Michael Roth

On Tue, Jul 21, 2020 at 03:31:52PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jul 21, 2020 at 12:03 PM Tomáš Golembiovský <tgolembi@redhat.com>
> wrote:
> 
> > Ping. Can we get this merged finally?
> >
> 
> We missed the feature deadline by a week:
> https://wiki.qemu.org/Planning/5.1

Note, if a patch series from a contributor is ready, the subsystem
maintainer should queue it in their subsystem tree, so it is ready
for a pull request when the main tree opens up again. ie the subsys
maintainer is the one responsible for dealing with feature freeze
coordination, not the individual contriubutors.

In this case the "Since 5.0" tags need updating to 5.2 now. The
maintainer could handle this, but since this series is over 6 months
old now, it is probably worth rebasing & reposting.

> > Thanks,
> >
> >     Tomas
> >
> > On Thu,  9 Jan 2020 13:39:36 +0100
> > 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).
> > >
> > > Example:
> > > [
> > >     {
> > >       "driver-date": "2019-08-12",
> > >       "driver-name": "Red Hat VirtIO SCSI controller",
> > >       "driver-version": "100.80.104.17300",
> > >       "address": {
> > >         "type": "pci",
> > >         "data": {
> > >           "device-id": 4162,
> > >           "vendor-id": 6900
> > >         }
> > >       }
> > >     },
> > >     ...
> > > ]
> > >
> > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > > ---
> > >
> > > Changes in v9: fixed compilation errors
> > >
> > >  qga/commands-posix.c |   9 ++
> > >  qga/commands-win32.c | 212 ++++++++++++++++++++++++++++++++++++++++++-
> > >  qga/qapi-schema.json |  51 +++++++++++
> > >  3 files changed, 271 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > > index 93474ff770..bffee8ce48 100644
> > > --- a/qga/commands-posix.c
> > > +++ b/qga/commands-posix.c
> > > @@ -2771,6 +2771,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;
> > >  }
> > >
> > > @@ -2991,3 +2993,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 2461fd19bf..b18d89d7ad 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,180 @@ 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)
> > > +{
> > > +    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);
> > > +    }
> > > +    return (GStrv)g_array_free(values, FALSE);
> > > +}
> > > +
> > > +/*
> > > + *
> > 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)");
> > > +            continue;
> > > +        }
> > > +        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;
> > > +            GuestDeviceAddressPCI *address;
> > > +            if (!g_regex_match(device_pci_re, hw_ids[j], 0,
> > &match_info)) {
> > > +                continue;
> > > +            }
> > > +            skip = false;
> > > +
> > > +            address = g_new0(GuestDeviceAddressPCI, 1);
> > > +            vendor_id = g_match_info_fetch(match_info, 1);
> > > +            device_id = g_match_info_fetch(match_info, 2);
> > > +            address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> > > +            address->device_id = g_ascii_strtoull(device_id, NULL, 16);
> > > +
> > > +            device->address = g_new0(GuestDeviceAddress, 1);
> > > +            device->has_address = true;
> > > +            device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
> > > +            device->address->u.pci.data = address;
> > > +
> > > +            g_match_info_free(match_info);
> > > +            break;
> > > +        }
> > > +        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)");
> > > +            continue;
> > > +        }
> > > +        device->has_driver_version = true;
> > > +
> > > +        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);
> > > +        device->has_driver_date = true;
> > > +
> > > +        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;
> > > +    }
> > > +
> > > +    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..92ed76c419 100644
> > > --- a/qga/qapi-schema.json
> > > +++ b/qga/qapi-schema.json
> > > @@ -1242,3 +1242,54 @@
> > >  ##
> > >  { 'command': 'guest-get-osinfo',
> > >    'returns': 'GuestOSInfo' }
> > > +
> > > +##
> > > +# @GuestDeviceAddressPCI:
> > > +#
> > > +# @vendor-id: vendor ID
> > > +# @device-id: device ID
> > > +#
> > > +# Since: 5.0
> > > +##
> > > +{ 'struct': 'GuestDeviceAddressPCI',
> > > +  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
> > > +
> > > +##
> > > +# @GuestDeviceAddress:
> > > +#
> > > +# Address of the device
> > > +# - @pci: address of PCI device, since: 5.0
> > > +#
> > > +# Since: 5.0
> > > +##
> > > +{ 'union': 'GuestDeviceAddress',
> > > +  'data': { 'pci': 'GuestDeviceAddressPCI' } }
> > > +
> > > +##
> > > +# @GuestDeviceInfo:
> > > +#
> > > +# @driver-name: name of the associated driver
> > > +# @driver-date: driver release date in format YYYY-MM-DD
> > > +# @driver-version: driver version
> > > +#
> > > +# Since: 5.0
> > > +##
> > > +{ 'struct': 'GuestDeviceInfo',
> > > +  'data': {
> > > +      'driver-name': 'str',
> > > +      '*driver-date': 'str',
> > > +      '*driver-version': 'str',
> > > +      '*address': 'GuestDeviceAddress'
> > > +  } }
> > > +
> > > +##
> > > +# @guest-get-devices:
> > > +#
> > > +# Retrieve information about device drivers in Windows guest
> > > +#
> > > +# Returns: @GuestDeviceInfo
> > > +#
> > > +# Since: 5.0
> > > +##
> > > +{ 'command': 'guest-get-devices',
> > > +  'returns': ['GuestDeviceInfo'] }
> > > --
> > > 2.24.1
> > >
> >
> >
> > --
> > Tomáš Golembiovský <tgolembi@redhat.com>
> >
> >
> >
> 
> -- 
> Marc-André Lureau

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices
  2020-07-21 11:43     ` Daniel P. Berrangé
@ 2020-07-21 15:28       ` Tomáš Golembiovský
  0 siblings, 0 replies; 7+ messages in thread
From: Tomáš Golembiovský @ 2020-07-21 15:28 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: Marc-André Lureau, QEMU, Michael Roth

On Tue, 21 Jul 2020 12:43:35 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Tue, Jul 21, 2020 at 03:31:52PM +0400, Marc-André Lureau wrote:
> > Hi
> > 
> > On Tue, Jul 21, 2020 at 12:03 PM Tomáš Golembiovský <tgolembi@redhat.com>
> > wrote:
> >   
> > > Ping. Can we get this merged finally?
> > >  
> > 
> > We missed the feature deadline by a week:
> > https://wiki.qemu.org/Planning/5.1  
> 
> Note, if a patch series from a contributor is ready, the subsystem
> maintainer should queue it in their subsystem tree, so it is ready
> for a pull request when the main tree opens up again. ie the subsys
> maintainer is the one responsible for dealing with feature freeze
> coordination, not the individual contriubutors.
> 
> In this case the "Since 5.0" tags need updating to 5.2 now. The
> maintainer could handle this, but since this series is over 6 months
> old now, it is probably worth rebasing & reposting.

Good point. I will rebase and post again.

    Tomas

> 
> > > Thanks,
> > >
> > >     Tomas
> > >
> > > On Thu,  9 Jan 2020 13:39:36 +0100
> > > 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).
> > > >
> > > > Example:
> > > > [
> > > >     {
> > > >       "driver-date": "2019-08-12",
> > > >       "driver-name": "Red Hat VirtIO SCSI controller",
> > > >       "driver-version": "100.80.104.17300",
> > > >       "address": {
> > > >         "type": "pci",
> > > >         "data": {
> > > >           "device-id": 4162,
> > > >           "vendor-id": 6900
> > > >         }
> > > >       }
> > > >     },
> > > >     ...
> > > > ]
> > > >
> > > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > > > ---
> > > >
> > > > Changes in v9: fixed compilation errors
> > > >
> > > >  qga/commands-posix.c |   9 ++
> > > >  qga/commands-win32.c | 212 ++++++++++++++++++++++++++++++++++++++++++-
> > > >  qga/qapi-schema.json |  51 +++++++++++
> > > >  3 files changed, 271 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > > > index 93474ff770..bffee8ce48 100644
> > > > --- a/qga/commands-posix.c
> > > > +++ b/qga/commands-posix.c
> > > > @@ -2771,6 +2771,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;
> > > >  }
> > > >
> > > > @@ -2991,3 +2993,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 2461fd19bf..b18d89d7ad 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,180 @@ 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)
> > > > +{
> > > > +    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);
> > > > +    }
> > > > +    return (GStrv)g_array_free(values, FALSE);
> > > > +}
> > > > +
> > > > +/*
> > > > + *  
> > > 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)");
> > > > +            continue;
> > > > +        }
> > > > +        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;
> > > > +            GuestDeviceAddressPCI *address;
> > > > +            if (!g_regex_match(device_pci_re, hw_ids[j], 0,  
> > > &match_info)) {  
> > > > +                continue;
> > > > +            }
> > > > +            skip = false;
> > > > +
> > > > +            address = g_new0(GuestDeviceAddressPCI, 1);
> > > > +            vendor_id = g_match_info_fetch(match_info, 1);
> > > > +            device_id = g_match_info_fetch(match_info, 2);
> > > > +            address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> > > > +            address->device_id = g_ascii_strtoull(device_id, NULL, 16);
> > > > +
> > > > +            device->address = g_new0(GuestDeviceAddress, 1);
> > > > +            device->has_address = true;
> > > > +            device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
> > > > +            device->address->u.pci.data = address;
> > > > +
> > > > +            g_match_info_free(match_info);
> > > > +            break;
> > > > +        }
> > > > +        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)");  
> > > > +            continue;
> > > > +        }
> > > > +        device->has_driver_version = true;
> > > > +
> > > > +        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);
> > > > +        device->has_driver_date = true;
> > > > +
> > > > +        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;
> > > > +    }
> > > > +
> > > > +    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..92ed76c419 100644
> > > > --- a/qga/qapi-schema.json
> > > > +++ b/qga/qapi-schema.json
> > > > @@ -1242,3 +1242,54 @@
> > > >  ##
> > > >  { 'command': 'guest-get-osinfo',
> > > >    'returns': 'GuestOSInfo' }
> > > > +
> > > > +##
> > > > +# @GuestDeviceAddressPCI:
> > > > +#
> > > > +# @vendor-id: vendor ID
> > > > +# @device-id: device ID
> > > > +#
> > > > +# Since: 5.0
> > > > +##
> > > > +{ 'struct': 'GuestDeviceAddressPCI',
> > > > +  'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
> > > > +
> > > > +##
> > > > +# @GuestDeviceAddress:
> > > > +#
> > > > +# Address of the device
> > > > +# - @pci: address of PCI device, since: 5.0
> > > > +#
> > > > +# Since: 5.0
> > > > +##
> > > > +{ 'union': 'GuestDeviceAddress',
> > > > +  'data': { 'pci': 'GuestDeviceAddressPCI' } }
> > > > +
> > > > +##
> > > > +# @GuestDeviceInfo:
> > > > +#
> > > > +# @driver-name: name of the associated driver
> > > > +# @driver-date: driver release date in format YYYY-MM-DD
> > > > +# @driver-version: driver version
> > > > +#
> > > > +# Since: 5.0
> > > > +##
> > > > +{ 'struct': 'GuestDeviceInfo',
> > > > +  'data': {
> > > > +      'driver-name': 'str',
> > > > +      '*driver-date': 'str',
> > > > +      '*driver-version': 'str',
> > > > +      '*address': 'GuestDeviceAddress'
> > > > +  } }
> > > > +
> > > > +##
> > > > +# @guest-get-devices:
> > > > +#
> > > > +# Retrieve information about device drivers in Windows guest
> > > > +#
> > > > +# Returns: @GuestDeviceInfo
> > > > +#
> > > > +# Since: 5.0
> > > > +##
> > > > +{ 'command': 'guest-get-devices',
> > > > +  'returns': ['GuestDeviceInfo'] }
> > > > --
> > > > 2.24.1
> > > >  
> > >
> > >
> > > --
> > > Tomáš Golembiovský <tgolembi@redhat.com>
> > >
> > >
> > >  
> > 
> > -- 
> > Marc-André Lureau  
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> 


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



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-07-21 15:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09 12:39 [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices Tomáš Golembiovský
2020-01-09 13:47 ` Marc-André Lureau
2020-02-26 17:01 ` Philippe Mathieu-Daudé
2020-07-21  8:01 ` Tomáš Golembiovský
2020-07-21 11:31   ` Marc-André Lureau
2020-07-21 11:43     ` Daniel P. Berrangé
2020-07-21 15:28       ` Tomáš Golembiovský

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.