All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, "Tomáš Golembiovský" <tgolembi@redhat.com>
Subject: [Qemu-devel] [PULL v2 14/24] qga-win: handle multi-disk volumes
Date: Tue, 30 Oct 2018 20:38:11 -0500	[thread overview]
Message-ID: <20181031013821.24023-15-mdroth@linux.vnet.ibm.com> (raw)
In-Reply-To: <20181031013821.24023-1-mdroth@linux.vnet.ibm.com>

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

Probe the volume for disk extents and return list of all disks.
Originally only first disk of composite volume was returned.

Note that the patch changes get_pci_info() from one state of brokenness
into a different state of brokenness. In other words it still does not do
what it's supposed to do (see comment in code). If anyone knows how to
fix it, please step in.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-win32.c | 126 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 108 insertions(+), 18 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 21a88d31e1..90432bbfce 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -491,9 +491,26 @@ 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\0000001d" 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);
+
 
 static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
 {
@@ -517,7 +534,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
         goto out;
     }
 
-    dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_VOLUME, 0, 0,
+    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 devices tree");
@@ -672,20 +689,20 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
 {
     SCSI_ADDRESS addr, *scsi_ad;
     DWORD len;
-    HANDLE vol_h;
+    HANDLE disk_h;
     Error *local_err = NULL;
 
     scsi_ad = &addr;
 
     g_debug("getting disk info for: %s", name);
-    vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+    disk_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
                        0, NULL);
-    if (vol_h == INVALID_HANDLE_VALUE) {
-        error_setg_win32(errp, GetLastError(), "failed to open volume");
-        goto err;
+    if (disk_h == INVALID_HANDLE_VALUE) {
+        error_setg_win32(errp, GetLastError(), "failed to open disk");
+        return;
     }
 
-    get_disk_properties(vol_h, disk, &local_err);
+    get_disk_properties(disk_h, disk, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto err_close;
@@ -714,7 +731,7 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
          * according to Microsoft docs
          * https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx */
         g_debug("getting pci-controller info");
-        if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
+        if (DeviceIoControl(disk_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
                             sizeof(SCSI_ADDRESS), &len, NULL)) {
             disk->unit = addr.Lun;
             disk->target = addr.TargetId;
@@ -725,8 +742,7 @@ static void get_single_disk_info(char *name, GuestDiskAddress *disk,
     }
 
 err_close:
-    CloseHandle(vol_h);
-err:
+    CloseHandle(disk_h);
     return;
 }
 
@@ -738,6 +754,10 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
     Error *local_err = NULL;
     GuestDiskAddressList *list = NULL, *cur_item = NULL;
     GuestDiskAddress *disk = NULL;
+    int i;
+    HANDLE vol_h;
+    DWORD size;
+    PVOLUME_DISK_EXTENTS extents = NULL;
 
     /* strip final backslash */
     char *name = g_strdup(guid);
@@ -745,20 +765,90 @@ static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
         name[strlen(name) - 1] = 0;
     }
 
-    disk = g_malloc0(sizeof(GuestDiskAddress));
-    get_single_disk_info(name, disk, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    g_debug("opening %s", name);
+    vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+                       0, NULL);
+    if (vol_h == INVALID_HANDLE_VALUE) {
+        error_setg_win32(errp, GetLastError(), "failed to open volume");
         goto out;
     }
 
-    cur_item = g_malloc0(sizeof(*list));
-    cur_item->value = disk;
-    disk = NULL;
-    list = cur_item;
+    /* Get list of extents */
+    g_debug("getting disk extents");
+    size = sizeof(VOLUME_DISK_EXTENTS);
+    extents = g_malloc0(size);
+    if (!DeviceIoControl(vol_h, IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS, NULL,
+                         0, extents, size, NULL, NULL)) {
+        DWORD last_err = GetLastError();
+        if (last_err == ERROR_MORE_DATA) {
+            /* Try once more with big enough buffer */
+            size = sizeof(VOLUME_DISK_EXTENTS)
+                + extents->NumberOfDiskExtents*sizeof(DISK_EXTENT);
+            g_free(extents);
+            extents = g_malloc0(size);
+            if (!DeviceIoControl(
+                    vol_h, IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS, NULL,
+                    0, extents, size, NULL, NULL)) {
+                error_setg_win32(errp, GetLastError(),
+                    "failed to get disk extents");
+                return NULL;
+            }
+        } else if (last_err == ERROR_INVALID_FUNCTION) {
+            /* Possibly CD-ROM or a shared drive. Try to pass the volume */
+            g_debug("volume not on disk");
+            disk = g_malloc0(sizeof(GuestDiskAddress));
+            get_single_disk_info(name, disk, &local_err);
+            if (local_err) {
+                g_debug("failed to get disk info, ignoring error: %s",
+                    error_get_pretty(local_err));
+                error_free(local_err);
+                goto out;
+            }
+            list = g_malloc0(sizeof(*list));
+            list->value = disk;
+            disk = NULL;
+            list->next = NULL;
+            goto out;
+        } else {
+            error_setg_win32(errp, GetLastError(),
+                "failed to get disk extents");
+            goto out;
+        }
+    }
+    g_debug("Number of extents: %lu", extents->NumberOfDiskExtents);
+
+    /* Go through each extent */
+    for (i = 0; i < extents->NumberOfDiskExtents; i++) {
+        char *disk_name = NULL;
+        disk = g_malloc0(sizeof(GuestDiskAddress));
+
+        /* Disk numbers directly correspond to numbers used in UNCs
+         *
+         * See documentation for DISK_EXTENT:
+         * https://docs.microsoft.com/en-us/windows/desktop/api/winioctl/ns-winioctl-_disk_extent
+         *
+         * See also Naming Files, Paths and Namespaces:
+         * https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#win32-device-namespaces
+         */
+        disk_name = g_strdup_printf("\\\\.\\PhysicalDrive%lu",
+            extents->Extents[i].DiskNumber);
+        get_single_disk_info(disk_name, disk, &local_err);
+        g_free(disk_name);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            goto out;
+        }
+        cur_item = g_malloc0(sizeof(*list));
+        cur_item->value = disk;
+        disk = NULL;
+        cur_item->next = list;
+        list = cur_item;
+    }
+
 
 out:
     qapi_free_GuestDiskAddress(disk);
