[PATCH v2 3/3] qga: add implementation of guest-get-disks for Windows

Tomáš Golembiovský posted 3 patches 5 years, 3 months ago
[PATCH v2 3/3] qga: add implementation of guest-get-disks for Windows
Posted by Tomáš Golembiovský 5 years, 3 months ago
The command lists all the physical disk drives. Unlike for Linux
partitions and virtual volumes are not listed.

Example output:

{
  "return": [
    {
      "name": "\\\\.\\PhysicalDrive0",
      "partition": false,
      "address": {
        "serial": "QM00001",
        "bus-type": "sata",
        ...
      },
      "slaves": []
    }
  ]
}

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 qga/commands-win32.c | 97 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 6 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index e9976a0c46..9ac847a187 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -945,6 +945,91 @@ out:
     return list;
 }
 
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+    GuestDiskInfoList *new = NULL, *ret = NULL;
+    HDEVINFO dev_info;
+    SP_DEVICE_INTERFACE_DATA dev_iface_data;
+    int i;
+
+    dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_DISK, 0, 0,
+        DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
+    if (dev_info == INVALID_HANDLE_VALUE) {
+        error_setg_win32(errp, GetLastError(), "failed to get device tree");
+        return NULL;
+    }
+
+    g_debug("enumerating devices");
+    dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
+    for (i = 0;
+        SetupDiEnumDeviceInterfaces(dev_info, NULL, &GUID_DEVINTERFACE_DISK,
+            i, &dev_iface_data);
+        i++) {
+        GuestDiskAddress *address = NULL;
+        GuestDiskInfo *disk = NULL;
+        Error *local_err = NULL;
+        g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA
+            pdev_iface_detail_data = NULL;
+        STORAGE_DEVICE_NUMBER sdn;
+        HANDLE dev_file;
+        DWORD size = 0;
+
+        g_debug("  getting device path");
+        while (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
+                pdev_iface_detail_data,
+                size, &size,
+                NULL)) {
+            if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
+                pdev_iface_detail_data = g_malloc(size);
+                pdev_iface_detail_data->cbSize =
+                    sizeof(*pdev_iface_detail_data);
+            } else {
+                g_debug("failed to get device interface details");
+                continue;
+            }
+        }
+
+        g_debug("  device: %s", pdev_iface_detail_data->DevicePath);
+        dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
+            FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);
+        if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
+                NULL, 0, &sdn, sizeof(sdn), &size, NULL)) {
+            CloseHandle(dev_file);
+            debug_error("failed to get storage device number");
+            continue;
+        }
+        CloseHandle(dev_file);
+
+        disk = g_new0(GuestDiskInfo, 1);
+        disk->name = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
+            sdn.DeviceNumber);
+
+        g_debug("  number: %lu", sdn.DeviceNumber);
+        address = g_malloc0(sizeof(GuestDiskAddress));
+        address->has_dev = true;
+        address->dev = g_strdup(disk->name);
+        get_single_disk_info(sdn.DeviceNumber, address, &local_err);
+        if (local_err) {
+            g_debug("failed to get disk info: %s",
+                error_get_pretty(local_err));
+            error_free(local_err);
+            qapi_free_GuestDiskAddress(address);
+            address = NULL;
+        } else {
+            disk->address = address;
+            disk->has_address = true;
+        }
+
+        new = g_malloc0(sizeof(GuestDiskInfoList));
+        new->value = disk;
+        new->next = ret;
+        ret = new;
+    }
+
+    SetupDiDestroyDeviceInfoList(dev_info);
+    return ret;
+}
+
 #else
 
 static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
@@ -952,6 +1037,12 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
     return NULL;
 }
 
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
 #endif /* CONFIG_QGA_NTDDSCSI */
 
 static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
@@ -2229,9 +2320,3 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
 
     return info;
 }
-
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
-{
-    error_setg(errp, QERR_UNSUPPORTED);
-    return NULL;
-}
-- 
2.25.0


Re: [PATCH v2 3/3] qga: add implementation of guest-get-disks for Windows
Posted by Philippe Mathieu-Daudé 5 years, 3 months ago
Cc'ing Yonggang Luo for review.

