Re: [Qemu-devel] [PATCH v3] QGA: Fix guest-get-fsinfo PCI address collection in Windows
Patchew URL:
https://patchew.org/QEMU/20190128223056.19452-1-mhi...@scalecomputing.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v3] QGA: Fix guest-get-fsinfo PCI address
collection in Windows
Type: series
Message-id: 20190128223056.19452-1-mhi...@scalecomputing.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
6805362916 QGA: Fix guest-get-fsinfo PCI address collection in Windows
=== OUTPUT BEGIN ===
WARNING: Block comments should align the * on each line
#198: FILE: qga/commands-win32.c:592:
+/*
+* CM API used here as opposed to
WARNING: line over 80 characters
#302: FILE: qga/commands-win32.c:661:
+/* There is no need to allocate buffer in the next functions. The
size
WARNING: Block comments use a leading /* on a separate line
#302: FILE: qga/commands-win32.c:661:
+/* There is no need to allocate buffer in the next functions. The
size
WARNING: Block comments should align the * on each line
#303: FILE: qga/commands-win32.c:662:
+/* There is no need to allocate buffer in the next functions. The
size
+* is known and ULONG according to
ERROR: spaces required around that '&' (ctx:VxV)
#308: FILE: qga/commands-win32.c:667:
+ , (PBYTE), size, NULL)) {
^
WARNING: Block comments use a leading /* on a separate line
#326: FILE: qga/commands-win32.c:673:
+/* The function retrieves the device's address. This value will be
WARNING: Block comments use a trailing */ on a separate line
#327: FILE: qga/commands-win32.c:674:
+* transformed into device function and number */
WARNING: Block comments should align the * on each line
#327: FILE: qga/commands-win32.c:674:
+/* The function retrieves the device's address. This value will be
+* transformed into device function and number */
ERROR: spaces required around that '&' (ctx:VxV)
#330: FILE: qga/commands-win32.c:677:
+, (PBYTE), size, NULL)) {
^
WARNING: Block comments use a leading /* on a separate line
#336: FILE: qga/commands-win32.c:683:
+/* This call returns UINumber of DEVICE_CAPABILITIES structure.
WARNING: Block comments use a trailing */ on a separate line
#337: FILE: qga/commands-win32.c:684:
+* This number is typically a user-perceived slot number. */
WARNING: Block comments should align the * on each line
#337: FILE: qga/commands-win32.c:684:
+/* This call returns UINumber of DEVICE_CAPABILITIES structure.
+* This number is typically a user-perceived slot number. */
ERROR: spaces required around that '&' (ctx:VxV)
#340: FILE: qga/commands-win32.c:687:
+, (PBYTE), size, NULL)) {
^
WARNING: Block comments use a leading /* on a separate line
#346: FILE: qga/commands-win32.c:693:
+/* SetupApi gives us the same information as driver with
WARNING: Block comments should align the * on each line
#347: FILE: qga/commands-win32.c:694:
+/* SetupApi gives us the same information as driver with
+* IoGetDeviceProperty. According to Microsoft
WARNING: Block comments use a trailing */ on a separate line
#351: FILE: qga/commands-win32.c:698:
+* SPDRP_ADDRESS is propertyAddress, so we do the same.*/
total: 3 errors, 13 warnings, 407 lines checked
Commit 680536291660 (QGA: Fix guest-get-fsinfo PCI address collection in
Windows) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190128223056.19452-1-mhi...@scalecomputing.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
[Qemu-devel] [PATCH v3] QGA: Fix guest-get-fsinfo PCI address collection in Windows
From: Matt Hines
The Windows QEMU guest agent erroneously tries to collect PCI information
directly from the physical drive. However, windows stores SCSI/IDE information
with the drive and PCI information with the underlying storage controller
This changes get_pci_info to use the physical drive's underlying storage
controller to get PCI information.
* Additionally Fixes incorrect size being passed to DeviceIoControl
when getting volume extents. Can occasionally crash the guest agent
Signed-off-by: Matt Hines
---
configure| 2 +-
qga/commands-win32.c | 305 +--
2 files changed, 199 insertions(+), 108 deletions(-)
diff --git a/configure b/configure
index 5b1d83ea26..46f21c089f 100755
--- a/configure
+++ b/configure
@@ -4694,7 +4694,7 @@ int main(void) {
EOF
if compile_prog "" "" ; then
guest_agent_ntddscsi=yes
-libs_qga="-lsetupapi $libs_qga"
+libs_qga="-lsetupapi -lcfgmgr32 $libs_qga"
fi
fi
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 62e1b51dfe..5f8e797032 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#endif
#include
@@ -491,56 +492,29 @@ static GuestDiskBusType find_bus_type(STORAGE_BUS_TYPE
bus)
return win2qemu[(int)bus];
}
-/* XXX: The following function is BROKEN!
- *
- * It does not work and probably has never worked. When we query for list of
- * disks we get cryptic names like "\Device\001d" instead of
- * "\PhysicalDriveX" or "\HarddiskX". Whether the names can be translated one
- * way or the other for comparison is an open question.
- *
- * When we query volume names (the original version) we are able to match those
- * but then the property queries report error "Invalid function". (duh!)
- */
-
-/*
-DEFINE_GUID(GUID_DEVINTERFACE_VOLUME,
-0x53f5630dL, 0xb6bf, 0x11d0, 0x94, 0xf2,
-0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
-*/
DEFINE_GUID(GUID_DEVINTERFACE_DISK,
0x53f56307L, 0xb6bf, 0x11d0, 0x94, 0xf2,
0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
+DEFINE_GUID(GUID_DEVINTERFACE_STORAGEPORT,
+0x2accfe60L, 0xc130, 0x11d2, 0xb0, 0x82,
+0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
-
-static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
+static GuestPCIAddress *get_pci_info(int number, Error **errp)
{
HDEVINFO dev_info;
SP_DEVINFO_DATA dev_info_data;
-DWORD size = 0;
+SP_DEVICE_INTERFACE_DATA dev_iface_data;
+HANDLE dev_file;
int i;
-char dev_name[MAX_PATH];
-char *buffer = NULL;
GuestPCIAddress *pci = NULL;
-char *name = NULL;
bool partial_pci = false;
+
pci = g_malloc0(sizeof(*pci));
pci->domain = -1;
pci->slot = -1;
pci->function = -1;
pci->bus = -1;
-if (g_str_has_prefix(guid, ".\\") ||
-g_str_has_prefix(guid, "?\\")) {
-name = g_strdup(guid + 4);
-} else {
-name = g_strdup(guid);
-}
-
-if (!QueryDosDevice(name, dev_name, ARRAY_SIZE(dev_name))) {
-error_setg_win32(errp, GetLastError(), "failed to get dos device
name");
-goto out;
-}
-
dev_info = SetupDiGetClassDevs(_DEVINTERFACE_DISK, 0, 0,
DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
if (dev_info == INVALID_HANDLE_VALUE) {
@@ -550,90 +524,208 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
g_debug("enumerating devices");
dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
+dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, _info_data); i++) {
-DWORD addr, bus, slot, data, size2;
-int func, dev;
-while (!SetupDiGetDeviceRegistryProperty(dev_info, _info_data,
-SPDRP_PHYSICAL_DEVICE_OBJECT_NAME,
-, (PBYTE)buffer, size,
-)) {
-size = MAX(size, size2);
-if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
-g_free(buffer);
-/* Double the size to avoid problems on
- * W2k MBCS systems per KB 888609.
- * https://support.microsoft.com/en-us/kb/259695 */
-buffer = g_malloc(size * 2);
-} else {
+PSP_DEVICE_INTERFACE_DETAIL_DATA pdev_iface_detail_data = NULL;
+STORAGE_DEVICE_NUMBER sdn;
+char *parent_dev_id = NULL;
+HDEVINFO parent_dev_info;
+SP_DEVINFO_DATA parent_dev_info_data;
+DWORD j;
+DWORD size = 0;
+
+g_debug("getting device path");
+if (SetupDiEnumDeviceInterfaces(dev_info, _info_data,
+_DEVINTERFACE_DISK, 0,
+_iface_data)) {
+while (!SetupDiGetDeviceInterfaceDetail(dev_info, _iface_data,
+
