Quoting Sameeh Jubran (2018-06-26 10:10:38)
> From: Sameeh Jubran <sjub...@redhat.com>
>
> The call to SetupDiGetDeviceRegistryProperty might fail because the
> value doesn't exist in the registry, in this case we shouldn't exit from
> the loop but instead continue to look for other available values in the
> registry and set this value as unavailable (-1).
>
> Signed-off-by: Sameeh Jubran <sjub...@redhat.com>
The values I'm seeing look off:
win7:
{'execute':'guest-get-fsinfo','arguments':{}}
{"return": [{"name":
"\\\\?\\Volume{2823bc2b-b90f-11e7-9ea5-806e6f6e6963}\\", "total-bytes":
316628992, "mountpoint": "E:\\", "disk": [{"bus-type": "ide", "bus": 0,
"unit": 0, "pci-controller": {"bus": -1, "slot": -1, "domain": -1,
"function": -1}, "target": 0}], "used-bytes": 316628992, "type":
"CDFS"}, {"name":
"\\\\?\\Volume{a1ed8064-3f4a-11e1-972d-806e6f6e6963}\\", "total-bytes":
21367877632, "mountpoint": "C:\\", "disk": [{"bus-type": "scsi", "bus":
0, "unit": 0, "pci-controller": {"bus": -1, "slot": -1, "domain": 0,
"function": 2}, "target": 0}], "used-bytes": 19656269824, "type":
"NTFS"}, {"name":
"\\\\?\\Volume{a1ed8063-3f4a-11e1-972d-806e6f6e6963}\\", "mountpoint":
"System Reserved", "disk": [{"bus-type": "scsi", "bus": 0, "unit": 0,
"pci-controller": {"bus": -1, "slot": -1, "domain": 0, "function": 1},
"target": 0}], "type": "NTFS"}]}
win10:
{'execute':'guest-get-fsinfo','arguments':{}}
{"return": [{"name":
"\\\\?\\Volume{83835b2d-0032-11e6-a84f-806e6f6e6963}\\", "total-bytes":
160755712, "mountpoint": "D:\\", "disk": [{"bus-type": "ide", "bus": 0,
"unit": 0, "pci-controller": {"bus": -1, "slot": -1, "domain": -1,
"function": -1}, "target": 0}], "used-bytes": 160755712, "type":
"CDFS"}, {"name":
"\\\\?\\Volume{2ea839c6-0000-0000-0000-80620c000000}\\", "mountpoint":
"System Reserved", "disk": [{"bus-type": "scsi", "bus": 0, "unit": 0,
"pci-controller": {"bus": -1, "slot": -1, "domain": 0, "function": 3},
"target": 0}], "type": "NTFS"}, {"name":
"\\\\?\\Volume{2ea839c6-0000-0000-0000-501f00000000}\\", "total-bytes":
52665839616, "mountpoint": "C:\\", "disk": [{"bus-type": "scsi", "bus":
0, "unit": 0, "pci-controller": {"bus": -1, "slot": -1, "domain": 0,
"function": 2}, "target": 0}], "used-bytes": 29645811712, "type":
"NTFS"}, {"name":
"\\\\?\\Volume{2ea839c6-0000-0000-0000-100000000000}\\", "mountpoint":
"System Reserved", "disk": [{"bus-type": "scsi", "bus": 0, "unit": 0,
"pci-controller": {"bus": -1, "slot": -1, "domain": 0, "function": 1},
"target": 0}], "type": "NTFS"}]}
If any one of domain/bus/slot/func fail, we should initialize everything
to -1 since we can't partially report a PCI address. The fact that we
get partial success makes me think SPDRP_ADDRESS, SPDRP_UI_NUMBER, etc.
aren't reporting what we expect they should. The documentation seems
a bit nebulous. Also I'm not using multifunction to the non-0 function
values seem off.
Do you know of anyone manually setting CONFIG_QGA_NTDDSCSI in practice?
It's been left disabled in configure (left misnamed as CONFIG_QGA_NTDDDISK
due to some problems that popped up when we tried to enable it that I
don't quite recall, maybe similar issues to what you're seeing). I'm
starting to think it's better to leave this for 3.1 since it's not
technically a supported feature yet and may need some reworking outside
of the issue you were originally addressing.
> ---
> qga/commands-win32.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index c5f1c884e1..55e460dee3 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -505,7 +505,8 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
> **errp)
>
> dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
> for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
> - DWORD addr, bus, slot, func, dev, data, size2;
> + DWORD addr, bus, slot, data, size2;
> + int func, dev;
> while (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
>
> SPDRP_PHYSICAL_DEVICE_OBJECT_NAME,
> &data, (PBYTE)buffer, size,
> @@ -535,21 +536,21 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
> **errp)
> */
> if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
> SPDRP_BUSNUMBER, &data, (PBYTE)&bus, size, NULL)) {
> - break;
> + bus = -1;
> }
>
> /* The function retrieves the device's address. This value will be
> * transformed into device function and number */
> if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
> SPDRP_ADDRESS, &data, (PBYTE)&addr, size, NULL)) {
> - break;
> + addr = -1;
> }
>
> /* This call returns UINumber of DEVICE_CAPABILITIES structure.
> * This number is typically a user-perceived slot number. */
> if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
> SPDRP_UI_NUMBER, &data, (PBYTE)&slot, size, NULL)) {
> - break;
> + slot = -1;
> }
>
> /* SetupApi gives us the same information as driver with
> @@ -559,12 +560,12 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
> **errp)
> * DeviceNumber = (USHORT)(((propertyAddress) >> 16) & 0x0000FFFF);
> * SPDRP_ADDRESS is propertyAddress, so we do the same.*/
>
> - func = addr & 0x0000FFFF;
> - dev = (addr >> 16) & 0x0000FFFF;
> + func = ((int) addr == -1) ? -1 : addr & 0x0000FFFF;
> + dev = ((int) addr == -1) ? -1 : (addr >> 16) & 0x0000FFFF;
> pci->domain = dev;
> - pci->slot = slot;
> + pci->slot = (int) slot;
> pci->function = func;
> - pci->bus = bus;
> + pci->bus = (int) bus;
> break;
> }
>
> --
> 2.13.6
>