On 9/7/20 11:14 AM, Tomáš Golembiovský wrote:
> The command lists all the physical disk drives. Unlike for Linux
> partitions and virtual volumes are not listed.
> 
> Example output:
> 
> {
>   "return": [
>     {
>       "name": "\\\\.\\PhysicalDrive0",
>       "partition": false,
>       "address": {
>         "serial": "QM00001",
>         "bus-type": "sata",
>         ...
>       },
>       "slaves": []
>     }
>   ]
> }
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  qga/commands-win32.c | 97 +++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 91 insertions(+), 6 deletions(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index e9976a0c46..9ac847a187 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -945,6 +945,91 @@ out:
>      return list;
>  }
>  
> +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> +{
> +    GuestDiskInfoList *new = NULL, *ret = NULL;
> +    HDEVINFO dev_info;
> +    SP_DEVICE_INTERFACE_DATA dev_iface_data;
> +    int i;
> +
> +    dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_DISK, 0, 0,
> +        DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
> +    if (dev_info == INVALID_HANDLE_VALUE) {
> +        error_setg_win32(errp, GetLastError(), "failed to get device tree");
> +        return NULL;
> +    }
> +
> +    g_debug("enumerating devices");
> +    dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
> +    for (i = 0;
> +        SetupDiEnumDeviceInterfaces(dev_info, NULL, &GUID_DEVINTERFACE_DISK,
> +            i, &dev_iface_data);
> +        i++) {
> +        GuestDiskAddress *address = NULL;
> +        GuestDiskInfo *disk = NULL;
> +        Error *local_err = NULL;
> +        g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA
> +            pdev_iface_detail_data = NULL;
> +        STORAGE_DEVICE_NUMBER sdn;
> +        HANDLE dev_file;
> +        DWORD size = 0;
> +
> +        g_debug("  getting device path");
> +        while (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
> +                pdev_iface_detail_data,
> +                size, &size,
> +                NULL)) {
> +            if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
> +                pdev_iface_detail_data = g_malloc(size);
> +                pdev_iface_detail_data->cbSize =
> +                    sizeof(*pdev_iface_detail_data);
> +            } else {
> +                g_debug("failed to get device interface details");
> +                continue;
> +            }
> +        }
> +
> +        g_debug("  device: %s", pdev_iface_detail_data->DevicePath);
> +        dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
> +            FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);
> +        if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
> +                NULL, 0, &sdn, sizeof(sdn), &size, NULL)) {
> +            CloseHandle(dev_file);
> +            debug_error("failed to get storage device number");
> +            continue;
> +        }
> +        CloseHandle(dev_file);
> +
> +        disk = g_new0(GuestDiskInfo, 1);
> +        disk->name = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
> +            sdn.DeviceNumber);
> +
> +        g_debug("  number: %lu", sdn.DeviceNumber);
> +        address = g_malloc0(sizeof(GuestDiskAddress));
> +        address->has_dev = true;
> +        address->dev = g_strdup(disk->name);
> +        get_single_disk_info(sdn.DeviceNumber, address, &local_err);
> +        if (local_err) {
> +            g_debug("failed to get disk info: %s",
> +                error_get_pretty(local_err));
> +            error_free(local_err);
> +            qapi_free_GuestDiskAddress(address);
> +            address = NULL;
> +        } else {
> +            disk->address = address;
> +            disk->has_address = true;
> +        }
> +
> +        new = g_malloc0(sizeof(GuestDiskInfoList));
> +        new->value = disk;
> +        new->next = ret;
> +        ret = new;
> +    }
> +
> +    SetupDiDestroyDeviceInfoList(dev_info);
> +    return ret;
> +}
> +
>  #else
>  
>  static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
> @@ -952,6 +1037,12 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
>      return NULL;
>  }
>  
> +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> +{
> +    error_setg(errp, QERR_UNSUPPORTED);
> +    return NULL;
> +}
> +
>  #endif /* CONFIG_QGA_NTDDSCSI */
>  
>  static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
> @@ -2229,9 +2320,3 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
>  
>      return info;
>  }
> -
> -GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> -{
> -    error_setg(errp, QERR_UNSUPPORTED);
> -    return NULL;
> -}
> 


Re: [PATCH v2 3/3] qga: add implementation of guest-get-disks for Windows
Posted by Marc-André Lureau 5 years, 2 months ago
Hi

On Mon, Sep 7, 2020 at 1:15 PM Tomáš Golembiovský <tgolembi@redhat.com>
wrote:

