[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO d
|
From: |
Tomáš Golembiovský |
|
Subject: |
Re: [PATCH v9] qga: add command guest-get-devices for reporting VirtIO devices |
|
Date: |
Tue, 21 Jul 2020 17:28:54 +0200 |
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>