+    g_free(extents);
     g_free(name);
 
     return list;
-- 
2.17.1

  parent reply	other threads:[~2018-10-31  1:40 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-31  1:37 [Qemu-devel] [PULL v2 00/24] qemu-ga patch queue for soft-freeze Michael Roth
2018-10-31  1:37 ` [Qemu-devel] [PULL v2 01/24] qga: Support Unicode paths in guest-file-open on win32 Michael Roth
2018-10-31  1:37 ` [Qemu-devel] [PULL v2 02/24] qga-win: add support for qmp_guest_fsfreeze_freeze_list Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 03/24] qga: ignore non present cpus when handling qmp_guest_get_vcpus() Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 04/24] configure: add test for libudev Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 05/24] qga: linux: report disk serial number Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 06/24] qga: linux: return disk device in guest-get-fsinfo Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 07/24] qga-win: prevent crash when executing fsinfo command Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 08/24] qga-win: fsinfo: pci-info: allow partial info Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 09/24] build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 10/24] qga-win: add debugging information Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 11/24] qga-win: refactor disk properties (bus) Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 12/24] qga-win: report disk serial number Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 13/24] qga-win: refactor disk info Michael Roth
2018-10-31  1:38 ` Michael Roth [this message]
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 15/24] qga-win: return disk device in guest-get-fsinfo Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 16/24] qga-win: demystify namespace stripping Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 17/24] qga: fix an off-by-one issue Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 18/24] qga: group agent init/cleanup init separate routines Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 19/24] qga: hang GAConfig/socket_activation off of GAState global Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 20/24] qga: move w32 service handling out of run_agent() Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 21/24] qga: add --retry-path option for re-initializing channel on failure Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 22/24] qga-win: install service with --retry-path set by default Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 23/24] qga-win: report specific error when failing to open channel Michael Roth
2018-10-31  1:38 ` [Qemu-devel] [PULL v2 24/24] qga-win: changing --retry-path option behavior Michael Roth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181031013821.24023-15-mdroth@linux.vnet.ibm.com \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=tgolembi@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.