> The command lists all the physical disk drives. Unlike for Linux
> partitions and virtual volumes are not listed.
>
> Example output:
>
> {
>   "return": [
>     {
>       "name": "\\\\.\\PhysicalDrive0",
>       "partition": false,
>       "address": {
>         "serial": "QM00001",
>         "bus-type": "sata",
>         ...
>       },
>       "slaves": []
>     }
>   ]
> }
>
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  qga/commands-win32.c | 97 +++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 91 insertions(+), 6 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index e9976a0c46..9ac847a187 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -945,6 +945,91 @@ out:
>      return list;
>  }
>
> +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> +{
> +    GuestDiskInfoList *new = NULL, *ret = NULL;
> +    HDEVINFO dev_info;
> +    SP_DEVICE_INTERFACE_DATA dev_iface_data;
> +    int i;
> +
> +    dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_DISK, 0, 0,
> +        DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
> +    if (dev_info == INVALID_HANDLE_VALUE) {
> +        error_setg_win32(errp, GetLastError(), "failed to get device
> tree");
> +        return NULL;
> +    }
> +
> +    g_debug("enumerating devices");
> +    dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
> +    for (i = 0;
> +        SetupDiEnumDeviceInterfaces(dev_info, NULL,
> &GUID_DEVINTERFACE_DISK,
> +            i, &dev_iface_data);
> +        i++) {
> +        GuestDiskAddress *address = NULL;
> +        GuestDiskInfo *disk = NULL;
> +        Error *local_err = NULL;
> +        g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA
> +            pdev_iface_detail_data = NULL;
> +        STORAGE_DEVICE_NUMBER sdn;
> +        HANDLE dev_file;
> +        DWORD size = 0;
> +
> +        g_debug("  getting device path");
> +        while (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
> +                pdev_iface_detail_data,
> +                size, &size,
> +                NULL)) {
> +            if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
> +                pdev_iface_detail_data = g_malloc(size);
>

Since this is called in a loop, you should use g_realloc() to avoid
potential leaks.

+                pdev_iface_detail_data->cbSize =
> +                    sizeof(*pdev_iface_detail_data);
> +            } else {
> +                g_debug("failed to get device interface details");
> +                continue;
> +            }
> +        }
> +
> +        g_debug("  device: %s", pdev_iface_detail_data->DevicePath);
> +        dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
> +            FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);
> +        if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
> +                NULL, 0, &sdn, sizeof(sdn), &size, NULL)) {
> +            CloseHandle(dev_file);
> +            debug_error("failed to get storage device number");
> +            continue;
> +        }
> +        CloseHandle(dev_file);
> +
> +        disk = g_new0(GuestDiskInfo, 1);
> +        disk->name = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
> +            sdn.DeviceNumber);
> +
> +        g_debug("  number: %lu", sdn.DeviceNumber);
> +        address = g_malloc0(sizeof(GuestDiskAddress));
> +        address->has_dev = true;
> +        address->dev = g_strdup(disk->name);
> +        get_single_disk_info(sdn.DeviceNumber, address, &local_err);
> +        if (local_err) {
> +            g_debug("failed to get disk info: %s",
> +                error_get_pretty(local_err));
> +            error_free(local_err);
> +            qapi_free_GuestDiskAddress(address);
> +            address = NULL;
> +        } else {
> +            disk->address = address;
> +            disk->has_address = true;
> +        }
> +
> +        new = g_malloc0(sizeof(GuestDiskInfoList));
> +        new->value = disk;
> +        new->next = ret;
> +        ret = new;
> +    }
> +
> +    SetupDiDestroyDeviceInfoList(dev_info);
> +    return ret;
> +}
> +
>  #else
>
>  static GuestDiskAddressList *build_guest_disk_info(char *guid, Error
> **errp)
> @@ -952,6 +1037,12 @@ static GuestDiskAddressList
> *build_guest_disk_info(char *guid, Error **errp)
>      return NULL;
>  }
>
> +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> +{
> +    error_setg(errp, QERR_UNSUPPORTED);
> +    return NULL;
> +}
> +
>  #endif /* CONFIG_QGA_NTDDSCSI */
>
>  static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
> @@ -2229,9 +2320,3 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
>
>      return info;
>  }
> -
> -GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
> -{
> -    error_setg(errp, QERR_UNSUPPORTED);
> -    return NULL;
> -}
> --
> 2.25.0
>
>
>
The rest looks ok to me.

-- 
Marc-André Lureau