* [PATCH] Cygwin: Add /dev/disk/by-id symlinks
@ 2023-09-25 11:57 Christian Franke
2023-10-03 12:39 ` Christian Franke
0 siblings, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-09-25 11:57 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 964 bytes --]
This is a first attempt to partly emulate the Linux directory
/dev/disk/by-id. Useful to make sure the correct device is accessed in
conjunction with dd, ddrescue, fdisk, ....
The additional '*-partN' links to partitions are not yet included.
This only works properly if Win32 path '\\.\PhysicalDriveN' is always
trivially mapped to NT path '\Device\HarddiskN\Partition0'.
IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all
drivers. With stornvme.sys, it fails with permission denied. Perhaps
other permission bits are required for NtOpenFile(). Thanks for any info
regarding this.
Note that the BusType information is often not accurate. For example
drives behind Intel RST drivers may always be listed as
"raid-PRODUCT_SERIAL" even if not part of a RAID volume.
The changes for the generated file devices.cc are not included in the patch.
--
Regards,
Christian
[-- Attachment #2: 0001-Cygwin-Add-dev-disk-by-id-symlinks.patch --]
[-- Type: text/plain, Size: 17629 bytes --]
From 8a49ae067cfd318746e6fe332b8775011658d780 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Mon, 25 Sep 2023 13:24:40 +0200
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
The new directory '/dev/disk/by-id' provides symlinks for each
disk based on information from IOCTL_STORAGE_QUERY_PROPERTY:
'BUSTYPE_[VENDOR_]PRODUCT_SERIAL' -> '../../sdX'
Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
winsup/cygwin/Makefile.am | 1 +
winsup/cygwin/devices.in | 4 +
winsup/cygwin/dtable.cc | 3 +
winsup/cygwin/fhandler/dev_disk.cc | 389 ++++++++++++++++++++++++
winsup/cygwin/local_includes/devices.h | 6 +-
winsup/cygwin/local_includes/fhandler.h | 45 +++
winsup/cygwin/mount.cc | 10 +
7 files changed, 457 insertions(+), 1 deletion(-)
create mode 100644 winsup/cygwin/fhandler/dev_disk.cc
diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+ fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
const _device dev_fs_storage =
{"", {FH_FS}, "", exists};
+const _device dev_dev_disk_storage =
+ {"", {FH_DEV_DISK}, "", exists};
+
const _device dev_proc_storage =
{"", {FH_PROC}, "", exists};
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace. */
%%
"/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
"/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
"/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", exists_pty, S_IFCHR, =ptys_dev
":ptym%(0-127)d", BRACK(FHDEV(DEV_PTYM_MAJOR, {$1})), "/dev/ptym{$1}", exists_internal, S_IFCHR, =ptym_dev
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 21d525389..9508f3e0b 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -585,6 +585,9 @@ fh_alloc (path_conv& pc)
case FH_DEV:
fh = cnew (fhandler_dev);
break;
+ case FH_DEV_DISK:
+ fh = cnew (fhandler_dev_disk);
+ break;
case FH_DEV_FD:
fh = cnew (fhandler_dev_fd);
break;
diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
new file mode 100644
index 000000000..14b0cdf36
--- /dev/null
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -0,0 +1,389 @@
+/* fhandler/dev_disk.cc: fhandler for the /dev/disk/by-id/... symlinks.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license. Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "winsup.h"
+#include "path.h"
+#include "fhandler.h"
+#include <winioctl.h>
+
+/* Replace non-printing and unexpected characters, remove trailing spaces,
+ return remaining string length. */
+static int
+sanitize_id_string (char *s)
+{
+ int lastspace = -1, i;
+ for (i = 0; s[i]; i++)
+ {
+ char c = s[i];
+ if (c != ' ')
+ lastspace = -1;
+ else if (lastspace < 0)
+ lastspace = i;
+ if (('0' <= c && c <= '9') || c == '.' || c == '-'
+ || ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'))
+ continue;
+ s[i] = '_';
+ }
+ if (lastspace >= 0)
+ s[(i = lastspace)] = '\0';
+ return i;
+}
+
+/* Get ID string for drive number. */
+static bool
+get_drive_id_name (u_int8_t drive, char (& name)[NAME_MAX + 1])
+{
+ /* Using CreateFile () because with NtOpenFile (., READ_CONTROL, ...) the
+ IOCTL_STORAGE_QUERY_PROPERTY call fails with ERROR_ACCESS_DENIED for
+ drives behind some drivers (nvmestor.sys). */
+ char path[64];
+ __small_sprintf (path, "\\\\.\\PhysicalDrive%d", drive);
+ HANDLE h = CreateFileA (path, 0, FILE_SHARE_READ | FILE_SHARE_WRITE,
+ nullptr, OPEN_EXISTING, 0, nullptr);
+ if (h == INVALID_HANDLE_VALUE)
+ return false;
+
+ STORAGE_PROPERTY_QUERY query =
+ { StorageDeviceProperty, PropertyStandardQuery, { 0 } };
+ union {
+ char raw[1024];
+ STORAGE_DEVICE_DESCRIPTOR desc;
+ } desc_data;
+ DWORD bytes_read;
+ BOOL ok = DeviceIoControl (h, IOCTL_STORAGE_QUERY_PROPERTY,
+ &query, sizeof (query),
+ &desc_data, sizeof (desc_data),
+ &bytes_read, nullptr);
+ CloseHandle (h);
+ if (!ok)
+ {
+ debug_printf ("ignoring drive %d: IOCTL_STORAGE_QUERY_PROPERTY failed: %u",
+ drive, GetLastError ());
+ return false;
+ }
+
+ /* Ignore drive if information is missing, too short or too long. */
+ int vendor_len = 0, product_len = 0, serial_len = 0;
+ if (desc_data.desc.VendorIdOffset)
+ vendor_len = sanitize_id_string (desc_data.raw + desc_data.desc.VendorIdOffset);
+ if (desc_data.desc.ProductIdOffset)
+ product_len = sanitize_id_string (desc_data.raw + desc_data.desc.ProductIdOffset);
+ if (desc_data.desc.SerialNumberOffset)
+ serial_len = sanitize_id_string (desc_data.raw + desc_data.desc.SerialNumberOffset);
+
+ bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
+ && vendor_len + 1 + product_len + 1 + serial_len < (int) sizeof (name));
+ debug_printf ("drive %d: '%s' '%s' '%s'%s", drive,
+ (vendor_len ? desc_data.raw + desc_data.desc.VendorIdOffset : ""),
+ (product_len ? desc_data.raw + desc_data.desc.ProductIdOffset : ""),
+ (serial_len ? desc_data.raw + desc_data.desc.SerialNumberOffset : ""),
+ (valid ? "" : " (ignored)"));
+ if (!valid)
+ return false;
+
+ /* Translate bus types. */
+ const char *bus;
+ switch (desc_data.desc.BusType)
+ {
+ case BusTypeAta: bus = "ata-"; break;
+ case BusTypeFibre: bus = "fibre-"; break;
+ case BusTypeNvme: bus = "nvme-"; break;
+ case BusTypeRAID: bus = "raid-"; break;
+ case BusTypeSas: bus = "sas-"; break;
+ case BusTypeSata: bus = "sata-"; break;
+ case BusTypeScsi: bus = "scsi-"; break;
+ case BusTypeUsb: bus = "usb-"; break;
+ case BusTypeVirtual: bus = "virtual-"; break;
+ default: bus = nullptr; break;
+ }
+ if (bus)
+ strcpy (name, bus);
+ else
+ __small_sprintf (name, "bustype0x%02x-", desc_data.desc.BusType);
+
+ /* Create "BUSTYPE-[VENDOR_]PRODUCT_SERIAL" string. */
+ if (vendor_len)
+ strcat (name, desc_data.raw + desc_data.desc.VendorIdOffset);
+ if (product_len)
+ {
+ if (vendor_len)
+ strcat (name, "_");
+ strcat (name, desc_data.raw + desc_data.desc.ProductIdOffset);
+ }
+ strcat (name, "_");
+ strcat (name, desc_data.raw + desc_data.desc.SerialNumberOffset);
+ return true;
+}
+
+struct by_id_entry
+{
+ char name[NAME_MAX + 1];
+ u_int8_t drive;
+};
+
+static int
+by_id_compare_name (const void *a, const void *b)
+{
+ const by_id_entry *ap = reinterpret_cast<const by_id_entry *>(a);
+ const by_id_entry *bp = reinterpret_cast<const by_id_entry *>(b);
+ return strcmp (ap->name, bp->name);
+}
+
+/* Create sorted name -> drive mapping table. Must be freed by caller. */
+static by_id_entry *
+get_by_id_table (int * ret_size = nullptr)
+{
+ /* Scan 128 drives like fhandler_dev::readdir () does. */
+ by_id_entry * table = nullptr;
+ int alloc_size = 0, size = 0;
+ for (u_int8_t drive = 0; drive < 128; drive++)
+ {
+ if (alloc_size <= size)
+ {
+ alloc_size = size + 8;
+ by_id_entry * table2 = reinterpret_cast<by_id_entry *>(
+ realloc (table, alloc_size * sizeof (*table))
+ );
+ if (!table2)
+ {
+ free (table);
+ return nullptr;
+ }
+ table = table2;
+ }
+
+ if (!get_drive_id_name (drive, table[size].name))
+ continue;
+ table[size].drive = drive;
+ size++;
+ }
+
+ /* Sort by name and remove duplicates. */
+ qsort (table, size, sizeof (*table), by_id_compare_name);
+ for (int i = 0; i < size; i++)
+ {
+ int j = i + 1;
+ while (j < size && !strcmp (table[i].name, table[j].name))
+ j++;
+ if (j == i + 1)
+ continue;
+ /* Duplicate(s) found, remove all entries with this name. */
+ debug_printf ("removing duplicates %d-%d: '%s'", i, j - 1, table[i].name);
+ if (j < size)
+ memmove (table + i, table + j, (size - j) * sizeof (*table));
+ size -= j - i;
+ i--;
+ }
+
+ /* Mark end of table for readdir (). */
+ table[size].name[0] = '\0';
+
+ if (ret_size)
+ *ret_size = size;
+ debug_printf ("size: %d\n", size);
+ return table;
+}
+
+const char dev_disk[] = "/dev/disk";
+const size_t dev_disk_len = sizeof (dev_disk) - 1;
+
+fhandler_dev_disk::fhandler_dev_disk ():
+ fhandler_virtual (),
+ loc (unknown_loc),
+ drive_from_id (-1)
+{
+}
+
+void
+fhandler_dev_disk::init_dev_disk ()
+{
+ if (loc != unknown_loc)
+ return;
+
+ static const char by_id[] = "/by-id";
+ const size_t by_id_len = sizeof(by_id) - 1;
+
+ /* Determine location. */
+ const char *path = get_name ();
+ if (!path_prefix_p (dev_disk, path, dev_disk_len, false))
+ loc = invalid_loc; // should not happen
+ else if (!path[dev_disk_len])
+ loc = disk_dir; // "/dev/disk"
+ else if (!path_prefix_p (by_id, path + dev_disk_len, by_id_len, false))
+ loc = invalid_loc; // "/dev/disk/invalid"
+ else if (!path[dev_disk_len + by_id_len])
+ loc = by_id_dir; // "/dev/disk/by-id"
+ else if (strchr (path + dev_disk_len + by_id_len + 1, '/'))
+ loc = invalid_loc; // "/dev/disk/by-id/dir/invalid"
+ else
+ loc = by_id_link; // possible "/dev/disk/by-id/LINK"
+ debug_printf ("'%s': loc %d", path, (int)loc);
+
+ /* Done if "/dev/disk", "/dev/disk/by_id" or invalid. */
+ if (loc != by_id_link)
+ return;
+
+ /* Check whether "/dev/disk/by_id/LINK" exists. */
+ int tabsize;
+ by_id_entry *table = get_by_id_table (&tabsize);
+ if (!table)
+ {
+ loc = invalid_loc;
+ return;
+ }
+
+ by_id_entry key;
+ strcpy (key.name, path + dev_disk_len + by_id_len + 1);
+ const void *found = bsearch (&key, table, tabsize, sizeof (*table),
+ by_id_compare_name);
+ if (found)
+ /* Preserve drive number for fillbuf (). */
+ drive_from_id = reinterpret_cast<const by_id_entry *>(found)->drive;
+ else
+ loc = invalid_loc;
+ free (table);
+}
+
+virtual_ftype_t
+fhandler_dev_disk::exists ()
+{
+ debug_printf ("exists (%s)", get_name ());
+ ensure_inited ();
+ switch (loc)
+ {
+ case disk_dir:
+ case by_id_dir:
+ return virt_directory;
+ case by_id_link:
+ return virt_symlink;
+ default:
+ return virt_none;
+ }
+}
+
+int
+fhandler_dev_disk::fstat (struct stat *buf)
+{
+ debug_printf ("fstat (%s)", get_name ());
+ ensure_inited ();
+ if (loc == invalid_loc)
+ {
+ set_errno (ENOENT);
+ return -1;
+ }
+
+ fhandler_base::fstat (buf);
+ buf->st_mode = (loc == by_id_link ? S_IFLNK | S_IWUSR | S_IWGRP | S_IWOTH
+ : S_IFDIR) | STD_RBITS | STD_XBITS;
+ buf->st_ino = get_ino ();
+ return 0;
+}
+
+DIR *
+fhandler_dev_disk::opendir (int fd)
+{
+ ensure_inited ();
+ if (!(loc == disk_dir || loc == by_id_dir))
+ {
+ set_errno (ENOTDIR);
+ return nullptr;
+ }
+
+ by_id_entry *table;
+ if (loc == by_id_dir)
+ {
+ table = get_by_id_table ();
+ if (!table)
+ {
+ set_errno (ENOMEM);
+ return nullptr;
+ }
+ }
+ else
+ table = nullptr;
+
+ DIR *dir = fhandler_virtual::opendir (fd);
+ if (!dir)
+ {
+ free (table);
+ return nullptr;
+ }
+ dir->__flags = dirent_saw_dot | dirent_saw_dot_dot;
+ dir->__handle = table;
+ return dir;
+}
+
+int
+fhandler_dev_disk::closedir (DIR *dir)
+{
+ free (dir->__handle);
+ return fhandler_virtual::closedir (dir);
+}
+
+int
+fhandler_dev_disk::readdir (DIR *dir, dirent *de)
+{
+ int res;
+ if (dir->__d_position <= 1)
+ {
+ de->d_name[0] = '.';
+ de->d_name[1] = (dir->__d_position ? '.' : '\0');
+ de->d_name[2] = '\0';
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == disk_dir && dir->__d_position == 2)
+ {
+ strcpy (de->d_name, "by-id");
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == by_id_dir && dir->__handle)
+ {
+ const by_id_entry *table = reinterpret_cast<const by_id_entry *>(dir->__handle);
+ const char *name = table[dir->__d_position - 2].name;
+ if (name[0])
+ {
+ strcpy (de->d_name, name);
+ de->d_type = DT_LNK;
+ dir->__d_position++;
+ res = 0;
+ }
+ else
+ res = ENMFILE;
+ }
+ else
+ res = ENMFILE;
+
+ syscall_printf ("%d = readdir(%p, %p) (%s)", res, dir, de,
+ (!res ? de->d_name : ""));
+ return res;
+}
+
+bool
+fhandler_dev_disk::fill_filebuf ()
+{
+ debug_printf ("fill_filebuf (%s)", get_name ());
+ ensure_inited ();
+ if (!(loc == by_id_link && drive_from_id >= 0))
+ return false;
+
+ char buf[32];
+ if (drive_from_id + 'a' <= 'z')
+ __small_sprintf (buf, "../../sd%c", drive_from_id + 'a');
+ else
+ __small_sprintf (buf, "../../sd%c%c",
+ drive_from_id / ('z' - 'a' + 1) - 1 + 'a',
+ drive_from_id % ('z' - 'a' + 1) + 'a');
+
+ free (filebuf);
+ filebuf = cstrdup (buf);
+ return true;
+}
diff --git a/winsup/cygwin/local_includes/devices.h b/winsup/cygwin/local_includes/devices.h
index 10035263d..1e035f9d6 100644
--- a/winsup/cygwin/local_includes/devices.h
+++ b/winsup/cygwin/local_includes/devices.h
@@ -71,6 +71,7 @@ enum fh_devices
FH_DEV = FHDEV (DEV_VIRTFS_MAJOR, 193),
FH_CYGDRIVE= FHDEV (DEV_VIRTFS_MAJOR, 192),
FH_DEV_FD = FHDEV (DEV_VIRTFS_MAJOR, 191),
+ FH_DEV_DISK= FHDEV (DEV_VIRTFS_MAJOR, 190),
FH_SIGNALFD= FHDEV (DEV_VIRTFS_MAJOR, 13),
FH_TIMERFD = FHDEV (DEV_VIRTFS_MAJOR, 14),
@@ -415,6 +416,8 @@ extern const _device dev_piper_storage;
#define piper_dev ((device *) &dev_piper_storage)
extern const _device dev_pipew_storage;
#define pipew_dev ((device *) &dev_pipew_storage)
+extern const _device dev_dev_disk_storage;
+#define dev_disk_dev ((device *) &dev_dev_disk_storage)
extern const _device dev_proc_storage;
#define proc_dev ((device *) &dev_proc_storage)
extern const _device dev_dev_storage;
@@ -439,7 +442,8 @@ extern const _device dev_fs_storage;
#define isprocsys_dev(devn) (devn == FH_PROCSYS)
#define isvirtual_dev(devn) \
- (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE || devn == FH_DEV_FD)
+ (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE \
+ || devn == FH_DEV_FD || devn == FH_DEV_DISK)
#define iscons_dev(n) \
((device::major ((dev_t) (n)) == DEV_CONS_MAJOR) \
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index 212c22344..486b5f49d 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -40,6 +40,8 @@ details. */
extern const char *windows_device_names[];
extern struct __cygwin_perfile *perfile_table;
#define __fmode (*(user_data->fmode_ptr))
+extern const char dev_disk[];
+extern const size_t dev_disk_len;
extern const char proc[];
extern const size_t proc_len;
extern const char procsys[];
@@ -3190,6 +3192,48 @@ class fhandler_procnet: public fhandler_proc
}
};
+class fhandler_dev_disk: public fhandler_virtual
+{
+ enum dev_disk_location {
+ unknown_loc, invalid_loc, disk_dir, by_id_dir, by_id_link
+ };
+ dev_disk_location loc;
+
+ void init_dev_disk ();
+ void ensure_inited ()
+ {
+ if (loc == unknown_loc)
+ init_dev_disk ();
+ }
+
+ int drive_from_id;
+
+ public:
+ fhandler_dev_disk ();
+ fhandler_dev_disk (void *) {}
+ virtual_ftype_t exists();
+ DIR *opendir (int fd);
+ int closedir (DIR *);
+ int readdir (DIR *, dirent *);
+ int fstat (struct stat *buf);
+ bool fill_filebuf ();
+
+ void copy_from (fhandler_base *x)
+ {
+ pc.free_strings ();
+ *this = *reinterpret_cast<fhandler_dev_disk *> (x);
+ _copy_from_reset_helper ();
+ }
+
+ fhandler_dev_disk *clone (cygheap_types malloc_type = HEAP_FHANDLER)
+ {
+ void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_dev_disk));
+ fhandler_dev_disk *fh = new (ptr) fhandler_dev_disk (ptr);
+ fh->copy_from (this);
+ return fh;
+ }
+};
+
class fhandler_dev_fd: public fhandler_virtual
{
public:
@@ -3416,6 +3460,7 @@ typedef union
char __dev_raw[sizeof (fhandler_dev_raw)];
char __dev_tape[sizeof (fhandler_dev_tape)];
char __dev_zero[sizeof (fhandler_dev_zero)];
+ char __dev_disk[sizeof (fhandler_dev_disk)];
char __dev_fd[sizeof (fhandler_dev_fd)];
char __disk_file[sizeof (fhandler_disk_file)];
char __fifo[sizeof (fhandler_fifo)];
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index 36ab042a7..13ace6250 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -35,6 +35,9 @@ details. */
(path[mount_table->cygdrive_len + 1] == '/' || \
!path[mount_table->cygdrive_len + 1]))
+#define isdev_disk(path) \
+ (path_prefix_p (dev_disk, (path), dev_disk_len, false))
+
#define isproc(path) \
(path_prefix_p (proc, (path), proc_len, false))
@@ -685,6 +688,13 @@ mount_info::conv_to_win32_path (const char *src_path, char *dst, device& dev,
/* Go through chroot check */
goto out;
}
+ if (isdev_disk (src_path))
+ {
+ dev = *dev_disk_dev;
+ *flags = 0;
+ strcpy (dst, src_path);
+ goto out;
+ }
if (isproc (src_path))
{
dev = *proc_dev;
--
2.39.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-09-25 11:57 [PATCH] Cygwin: Add /dev/disk/by-id symlinks Christian Franke
@ 2023-10-03 12:39 ` Christian Franke
2023-11-03 9:55 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-10-03 12:39 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 1767 bytes --]
Christian Franke wrote:
> This is a first attempt to partly emulate the Linux directory
> /dev/disk/by-id. Useful to make sure the correct device is accessed in
> conjunction with dd, ddrescue, fdisk, ....
Attached is the second attempt.
> The additional '*-partN' links to partitions are not yet included.
These are now included.
> This only works properly if Win32 path '\\.\PhysicalDriveN' is always
> trivially mapped to NT path '\Device\HarddiskN\Partition0'.
> IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
> READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with
> all drivers. With stornvme.sys, it fails with permission denied.
> Perhaps other permission bits are required for NtOpenFile(). Thanks
> for any info regarding this.
According to NtQueryObject(., ObjectBasicInformation, ...), using
NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets
GrantedAccess to 0x001200a0
(FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE). For some
unknown reason, NVMe drives behind stornvme.sys additionally require
SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless bug
in the access check somewhere in the NVMe stack.
The disk scanning from the first patch has been reworked based on code
borrowed from proc.cc:format_proc_partitions(). For the longer term, it
may make sense to provide one flexible scanning function for /dev/sdXN,
/proc/partitions and /proc/disk/by-id.
> Note that the BusType information is often not accurate. For example
> drives behind Intel RST drivers may always be listed as
> "raid-PRODUCT_SERIAL" even if not part of a RAID volume.
>
> The changes for the generated file devices.cc are not included in the
> patch.
--
Regards,
Christian
[-- Attachment #2: 0001-Cygwin-Add-dev-disk-by-id-symlinks.patch --]
[-- Type: text/plain, Size: 23706 bytes --]
From ba1990a43b7585f88a9369f5db0bdc56d6e913bf Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Tue, 3 Oct 2023 14:15:12 +0200
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
The new directory '/dev/disk/by-id' provides symlinks for each
disk and its partitions:
'BUSTYPE-[VENDOR_]PRODUCT_SERIAL[-partN]' -> '../../sdX[N]'.
IOCTL_STORAGE_QUERY_PROPERTY is used to fetch bus type, vendor,
product and serial number information. Administrator privileges
are not required.
Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
winsup/cygwin/Makefile.am | 1 +
winsup/cygwin/devices.in | 4 +
winsup/cygwin/dtable.cc | 3 +
winsup/cygwin/fhandler/dev_disk.cc | 589 ++++++++++++++++++++++++
winsup/cygwin/local_includes/devices.h | 6 +-
winsup/cygwin/local_includes/fhandler.h | 47 ++
winsup/cygwin/mount.cc | 10 +
7 files changed, 659 insertions(+), 1 deletion(-)
create mode 100644 winsup/cygwin/fhandler/dev_disk.cc
diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+ fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
const _device dev_fs_storage =
{"", {FH_FS}, "", exists};
+const _device dev_dev_disk_storage =
+ {"", {FH_DEV_DISK}, "", exists};
+
const _device dev_proc_storage =
{"", {FH_PROC}, "", exists};
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace. */
%%
"/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
"/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
"/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", exists_pty, S_IFCHR, =ptys_dev
":ptym%(0-127)d", BRACK(FHDEV(DEV_PTYM_MAJOR, {$1})), "/dev/ptym{$1}", exists_internal, S_IFCHR, =ptym_dev
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 21d525389..9508f3e0b 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -585,6 +585,9 @@ fh_alloc (path_conv& pc)
case FH_DEV:
fh = cnew (fhandler_dev);
break;
+ case FH_DEV_DISK:
+ fh = cnew (fhandler_dev_disk);
+ break;
case FH_DEV_FD:
fh = cnew (fhandler_dev_fd);
break;
diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
new file mode 100644
index 000000000..caca57d63
--- /dev/null
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -0,0 +1,589 @@
+/* fhandler/dev_disk.cc: fhandler for the /dev/disk/by-id/... symlinks.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license. Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "winsup.h"
+#include "path.h"
+#include "fhandler.h"
+#include "tls_pbuf.h"
+#include <wctype.h>
+#include <winioctl.h>
+
+/* Replace non-printing and unexpected characters, remove trailing spaces,
+ return remaining string length. */
+static int
+sanitize_id_string (char *s)
+{
+ int lastspace = -1, i;
+ for (i = 0; s[i]; i++)
+ {
+ char c = s[i];
+ if (c != ' ')
+ lastspace = -1;
+ else if (lastspace < 0)
+ lastspace = i;
+ if (('0' <= c && c <= '9') || c == '.' || c == '-'
+ || ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'))
+ continue;
+ s[i] = '_';
+ }
+ if (lastspace >= 0)
+ s[(i = lastspace)] = '\0';
+ return i;
+}
+
+/* Get ID string from STORAGE_DEVICE_DESCRIPTIOR. */
+static bool
+stordesc_to_id_name (const UNICODE_STRING *upath, char *ioctl_buf,
+ char (& name)[NAME_MAX + 1])
+{
+ const STORAGE_DEVICE_DESCRIPTOR *desc =
+ reinterpret_cast<const STORAGE_DEVICE_DESCRIPTOR *>(ioctl_buf);
+ /* Ignore drive if information is missing, too short or too long. */
+ int vendor_len = 0, product_len = 0, serial_len = 0;
+ if (desc->VendorIdOffset)
+ vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
+ if (desc->ProductIdOffset)
+ product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
+ if (desc->SerialNumberOffset)
+ serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
+
+ bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
+ && (20 + vendor_len + 1 + product_len + 1 + serial_len + 10)
+ < (int) sizeof (name));
+ debug_printf ("%S: '%s' '%s' '%s'%s", upath,
+ (vendor_len ? ioctl_buf + desc->VendorIdOffset : ""),
+ (product_len ? ioctl_buf + desc->ProductIdOffset : ""),
+ (serial_len ? ioctl_buf + desc->SerialNumberOffset : ""),
+ (valid ? "" : " (ignored)"));
+ if (!valid)
+ return false;
+
+ /* Translate bus types. */
+ const char *bus;
+ switch (desc->BusType)
+ {
+ case BusTypeAta: bus = "ata-"; break;
+ case BusTypeFibre: bus = "fibre-"; break;
+ case BusTypeNvme: bus = "nvme-"; break;
+ case BusTypeRAID: bus = "raid-"; break;
+ case BusTypeSas: bus = "sas-"; break;
+ case BusTypeSata: bus = "sata-"; break;
+ case BusTypeScsi: bus = "scsi-"; break;
+ case BusTypeUsb: bus = "usb-"; break;
+ case BusTypeVirtual: bus = "virtual-"; break;
+ default: bus = nullptr; break;
+ }
+ if (bus)
+ strcpy (name, bus);
+ else
+ __small_sprintf (name, "bustype0x%02x-", desc->BusType);
+
+ /* Create "BUSTYPE-[VENDOR_]PRODUCT_SERIAL" string. */
+ if (vendor_len)
+ strcat (name, ioctl_buf + desc->VendorIdOffset);
+ if (product_len)
+ {
+ if (vendor_len)
+ strcat (name, "_");
+ strcat (name, ioctl_buf + desc->ProductIdOffset);
+ }
+ strcat (name, "_");
+ strcat (name, ioctl_buf + desc->SerialNumberOffset);
+ return true;
+}
+
+struct by_id_entry
+{
+ char name[NAME_MAX + 1];
+ u_int8_t drive;
+ u_int8_t part;
+};
+
+static int
+by_id_compare_name (const void *a, const void *b)
+{
+ const by_id_entry *ap = reinterpret_cast<const by_id_entry *>(a);
+ const by_id_entry *bp = reinterpret_cast<const by_id_entry *>(b);
+ return strcmp (ap->name, bp->name);
+}
+
+static by_id_entry *
+by_id_realloc (by_id_entry *p, size_t n)
+{
+ void *p2 = realloc(p, n * sizeof (*p));
+ if (!p2)
+ {
+ free (p);
+ return nullptr;
+ }
+ return reinterpret_cast<by_id_entry *>(p2);
+}
+
+/* Create sorted name -> drive mapping table. Must be freed by caller. */
+static int
+get_by_id_table (by_id_entry * &table)
+{
+ table = nullptr;
+
+ /* Open \Device object directory. */
+ wchar_t wpath[MAX_PATH] = L"\\Device";
+ UNICODE_STRING upath = {14, 16, wpath};
+ OBJECT_ATTRIBUTES attr;
+ InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE, nullptr, nullptr);
+ HANDLE dirhdl;
+ NTSTATUS status = NtOpenDirectoryObject (&dirhdl, DIRECTORY_QUERY, &attr);
+ if (!NT_SUCCESS (status))
+ {
+ debug_printf ("NtOpenDirectoryObject, status %y", status);
+ __seterrno_from_nt_status (status);
+ return -1;
+ }
+
+ /* Limit disk and partition numbers like fhandler_dev::readdir (). */
+ const unsigned max_drive_num = 127, max_part_num = 63;
+
+ unsigned alloc_size = 0, table_size = 0;
+ tmp_pathbuf tp;
+ char *ioctl_buf = tp.c_get ();
+ DIRECTORY_BASIC_INFORMATION *dbi_buf =
+ reinterpret_cast<DIRECTORY_BASIC_INFORMATION *>(tp.w_get ());
+
+ /* Traverse \Device directory ... */
+ bool errno_set = false;
+ BOOLEAN restart = TRUE;
+ bool last_run = false;
+ ULONG context = 0;
+ while (!last_run)
+ {
+ status = NtQueryDirectoryObject (dirhdl, dbi_buf, 65536, FALSE, restart,
+ &context, nullptr);
+ if (!NT_SUCCESS (status))
+ {
+ __seterrno_from_nt_status (status);
+ errno_set = true;
+ debug_printf ("NtQueryDirectoryObject, status %y", status);
+ break;
+ }
+ if (status != STATUS_MORE_ENTRIES)
+ last_run = true;
+ restart = FALSE;
+ for (const DIRECTORY_BASIC_INFORMATION *dbi = dbi_buf;
+ dbi->ObjectName.Length > 0;
+ dbi++)
+ {
+ /* ... and check for a "Harddisk[0-9]*" entry. */
+ if (dbi->ObjectName.Length < 9 * sizeof (WCHAR)
+ || wcsncasecmp (dbi->ObjectName.Buffer, L"Harddisk", 8) != 0
+ || !iswdigit (dbi->ObjectName.Buffer[8]))
+ continue;
+ /* Got it. Now construct the path to the entire disk, which is
+ "\\Device\\HarddiskX\\Partition0", and open the disk with
+ minimum permissions. */
+ unsigned long drive_num = wcstoul (dbi->ObjectName.Buffer + 8,
+ nullptr, 10);
+ if (drive_num > max_drive_num)
+ continue;
+ wcscpy (wpath, dbi->ObjectName.Buffer);
+ PWCHAR wpart = wpath + dbi->ObjectName.Length / sizeof (WCHAR);
+ wcpcpy (wpart, L"\\Partition0");
+ upath.Length = dbi->ObjectName.Length + 22;
+ upath.MaximumLength = upath.Length + sizeof (WCHAR);
+ InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE,
+ dirhdl, nullptr);
+ /* SYNCHRONIZE access is required for IOCTL_STORAGE_QUERY_PROPERTY
+ for drives behind some drivers (nvmestor.sys). */
+ HANDLE devhdl;
+ IO_STATUS_BLOCK io;
+ status = NtOpenFile (&devhdl, READ_CONTROL | SYNCHRONIZE, &attr, &io,
+ FILE_SHARE_VALID_FLAGS, 0);
+ if (!NT_SUCCESS (status))
+ {
+ __seterrno_from_nt_status (status);
+ errno_set = true;
+ debug_printf ("NtOpenFile(%S), status %y", &upath, status);
+ continue;
+ }
+
+ /* Fetch vendor, product and serial number. */
+ DWORD bytes_read;
+ STORAGE_PROPERTY_QUERY query =
+ { StorageDeviceProperty, PropertyStandardQuery, { 0 } };
+ if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
+ &query, sizeof (query),
+ ioctl_buf, NT_MAX_PATH,
+ &bytes_read, nullptr))
+ {
+ __seterrno_from_win_error (GetLastError ());
+ errno_set = true;
+ debug_printf ("DeviceIoControl (%S, "
+ "IOCTL_STORAGE_QUERY_PROPERTY): %E", &upath);
+ continue;
+ }
+
+ /* Add table space for drive, partitions and end marker. */
+ if (alloc_size <= table_size + max_part_num)
+ {
+ alloc_size = table_size + max_part_num + 8;
+ table = by_id_realloc (table, alloc_size);
+ if (!table)
+ return -1;
+ }
+
+ if (!stordesc_to_id_name (&upath, ioctl_buf, table[table_size].name))
+ continue;
+ int drive_index = table_size++;
+ size_t drive_len = strlen(table[drive_index].name);
+ table[drive_index].drive = drive_num;
+ table[drive_index].part = 0;
+
+ /* Fetch drive layout info to get size of all partitions on disk. */
+ DWORD part_cnt = 0;
+ PARTITION_INFORMATION_EX *pix = nullptr;
+ PARTITION_INFORMATION *pi = nullptr;
+ if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 0,
+ ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
+ {
+ DRIVE_LAYOUT_INFORMATION_EX *pdlix =
+ reinterpret_cast<DRIVE_LAYOUT_INFORMATION_EX *>(ioctl_buf);
+ part_cnt = pdlix->PartitionCount;
+ pix = pdlix->PartitionEntry;
+ }
+ else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, nullptr,
+ 0, ioctl_buf, NT_MAX_PATH, &bytes_read,
+ nullptr))
+ {
+ DRIVE_LAYOUT_INFORMATION *pdli =
+ reinterpret_cast<DRIVE_LAYOUT_INFORMATION *>(ioctl_buf);
+ part_cnt = pdli->PartitionCount;
+ pi = pdli->PartitionEntry;
+ }
+ else
+ debug_printf ("DeviceIoControl(%S, "
+ "IOCTL_DISK_GET_DRIVE_LAYOUT{_EX}): %E", &upath);
+
+ /* Loop over partitions. */
+ if (pix || pi)
+ for (DWORD i = 0; i < part_cnt; i++)
+ {
+ DWORD part_num;
+ if (pix)
+ {
+ part_num = pix->PartitionNumber;
+ ++pix;
+ }
+ else
+ {
+ part_num = pi->PartitionNumber;
+ ++pi;
+ }
+ /* A partition number of 0 denotes an extended partition or a
+ filler entry as described in
+ fhandler_dev_floppy::lock_partition. Just skip. */
+ if (part_num == 0)
+ continue;
+ if (part_num > max_part_num)
+ break;
+ table[table_size] = table[drive_index];
+ __small_sprintf(table[table_size].name + drive_len, "-part%u",
+ part_num);
+ table[table_size].part = part_num;
+ table_size++;
+ }
+ NtClose (devhdl);
+ }
+ }
+ NtClose (dirhdl);
+
+ if (!table_size && table)
+ {
+ free (table);
+ table = nullptr;
+ }
+ if (!table)
+ return (errno_set ? -1 : 0);
+
+ /* Sort by name and remove duplicates. */
+ qsort (table, table_size, sizeof (*table), by_id_compare_name);
+ for (unsigned i = 0; i < table_size; i++)
+ {
+ unsigned j = i + 1;
+ while (j < table_size && !strcmp (table[i].name, table[j].name))
+ j++;
+ if (j == i + 1)
+ continue;
+ /* Duplicate(s) found, remove all entries with this name. */
+ debug_printf ("removing duplicates %d-%d: '%s'", i, j - 1, table[i].name);
+ if (j < table_size)
+ memmove (table + i, table + j, (table_size - j) * sizeof (*table));
+ table_size -= j - i;
+ i--;
+ }
+
+ debug_printf ("table_size: %d", table_size);
+ return table_size;
+}
+
+const char dev_disk[] = "/dev/disk";
+const size_t dev_disk_len = sizeof (dev_disk) - 1;
+
+fhandler_dev_disk::fhandler_dev_disk ():
+ fhandler_virtual (),
+ loc (unknown_loc),
+ drive_from_id (-1),
+ part_from_id (0)
+{
+}
+
+void
+fhandler_dev_disk::init_dev_disk ()
+{
+ if (loc != unknown_loc)
+ return;
+
+ static const char by_id[] = "/by-id";
+ const size_t by_id_len = sizeof(by_id) - 1;
+
+ /* Determine location. */
+ const char *path = get_name ();
+ if (!path_prefix_p (dev_disk, path, dev_disk_len, false))
+ loc = invalid_loc; // should not happen
+ else if (!path[dev_disk_len])
+ loc = disk_dir; // "/dev/disk"
+ else if (!path_prefix_p (by_id, path + dev_disk_len, by_id_len, false))
+ loc = invalid_loc; // "/dev/disk/invalid"
+ else if (!path[dev_disk_len + by_id_len])
+ loc = by_id_dir; // "/dev/disk/by-id"
+ else if (strchr (path + dev_disk_len + by_id_len + 1, '/'))
+ loc = invalid_loc; // "/dev/disk/by-id/dir/invalid"
+ else
+ loc = by_id_link; // possible "/dev/disk/by-id/LINK"
+ debug_printf ("'%s': loc %d", path, (int)loc);
+
+ /* Done if "/dev/disk", "/dev/disk/by_id" or invalid. */
+ if (loc != by_id_link)
+ return;
+
+ /* Check whether "/dev/disk/by_id/LINK" exists. */
+ by_id_entry *table;
+ int table_size = get_by_id_table (table);
+ if (!table)
+ {
+ loc = invalid_loc;
+ return;
+ }
+
+ by_id_entry key;
+ strcpy (key.name, path + dev_disk_len + by_id_len + 1);
+ const void *found = bsearch (&key, table, table_size, sizeof (*table),
+ by_id_compare_name);
+ if (found)
+ {
+ /* Preserve drive and partition numbers for fillbuf (). */
+ const by_id_entry *e = reinterpret_cast<const by_id_entry *>(found);
+ drive_from_id = e->drive;
+ part_from_id = e->part;
+ }
+ else
+ loc = invalid_loc;
+ free (table);
+}
+
+virtual_ftype_t
+fhandler_dev_disk::exists ()
+{
+ debug_printf ("exists (%s)", get_name ());
+ ensure_inited ();
+ switch (loc)
+ {
+ case disk_dir:
+ case by_id_dir:
+ return virt_directory;
+ case by_id_link:
+ return virt_symlink;
+ default:
+ return virt_none;
+ }
+}
+
+int
+fhandler_dev_disk::fstat (struct stat *buf)
+{
+ debug_printf ("fstat (%s)", get_name ());
+ ensure_inited ();
+ if (loc == invalid_loc)
+ {
+ set_errno (ENOENT);
+ return -1;
+ }
+
+ fhandler_base::fstat (buf);
+ buf->st_mode = (loc == by_id_link ? S_IFLNK | S_IWUSR | S_IWGRP | S_IWOTH
+ : S_IFDIR) | STD_RBITS | STD_XBITS;
+ buf->st_ino = get_ino ();
+ return 0;
+}
+
+static inline by_id_entry **
+dir_id_table (DIR *dir)
+{
+ return reinterpret_cast<by_id_entry **>(&dir->__d_internal);
+}
+
+DIR *
+fhandler_dev_disk::opendir (int fd)
+{
+ ensure_inited ();
+ if (!(loc == disk_dir || loc == by_id_dir))
+ {
+ set_errno (ENOTDIR);
+ return nullptr;
+ }
+
+ by_id_entry *table = nullptr;
+ if (loc == by_id_dir)
+ {
+ int table_size = get_by_id_table (table);
+ if (table_size < 0)
+ return nullptr; /* errno is set. */
+ if (table)
+ {
+ /* Shrink to required table_size. */
+ table = by_id_realloc (table, table_size + 1);
+ if (!table)
+ return nullptr; /* Should not happen. */
+ /* Mark end of table for readdir (). */
+ table[table_size].name[0] = '\0';
+ }
+ }
+
+ DIR *dir = fhandler_virtual::opendir (fd);
+ if (!dir)
+ {
+ free (table);
+ return nullptr;
+ }
+ dir->__flags = dirent_saw_dot | dirent_saw_dot_dot;
+ *dir_id_table (dir) = table;
+ return dir;
+}
+
+int
+fhandler_dev_disk::closedir (DIR *dir)
+{
+ free (*dir_id_table (dir));
+ return fhandler_virtual::closedir (dir);
+}
+
+int
+fhandler_dev_disk::readdir (DIR *dir, dirent *de)
+{
+ int res;
+ if (dir->__d_position < 2)
+ {
+ de->d_name[0] = '.';
+ de->d_name[1] = (dir->__d_position ? '.' : '\0');
+ de->d_name[2] = '\0';
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == disk_dir && dir->__d_position == 2)
+ {
+ strcpy (de->d_name, "by-id");
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == by_id_dir && *dir_id_table (dir))
+ {
+ const char *name = (*dir_id_table (dir))[dir->__d_position - 2].name;
+ if (name[0])
+ {
+ strcpy (de->d_name, name);
+ de->d_type = DT_LNK;
+ dir->__d_position++;
+ res = 0;
+ }
+ else
+ res = ENMFILE;
+ }
+ else
+ res = ENMFILE;
+
+ syscall_printf ("%d = readdir(%p, %p) (%s)", res, dir, de,
+ (!res ? de->d_name : ""));
+ return res;
+}
+
+int
+fhandler_dev_disk::open (int flags, mode_t mode)
+{
+ ensure_inited ();
+ int err = 0;
+ if (!fhandler_virtual::open (flags, mode))
+ err = -1;
+ else if (loc == disk_dir || loc == by_id_dir)
+ {
+ if ((flags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
+ err = EEXIST;
+ else if (flags & O_WRONLY)
+ err = EISDIR;
+ else
+ diropen = true;
+ }
+ /* else if (loc == by_id_link) { } */ /* should not happen */
+ else /* (loc == invalid_loc) */
+ {
+ if (flags & O_CREAT)
+ err = EROFS;
+ else
+ err = ENOENT;
+ }
+
+ int res;
+ if (!err)
+ {
+ nohandle (true);
+ set_open_status ();
+ res = 1;
+ }
+ else
+ {
+ if (err > 0)
+ set_errno (err);
+ res = 0;
+ }
+
+ syscall_printf ("%d = fhandler_dev_disk::open(%y, 0%o)", res, flags, mode);
+ return res;
+}
+
+bool
+fhandler_dev_disk::fill_filebuf ()
+{
+ debug_printf ("fill_filebuf (%s)", get_name ());
+ ensure_inited ();
+ if (!(loc == by_id_link && drive_from_id >= 0))
+ return false;
+
+ char buf[32];
+ int len;
+ if (drive_from_id + 'a' <= 'z')
+ len = __small_sprintf (buf, "../../sd%c", drive_from_id + 'a');
+ else
+ len = __small_sprintf (buf, "../../sd%c%c",
+ drive_from_id / ('z' - 'a' + 1) - 1 + 'a',
+ drive_from_id % ('z' - 'a' + 1) + 'a');
+ if (part_from_id)
+ __small_sprintf (buf + len, "%d", part_from_id);
+
+ if (filebuf)
+ cfree (filebuf);
+ filebuf = cstrdup (buf);
+ return true;
+}
diff --git a/winsup/cygwin/local_includes/devices.h b/winsup/cygwin/local_includes/devices.h
index 10035263d..1e035f9d6 100644
--- a/winsup/cygwin/local_includes/devices.h
+++ b/winsup/cygwin/local_includes/devices.h
@@ -71,6 +71,7 @@ enum fh_devices
FH_DEV = FHDEV (DEV_VIRTFS_MAJOR, 193),
FH_CYGDRIVE= FHDEV (DEV_VIRTFS_MAJOR, 192),
FH_DEV_FD = FHDEV (DEV_VIRTFS_MAJOR, 191),
+ FH_DEV_DISK= FHDEV (DEV_VIRTFS_MAJOR, 190),
FH_SIGNALFD= FHDEV (DEV_VIRTFS_MAJOR, 13),
FH_TIMERFD = FHDEV (DEV_VIRTFS_MAJOR, 14),
@@ -415,6 +416,8 @@ extern const _device dev_piper_storage;
#define piper_dev ((device *) &dev_piper_storage)
extern const _device dev_pipew_storage;
#define pipew_dev ((device *) &dev_pipew_storage)
+extern const _device dev_dev_disk_storage;
+#define dev_disk_dev ((device *) &dev_dev_disk_storage)
extern const _device dev_proc_storage;
#define proc_dev ((device *) &dev_proc_storage)
extern const _device dev_dev_storage;
@@ -439,7 +442,8 @@ extern const _device dev_fs_storage;
#define isprocsys_dev(devn) (devn == FH_PROCSYS)
#define isvirtual_dev(devn) \
- (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE || devn == FH_DEV_FD)
+ (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE \
+ || devn == FH_DEV_FD || devn == FH_DEV_DISK)
#define iscons_dev(n) \
((device::major ((dev_t) (n)) == DEV_CONS_MAJOR) \
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index 212c22344..cd61aadf8 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -40,6 +40,8 @@ details. */
extern const char *windows_device_names[];
extern struct __cygwin_perfile *perfile_table;
#define __fmode (*(user_data->fmode_ptr))
+extern const char dev_disk[];
+extern const size_t dev_disk_len;
extern const char proc[];
extern const size_t proc_len;
extern const char procsys[];
@@ -3190,6 +3192,50 @@ class fhandler_procnet: public fhandler_proc
}
};
+class fhandler_dev_disk: public fhandler_virtual
+{
+ enum dev_disk_location {
+ unknown_loc, invalid_loc, disk_dir, by_id_dir, by_id_link
+ };
+ dev_disk_location loc;
+
+ void init_dev_disk ();
+ void ensure_inited ()
+ {
+ if (loc == unknown_loc)
+ init_dev_disk ();
+ }
+
+ int drive_from_id;
+ int part_from_id;
+
+ public:
+ fhandler_dev_disk ();
+ fhandler_dev_disk (void *) {}
+ virtual_ftype_t exists();
+ DIR *opendir (int fd);
+ int closedir (DIR *);
+ int readdir (DIR *, dirent *);
+ int open (int flags, mode_t mode = 0);
+ int fstat (struct stat *buf);
+ bool fill_filebuf ();
+
+ void copy_from (fhandler_base *x)
+ {
+ pc.free_strings ();
+ *this = *reinterpret_cast<fhandler_dev_disk *> (x);
+ _copy_from_reset_helper ();
+ }
+
+ fhandler_dev_disk *clone (cygheap_types malloc_type = HEAP_FHANDLER)
+ {
+ void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_dev_disk));
+ fhandler_dev_disk *fh = new (ptr) fhandler_dev_disk (ptr);
+ fh->copy_from (this);
+ return fh;
+ }
+};
+
class fhandler_dev_fd: public fhandler_virtual
{
public:
@@ -3416,6 +3462,7 @@ typedef union
char __dev_raw[sizeof (fhandler_dev_raw)];
char __dev_tape[sizeof (fhandler_dev_tape)];
char __dev_zero[sizeof (fhandler_dev_zero)];
+ char __dev_disk[sizeof (fhandler_dev_disk)];
char __dev_fd[sizeof (fhandler_dev_fd)];
char __disk_file[sizeof (fhandler_disk_file)];
char __fifo[sizeof (fhandler_fifo)];
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index 36ab042a7..13ace6250 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -35,6 +35,9 @@ details. */
(path[mount_table->cygdrive_len + 1] == '/' || \
!path[mount_table->cygdrive_len + 1]))
+#define isdev_disk(path) \
+ (path_prefix_p (dev_disk, (path), dev_disk_len, false))
+
#define isproc(path) \
(path_prefix_p (proc, (path), proc_len, false))
@@ -685,6 +688,13 @@ mount_info::conv_to_win32_path (const char *src_path, char *dst, device& dev,
/* Go through chroot check */
goto out;
}
+ if (isdev_disk (src_path))
+ {
+ dev = *dev_disk_dev;
+ *flags = 0;
+ strcpy (dst, src_path);
+ goto out;
+ }
if (isproc (src_path))
{
dev = *proc_dev;
--
2.39.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-10-03 12:39 ` Christian Franke
@ 2023-11-03 9:55 ` Corinna Vinschen
2023-11-03 10:09 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-03 9:55 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
Hi Christian,
On Oct 3 14:39, Christian Franke wrote:
> Christian Franke wrote:
> > This is a first attempt to partly emulate the Linux directory
> > /dev/disk/by-id. Useful to make sure the correct device is accessed in
> > conjunction with dd, ddrescue, fdisk, ....
>
> Attached is the second attempt.
>
>
> > The additional '*-partN' links to partitions are not yet included.
>
> These are now included.
>
>
> > This only works properly if Win32 path '\\.\PhysicalDriveN' is always
> > trivially mapped to NT path '\Device\HarddiskN\Partition0'.
> > IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
> > READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all
> > drivers. With stornvme.sys, it fails with permission denied. Perhaps
> > other permission bits are required for NtOpenFile(). Thanks for any info
> > regarding this.
>
> According to NtQueryObject(., ObjectBasicInformation, ...), using
> NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
> to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> For some unknown reason, NVMe drives behind stornvme.sys additionally
> require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
> bug in the access check somewhere in the NVMe stack.
>
> The disk scanning from the first patch has been reworked based on code
> borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
> make sense to provide one flexible scanning function for /dev/sdXN,
> /proc/partitions and /proc/disk/by-id.
I applied your patch locally (patch looks pretty well, btw) but found
that /dev/disk/by-id is empty, even when running with admin rights.
I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM. A
\Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
exists in both cases. I straced it, and found the following debug
output:
1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 'Red_Hat' 'VirtIO' '' (ignored)
Is that really desired?
Thanks,
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 9:55 ` Corinna Vinschen
@ 2023-11-03 10:09 ` Corinna Vinschen
2023-11-03 11:06 ` Christian Franke
2023-11-03 11:10 ` Corinna Vinschen
0 siblings, 2 replies; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-03 10:09 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 3 10:55, Corinna Vinschen wrote:
> Hi Christian,
>
> On Oct 3 14:39, Christian Franke wrote:
> > Christian Franke wrote:
> > > This is a first attempt to partly emulate the Linux directory
> > > /dev/disk/by-id. Useful to make sure the correct device is accessed in
> > > conjunction with dd, ddrescue, fdisk, ....
> >
> > Attached is the second attempt.
> >
> >
> > > The additional '*-partN' links to partitions are not yet included.
> >
> > These are now included.
> >
> >
> > > This only works properly if Win32 path '\\.\PhysicalDriveN' is always
> > > trivially mapped to NT path '\Device\HarddiskN\Partition0'.
> > > IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
> > > READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all
> > > drivers. With stornvme.sys, it fails with permission denied. Perhaps
> > > other permission bits are required for NtOpenFile(). Thanks for any info
> > > regarding this.
> >
> > According to NtQueryObject(., ObjectBasicInformation, ...), using
> > NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
> > to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> > For some unknown reason, NVMe drives behind stornvme.sys additionally
> > require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
> > bug in the access check somewhere in the NVMe stack.
> >
> > The disk scanning from the first patch has been reworked based on code
> > borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
> > make sense to provide one flexible scanning function for /dev/sdXN,
> > /proc/partitions and /proc/disk/by-id.
>
> I applied your patch locally (patch looks pretty well, btw) but found
> that /dev/disk/by-id is empty, even when running with admin rights.
>
> I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM. A
> \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> exists in both cases. I straced it, and found the following debug
> output:
>
> 1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 'Red_Hat' 'VirtIO' '' (ignored)
>
> Is that really desired?
Thread 1 "ls" hit Breakpoint 2, stordesc_to_id_name (upath=0x7ffffc500,
ioctl_buf=0x10e0720 "(", name=...)
at /home/corinna/src/cygwin/vanilla/winsup/cygwin/fhandler/dev_disk.cc:44
44 const STORAGE_DEVICE_DESCRIPTOR *desc =
(gdb) n
47 int vendor_len = 0, product_len = 0, serial_len = 0;
(gdb)
48 if (desc->VendorIdOffset)
(gdb)
49 vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
(gdb)
50 if (desc->ProductIdOffset)
(gdb)
51 product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
(gdb)
52 if (desc->SerialNumberOffset)
(gdb)
53 serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
(gdb)
55 bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb) p vendor_len
$1 = 7
(gdb) p product_len
$2 = 6
(gdb) p serial_len
$3 = 0
(gdb) n
[New Thread 3944.0x1958]
56 && (20 + vendor_len + 1 + product_len + 1 + serial_len + 10)
(gdb) n
55 bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb)
55 bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
(gdb)
58 debug_printf ("%S: '%s' '%s' '%s'%s", upath,
(gdb) p valid
$4 = false
HTH,
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 10:09 ` Corinna Vinschen
@ 2023-11-03 11:06 ` Christian Franke
2023-11-03 11:11 ` Corinna Vinschen
2023-11-03 11:10 ` Corinna Vinschen
1 sibling, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-11-03 11:06 UTC (permalink / raw)
To: cygwin-patches
Corinna Vinschen wrote:
> On Nov 3 10:55, Corinna Vinschen wrote:
>> Hi Christian,
>>
>> On Oct 3 14:39, Christian Franke wrote:
>>> Christian Franke wrote:
>>>> This is a first attempt to partly emulate the Linux directory
>>>> /dev/disk/by-id. Useful to make sure the correct device is accessed in
>>>> conjunction with dd, ddrescue, fdisk, ....
>>> Attached is the second attempt.
>>>
>>>
>>>> The additional '*-partN' links to partitions are not yet included.
>>> These are now included.
>>>
>>>
>>>> This only works properly if Win32 path '\\.\PhysicalDriveN' is always
>>>> trivially mapped to NT path '\Device\HarddiskN\Partition0'.
>>>> IOCTL_STORAGE_QUERY_PROPERTY with a handle from NtOpenFile(.,
>>>> READ_CONTROL,...) instead of CreateFile(., 0, ...) did not work with all
>>>> drivers. With stornvme.sys, it fails with permission denied. Perhaps
>>>> other permission bits are required for NtOpenFile(). Thanks for any info
>>>> regarding this.
>>> According to NtQueryObject(., ObjectBasicInformation, ...), using
>>> NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
>>> to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
>>> For some unknown reason, NVMe drives behind stornvme.sys additionally
>>> require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
>>> bug in the access check somewhere in the NVMe stack.
>>>
>>> The disk scanning from the first patch has been reworked based on code
>>> borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
>>> make sense to provide one flexible scanning function for /dev/sdXN,
>>> /proc/partitions and /proc/disk/by-id.
>> I applied your patch locally (patch looks pretty well, btw) but found
Thanks!
Meantime I found 4 missing NtClose() in the main loop of
get_by_id_table(). Will be fixed in the next version of the patch.
>> that /dev/disk/by-id is empty, even when running with admin rights.
Admin rights should not be necessary for IOCTL_STORAGE_QUERY_PROPERTY.
>>
>> I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM. A
>> \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
>> exists in both cases. I straced it, and found the following debug
>> output:
>>
>> 1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 'Red_Hat' 'VirtIO' '' (ignored)
>>
>> Is that really desired?
Yes - if IOCTL_STORAGE_QUERY_PROPERTY{... PropertyStandardQuery} does
not return a serial number (''), the device is intentionally ignored.
> Thread 1 "ls" hit Breakpoint 2, stordesc_to_id_name (upath=0x7ffffc500,
> ioctl_buf=0x10e0720 "(", name=...)
> at /home/corinna/src/cygwin/vanilla/winsup/cygwin/fhandler/dev_disk.cc:44
> 44 const STORAGE_DEVICE_DESCRIPTOR *desc =
> (gdb) n
> 47 int vendor_len = 0, product_len = 0, serial_len = 0;
> (gdb)
> 48 if (desc->VendorIdOffset)
> (gdb)
> 49 vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
> (gdb)
> 50 if (desc->ProductIdOffset)
> (gdb)
> 51 product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
> (gdb)
> 52 if (desc->SerialNumberOffset)
> (gdb)
> 53 serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
If possibly, please check whether (desc->SerialNumberOffset) is 0 or
(ioctl_buf + desc->SerialNumberOffset) points to '\0' or a string of
spaces. If no, there is possibly something wrong in sanitize_id_string().
> (gdb)
> 55 bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
> (gdb) p vendor_len
> $1 = 7
> (gdb) p product_len
> $2 = 6
> (gdb) p serial_len
> $3 = 0
> (gdb) n
> [New Thread 3944.0x1958]
> 56 && (20 + vendor_len + 1 + product_len + 1 + serial_len + 10)
> (gdb) n
> 55 bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
> (gdb)
> 55 bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
> (gdb)
> 58 debug_printf ("%S: '%s' '%s' '%s'%s", upath,
> (gdb) p valid
> $4 = false
--
Regards,
Christian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 10:09 ` Corinna Vinschen
2023-11-03 11:06 ` Christian Franke
@ 2023-11-03 11:10 ` Corinna Vinschen
2023-11-03 13:27 ` Corinna Vinschen
1 sibling, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-03 11:10 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 3 11:09, Corinna Vinschen wrote:
> On Nov 3 10:55, Corinna Vinschen wrote:
> > On Oct 3 14:39, Christian Franke wrote:
> > > According to NtQueryObject(., ObjectBasicInformation, ...), using
> > > NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
> > > to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> > > For some unknown reason, NVMe drives behind stornvme.sys additionally
> > > require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
> > > bug in the access check somewhere in the NVMe stack.
> > >
> > > The disk scanning from the first patch has been reworked based on code
> > > borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
> > > make sense to provide one flexible scanning function for /dev/sdXN,
> > > /proc/partitions and /proc/disk/by-id.
> >
> > I applied your patch locally (patch looks pretty well, btw) but found
> > that /dev/disk/by-id is empty, even when running with admin rights.
> >
> > I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM. A
> > \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> > exists in both cases. I straced it, and found the following debug
> > output:
> >
> > 1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 'Red_Hat' 'VirtIO' '' (ignored)
> >
> > Is that really desired?
We could fake a serial number dependent on the path. See
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/mount.cc;hb=main#l253
Alternatively, there's also a symlink in GLOBAL?? pointing
to the same target as \Device\Harddisk0\Partition0, i. e.:
\Device\Harddisk0\Partition0 -> \Device\Harddisk0\DR0
and
\GLOBAL??\Disk{4c622943-27e4-e81d-3fc7-c43fc9c7e126} -> \Device\Harddisk0\DR0
We could use that UUID from that path, but that's quite a hassle
to grab, because it requires to enumerate GLOBAL??.
But then again, where does Windows get the UUID from? Something to
find out...
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 11:06 ` Christian Franke
@ 2023-11-03 11:11 ` Corinna Vinschen
0 siblings, 0 replies; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-03 11:11 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 3 12:06, Christian Franke wrote:
> Corinna Vinschen wrote:
> > > I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM. A
> > > \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> > > exists in both cases. I straced it, and found the following debug
> > > output:
> > >
> > > 1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 'Red_Hat' 'VirtIO' '' (ignored)
> > >
> > > Is that really desired?
>
> Yes - if IOCTL_STORAGE_QUERY_PROPERTY{... PropertyStandardQuery} does not
> return a serial number (''), the device is intentionally ignored.
See my other mail I just sent.
> > Thread 1 "ls" hit Breakpoint 2, stordesc_to_id_name (upath=0x7ffffc500,
> > ioctl_buf=0x10e0720 "(", name=...)
> > at /home/corinna/src/cygwin/vanilla/winsup/cygwin/fhandler/dev_disk.cc:44
> > 44 const STORAGE_DEVICE_DESCRIPTOR *desc =
> > (gdb) n
> > 47 int vendor_len = 0, product_len = 0, serial_len = 0;
> > (gdb)
> > 48 if (desc->VendorIdOffset)
> > (gdb)
> > 49 vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
> > (gdb)
> > 50 if (desc->ProductIdOffset)
> > (gdb)
> > 51 product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
> > (gdb)
> > 52 if (desc->SerialNumberOffset)
> > (gdb)
> > 53 serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
>
> If possibly, please check whether (desc->SerialNumberOffset) is 0 or
> (ioctl_buf + desc->SerialNumberOffset) points to '\0' or a string of spaces.
> If no, there is possibly something wrong in sanitize_id_string().
desc->SerialNumberOffset is > 0, serial_len is 0, it points to an empty
string.
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 11:10 ` Corinna Vinschen
@ 2023-11-03 13:27 ` Corinna Vinschen
2023-11-03 16:09 ` Christian Franke
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-03 13:27 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 3 12:10, Corinna Vinschen wrote:
> On Nov 3 11:09, Corinna Vinschen wrote:
> > On Nov 3 10:55, Corinna Vinschen wrote:
> > > On Oct 3 14:39, Christian Franke wrote:
> > > > According to NtQueryObject(., ObjectBasicInformation, ...), using
> > > > NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
> > > > to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
> > > > For some unknown reason, NVMe drives behind stornvme.sys additionally
> > > > require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
> > > > bug in the access check somewhere in the NVMe stack.
> > > >
> > > > The disk scanning from the first patch has been reworked based on code
> > > > borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
> > > > make sense to provide one flexible scanning function for /dev/sdXN,
> > > > /proc/partitions and /proc/disk/by-id.
> > >
> > > I applied your patch locally (patch looks pretty well, btw) but found
> > > that /dev/disk/by-id is empty, even when running with admin rights.
> > >
> > > I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM. A
> > > \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
> > > exists in both cases. I straced it, and found the following debug
> > > output:
> > >
> > > 1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 'Red_Hat' 'VirtIO' '' (ignored)
> > >
> > > Is that really desired?
>
> We could fake a serial number dependent on the path. See
> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/mount.cc;hb=main#l253
>
> Alternatively, there's also a symlink in GLOBAL?? pointing
> to the same target as \Device\Harddisk0\Partition0, i. e.:
>
> \Device\Harddisk0\Partition0 -> \Device\Harddisk0\DR0
>
> and
>
> \GLOBAL??\Disk{4c622943-27e4-e81d-3fc7-c43fc9c7e126} -> \Device\Harddisk0\DR0
>
> We could use that UUID from that path, but that's quite a hassle
> to grab, because it requires to enumerate GLOBAL??.
>
> But then again, where does Windows get the UUID from? Something to
> find out...
I haven't found out where the UUID is coming from, yet, but based on the
description from
https://learn.microsoft.com/en-us/windows-hardware/drivers/storage/device-unique-identifiers--duids--for-storage-devices
I came up with this Q&D solution:
=============== SNIP ================
diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
index caca57d63216..74abfb8a3288 100644
--- a/winsup/cygwin/fhandler/dev_disk.cc
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -36,29 +36,51 @@ sanitize_id_string (char *s)
return i;
}
+typedef struct _STORAGE_DEVICE_UNIQUE_IDENTIFIER {
+ ULONG Version;
+ ULONG Size;
+ ULONG StorageDeviceIdOffset;
+ ULONG StorageDeviceOffset;
+ ULONG DriveLayoutSignatureOffset;
+} STORAGE_DEVICE_UNIQUE_IDENTIFIER, *PSTORAGE_DEVICE_UNIQUE_IDENTIFIER;
+
+typedef struct _STORAGE_DEVICE_LAYOUT_SIGNATURE {
+ ULONG Version;
+ ULONG Size;
+ BOOLEAN Mbr;
+ union {
+ ULONG MbrSignature;
+ GUID GptDiskId;
+ } DeviceSpecific;
+} STORAGE_DEVICE_LAYOUT_SIGNATURE, *PSTORAGE_DEVICE_LAYOUT_SIGNATURE;
+
/* Get ID string from STORAGE_DEVICE_DESCRIPTIOR. */
static bool
stordesc_to_id_name (const UNICODE_STRING *upath, char *ioctl_buf,
char (& name)[NAME_MAX + 1])
{
+ const STORAGE_DEVICE_UNIQUE_IDENTIFIER *id =
+ reinterpret_cast<const STORAGE_DEVICE_UNIQUE_IDENTIFIER *>(ioctl_buf);
+ char *desc_buf = ioctl_buf + id->StorageDeviceOffset;
const STORAGE_DEVICE_DESCRIPTOR *desc =
- reinterpret_cast<const STORAGE_DEVICE_DESCRIPTOR *>(ioctl_buf);
+ reinterpret_cast<const STORAGE_DEVICE_DESCRIPTOR *>(desc_buf);
+
/* Ignore drive if information is missing, too short or too long. */
int vendor_len = 0, product_len = 0, serial_len = 0;
if (desc->VendorIdOffset)
- vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
+ vendor_len = sanitize_id_string (desc_buf + desc->VendorIdOffset);
if (desc->ProductIdOffset)
- product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
+ product_len = sanitize_id_string (desc_buf + desc->ProductIdOffset);
if (desc->SerialNumberOffset)
- serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
+ serial_len = sanitize_id_string (desc_buf + desc->SerialNumberOffset);
- bool valid = (4 <= vendor_len + product_len && 4 <= serial_len
- && (20 + vendor_len + 1 + product_len + 1 + serial_len + 10)
+ bool valid = (4 <= vendor_len + product_len
+ && (20 + vendor_len + 1 + product_len + 1 + 10)
< (int) sizeof (name));
debug_printf ("%S: '%s' '%s' '%s'%s", upath,
- (vendor_len ? ioctl_buf + desc->VendorIdOffset : ""),
- (product_len ? ioctl_buf + desc->ProductIdOffset : ""),
- (serial_len ? ioctl_buf + desc->SerialNumberOffset : ""),
+ (vendor_len ? desc_buf + desc->VendorIdOffset : ""),
+ (product_len ? desc_buf + desc->ProductIdOffset : ""),
+ (serial_len ? desc_buf + desc->SerialNumberOffset : ""),
(valid ? "" : " (ignored)"));
if (!valid)
return false;
@@ -85,15 +107,24 @@ stordesc_to_id_name (const UNICODE_STRING *upath, char *ioctl_buf,
/* Create "BUSTYPE-[VENDOR_]PRODUCT_SERIAL" string. */
if (vendor_len)
- strcat (name, ioctl_buf + desc->VendorIdOffset);
+ strcat (name, desc_buf + desc->VendorIdOffset);
if (product_len)
{
if (vendor_len)
strcat (name, "_");
- strcat (name, ioctl_buf + desc->ProductIdOffset);
+ strcat (name, desc_buf + desc->ProductIdOffset);
}
strcat (name, "_");
- strcat (name, ioctl_buf + desc->SerialNumberOffset);
+ if (1) /* Utilize the DUID as defined by MSDN */
+ {
+ unsigned long hash = 0;
+
+ for (ULONG i = 0; i < id->Size; ++i)
+ hash = ioctl_buf[i] + (hash << 6) + (hash << 16) - hash;
+ __small_sprintf (name + strlen (name), "%X", hash);
+ }
+ else
+ strcat (name, desc_buf + desc->SerialNumberOffset);
return true;
}
@@ -212,7 +243,7 @@ get_by_id_table (by_id_entry * &table)
/* Fetch vendor, product and serial number. */
DWORD bytes_read;
STORAGE_PROPERTY_QUERY query =
- { StorageDeviceProperty, PropertyStandardQuery, { 0 } };
+ { StorageDeviceUniqueIdProperty, PropertyStandardQuery, { 0 } };
if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
&query, sizeof (query),
ioctl_buf, NT_MAX_PATH,
=============== SNAP ================
And, btw, rather than using strcpy/strcat, can you please utilize
stpcpy? You just have to keep the pointer around and you can
concat w/o always having to go over the full length of the string.
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 13:27 ` Corinna Vinschen
@ 2023-11-03 16:09 ` Christian Franke
2023-11-03 16:27 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-11-03 16:09 UTC (permalink / raw)
To: cygwin-patches
Corinna Vinschen wrote:
> On Nov 3 12:10, Corinna Vinschen wrote:
>> On Nov 3 11:09, Corinna Vinschen wrote:
>>> On Nov 3 10:55, Corinna Vinschen wrote:
>>>> On Oct 3 14:39, Christian Franke wrote:
>>>>> According to NtQueryObject(., ObjectBasicInformation, ...), using
>>>>> NtOpenFile(., MAXIMUM_ALLOWED, ...) without admin rights sets GrantedAccess
>>>>> to 0x001200a0 (FILE_EXECUTE|FILE_READ_ATTRIBUTES|READ_CONTROL|SYNCHRONIZE).
>>>>> For some unknown reason, NVMe drives behind stornvme.sys additionally
>>>>> require SYNCHRONIZE to use IOCTL_STORAGE_QUERY_PROPERTY. Possibly a harmless
>>>>> bug in the access check somewhere in the NVMe stack.
>>>>>
>>>>> The disk scanning from the first patch has been reworked based on code
>>>>> borrowed from proc.cc:format_proc_partitions(). For the longer term, it may
>>>>> make sense to provide one flexible scanning function for /dev/sdXN,
>>>>> /proc/partitions and /proc/disk/by-id.
>>>> I applied your patch locally (patch looks pretty well, btw) but found
>>>> that /dev/disk/by-id is empty, even when running with admin rights.
>>>>
>>>> I ran this on Windows 11 and Windows 2K19 in a QEMU/KVM VM. A
>>>> \Device\Harddisk0\Partition0 symlink pointing to \Device\Harddisk0\DR0
>>>> exists in both cases. I straced it, and found the following debug
>>>> output:
>>>>
>>>> 1015 1155432 [main] ls 361 stordesc_to_id_name: Harddisk0\Partition0: 'Red_Hat' 'VirtIO' '' (ignored)
>>>>
>>>> Is that really desired?
>> We could fake a serial number dependent on the path. See
>> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/mount.cc;hb=main#l253
>>
>> Alternatively, there's also a symlink in GLOBAL?? pointing
>> to the same target as \Device\Harddisk0\Partition0, i. e.:
>>
>> \Device\Harddisk0\Partition0 -> \Device\Harddisk0\DR0
>>
>> and
>>
>> \GLOBAL??\Disk{4c622943-27e4-e81d-3fc7-c43fc9c7e126} -> \Device\Harddisk0\DR0
>>
>> We could use that UUID from that path, but that's quite a hassle
>> to grab, because it requires to enumerate GLOBAL??.
>>
>> But then again, where does Windows get the UUID from? Something to
>> find out...
> I haven't found out where the UUID is coming from, yet, but based on the
> description from
> https://learn.microsoft.com/en-us/windows-hardware/drivers/storage/device-unique-identifiers--duids--for-storage-devices
> I came up with this Q&D solution:
>
> =============== SNIP ================
> diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
> index caca57d63216..74abfb8a3288 100644
> --- a/winsup/cygwin/fhandler/dev_disk.cc
> +++ b/winsup/cygwin/fhandler/dev_disk.cc
> @@ -36,29 +36,51 @@ sanitize_id_string (char *s)
> return i;
> }
>
> +typedef struct _STORAGE_DEVICE_UNIQUE_IDENTIFIER {
> + ULONG Version;
> + ULONG Size;
> + ULONG StorageDeviceIdOffset;
> + ULONG StorageDeviceOffset;
> + ULONG DriveLayoutSignatureOffset;
> +} STORAGE_DEVICE_UNIQUE_IDENTIFIER, *PSTORAGE_DEVICE_UNIQUE_IDENTIFIER;
> +
> +typedef struct _STORAGE_DEVICE_LAYOUT_SIGNATURE {
> + ULONG Version;
> + ULONG Size;
> + BOOLEAN Mbr;
> + union {
> + ULONG MbrSignature;
> + GUID GptDiskId;
> + } DeviceSpecific;
> +} STORAGE_DEVICE_LAYOUT_SIGNATURE, *PSTORAGE_DEVICE_LAYOUT_SIGNATURE;
> +
These are available in storduid.h
> /* Get ID string from STORAGE_DEVICE_DESCRIPTIOR. */
> static bool
> stordesc_to_id_name (const UNICODE_STRING *upath, char *ioctl_buf,
> char (& name)[NAME_MAX + 1])
> {
> + const STORAGE_DEVICE_UNIQUE_IDENTIFIER *id =
> + reinterpret_cast<const STORAGE_DEVICE_UNIQUE_IDENTIFIER *>(ioctl_buf);
> + char *desc_buf = ioctl_buf + id->StorageDeviceOffset;
> const STORAGE_DEVICE_DESCRIPTOR *desc =
> [...]
> strcat (name, "_");
> - strcat (name, ioctl_buf + desc->SerialNumberOffset);
> + if (1) /* Utilize the DUID as defined by MSDN */
> + {
> + unsigned long hash = 0;
> +
> + for (ULONG i = 0; i < id->Size; ++i)
> + hash = ioctl_buf[i] + (hash << 6) + (hash << 16) - hash;
> + __small_sprintf (name + strlen (name), "%X", hash);
> + }
> + else
> + strcat (name, desc_buf + desc->SerialNumberOffset);
> return true;
> }
>
> @@ -212,7 +243,7 @@ get_by_id_table (by_id_entry * &table)
> /* Fetch vendor, product and serial number. */
> DWORD bytes_read;
> STORAGE_PROPERTY_QUERY query =
> - { StorageDeviceProperty, PropertyStandardQuery, { 0 } };
> + { StorageDeviceUniqueIdProperty, PropertyStandardQuery, { 0 } };
> if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
> &query, sizeof (query),
> ioctl_buf, NT_MAX_PATH,
> =============== SNAP ================
Thanks. Using this makes plenty of sense as a fallback if the serial
number is unavailable. But if available, the serial number should be in
the generated name as on Linux. This would provide a persistent name
which reflects the actual device without a number invented by MS.
The serial number is usually available with (S)ATA and NVMe (namespace
uuid in the latter case). I'm not familiar with QEMU/KVM details. The
fact that both 'vendor' and 'product' are returned on your system
suggests that a SCSI/SAS controller is emulated. Unlike (S)ATA and NVMe,
the serial number is not available for free in the device identify data
block but requires an extra command (SCSI INQUIRY of VPD page 0x80).
This might not be supported by the emulated controller or Windows does
not use this command.
IIRC the serial number is sometimes available via WMI even if missing in
IOCTL_STORAGE_QUERY_PROPERTY:
wmic diskdrive get manufacturer,model,serialnumber
> And, btw, rather than using strcpy/strcat, can you please utilize
> stpcpy? You just have to keep the pointer around and you can
> concat w/o always having to go over the full length of the string.
Of course.
Christian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 16:09 ` Christian Franke
@ 2023-11-03 16:27 ` Corinna Vinschen
2023-11-03 16:30 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-03 16:27 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 3 17:09, Christian Franke wrote:
> Corinna Vinschen wrote:
> > I haven't found out where the UUID is coming from, yet, but based on the
> > description from
> > https://learn.microsoft.com/en-us/windows-hardware/drivers/storage/device-unique-identifiers--duids--for-storage-devices
> > I came up with this Q&D solution:
> >
> > =============== SNIP ================
> > diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
> > index caca57d63216..74abfb8a3288 100644
> > --- a/winsup/cygwin/fhandler/dev_disk.cc
> > +++ b/winsup/cygwin/fhandler/dev_disk.cc
> > @@ -36,29 +36,51 @@ sanitize_id_string (char *s)
> > return i;
> > }
> > +typedef struct _STORAGE_DEVICE_UNIQUE_IDENTIFIER {
> > + ULONG Version;
> > + ULONG Size;
> > + ULONG StorageDeviceIdOffset;
> > + ULONG StorageDeviceOffset;
> > + ULONG DriveLayoutSignatureOffset;
> > +} STORAGE_DEVICE_UNIQUE_IDENTIFIER, *PSTORAGE_DEVICE_UNIQUE_IDENTIFIER;
> > +
> > +typedef struct _STORAGE_DEVICE_LAYOUT_SIGNATURE {
> > + ULONG Version;
> > + ULONG Size;
> > + BOOLEAN Mbr;
> > + union {
> > + ULONG MbrSignature;
> > + GUID GptDiskId;
> > + } DeviceSpecific;
> > +} STORAGE_DEVICE_LAYOUT_SIGNATURE, *PSTORAGE_DEVICE_LAYOUT_SIGNATURE;
> > +
>
> These are available in storduid.h
Yeah, and defining STORAGE_DEVICE_LAYOUT_SIGNATURE is useless,
but it was just for PoC.
> Thanks. Using this makes plenty of sense as a fallback if the serial number
> is unavailable. But if available, the serial number should be in the
> generated name as on Linux. This would provide a persistent name which
> reflects the actual device without a number invented by MS.
ACK
> The serial number is usually available with (S)ATA and NVMe (namespace uuid
> in the latter case). I'm not familiar with QEMU/KVM details. The fact that
> both 'vendor' and 'product' are returned on your system suggests that a
> SCSI/SAS controller is emulated.
Yes.
> Unlike (S)ATA and NVMe, the serial number
> is not available for free in the device identify data block but requires an
> extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
> by the emulated controller or Windows does not use this command.
AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
which is equivalent to the data from VPD page 0x83. As you can see,
it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data. The data
returned for the VirtIo device is the identifier string "\x01\x00",
which is a bit underwhelming.
Would be great if we would learn how to access page 0x80...
> IIRC the serial number is sometimes available via WMI even if missing in
> IOCTL_STORAGE_QUERY_PROPERTY:
>
> wmic diskdrive get manufacturer,model,serialnumber
Nope, it returns an empty string, just as the date from
STORAGE_DEVICE_DESCRIPTOR.
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 16:27 ` Corinna Vinschen
@ 2023-11-03 16:30 ` Corinna Vinschen
2023-11-03 17:54 ` Christian Franke
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-03 16:30 UTC (permalink / raw)
To: cygwin-patches
On Nov 3 17:27, Corinna Vinschen wrote:
> On Nov 3 17:09, Christian Franke wrote:
> > Unlike (S)ATA and NVMe, the serial number
> > is not available for free in the device identify data block but requires an
> > extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
> > by the emulated controller or Windows does not use this command.
>
> AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
> which is equivalent to the data from VPD page 0x83. As you can see,
> it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data. The data
> returned for the VirtIo device is the identifier string "\x01\x00",
> which is a bit underwhelming.
>
> Would be great if we would learn how to access page 0x80...
Uhm...
MSDN claims:
If the storage device is SCSI-compliant, the port driver attempts to
extract the serial number from the optional Unit Serial Number page
(page 0x80) of the VPD.
Now I'm puzzled.
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 16:30 ` Corinna Vinschen
@ 2023-11-03 17:54 ` Christian Franke
2023-11-04 9:34 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-11-03 17:54 UTC (permalink / raw)
To: cygwin-patches
Corinna Vinschen wrote:
> On Nov 3 17:27, Corinna Vinschen wrote:
>> On Nov 3 17:09, Christian Franke wrote:
>>> Unlike (S)ATA and NVMe, the serial number
>>> is not available for free in the device identify data block but requires an
>>> extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
>>> by the emulated controller or Windows does not use this command.
>> AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
>> which is equivalent to the data from VPD page 0x83. As you can see,
>> it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data. The data
>> returned for the VirtIo device is the identifier string "\x01\x00",
>> which is a bit underwhelming.
>>
>> Would be great if we would learn how to access page 0x80...
> Uhm...
>
> MSDN claims:
>
> If the storage device is SCSI-compliant, the port driver attempts to
> extract the serial number from the optional Unit Serial Number page
> (page 0x80) of the VPD.
>
> Now I'm puzzled.
A quick test with a Debian 12 VM in VirtualBox with many virtual
controllers+drives shows the same problem:
Entries in /dev/disk/by-id appear only for virtual disks behind emulated
SATA and NVMe controllers, but not for SCSI and SAS controllers.
A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a
serial number. In debug mode it prints "Vital Product Data (VPD) INQUIRY
failed..." and other messages that suggest limited/buggy support of
optional SCSI commands.
If a Win11 PE (from install ISO) is run in same VM, the
STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe
drives not detected), but not for SCSI.
Conclusion: The behavior of the current patch is compatible with Linux :-)
Christian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-03 17:54 ` Christian Franke
@ 2023-11-04 9:34 ` Corinna Vinschen
2023-11-04 9:57 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-04 9:34 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 3 18:54, Christian Franke wrote:
> Corinna Vinschen wrote:
> > On Nov 3 17:27, Corinna Vinschen wrote:
> > > On Nov 3 17:09, Christian Franke wrote:
> > > > Unlike (S)ATA and NVMe, the serial number
> > > > is not available for free in the device identify data block but requires an
> > > > extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
> > > > by the emulated controller or Windows does not use this command.
> > > AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
> > > which is equivalent to the data from VPD page 0x83. As you can see,
> > > it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data. The data
> > > returned for the VirtIo device is the identifier string "\x01\x00",
> > > which is a bit underwhelming.
> > >
> > > Would be great if we would learn how to access page 0x80...
> > Uhm...
> >
> > MSDN claims:
> >
> > If the storage device is SCSI-compliant, the port driver attempts to
> > extract the serial number from the optional Unit Serial Number page
> > (page 0x80) of the VPD.
> >
> > Now I'm puzzled.
>
> A quick test with a Debian 12 VM in VirtualBox with many virtual
> controllers+drives shows the same problem:
> Entries in /dev/disk/by-id appear only for virtual disks behind emulated
> SATA and NVMe controllers, but not for SCSI and SAS controllers.
> A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a serial
> number. In debug mode it prints "Vital Product Data (VPD) INQUIRY failed..."
> and other messages that suggest limited/buggy support of optional SCSI
> commands.
>
> If a Win11 PE (from install ISO) is run in same VM, the
> STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe
> drives not detected), but not for SCSI.
>
> Conclusion: The behavior of the current patch is compatible with Linux :-)
Ok, but with the DUID we have a workaround which makes it work even
better than on Linux, so it would begreat if we used it, unless we find
out where the UUID in "\GLOBAL??\Disk{<UUID>} comes from...
Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
even contemplate a 128 bit hash, just to be on the safe side.
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-04 9:34 ` Corinna Vinschen
@ 2023-11-04 9:57 ` Corinna Vinschen
2023-11-04 11:34 ` Christian Franke
2023-11-04 15:53 ` Christian Franke
0 siblings, 2 replies; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-04 9:57 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 4 10:34, Corinna Vinschen wrote:
> On Nov 3 18:54, Christian Franke wrote:
> > Corinna Vinschen wrote:
> > > On Nov 3 17:27, Corinna Vinschen wrote:
> > > > On Nov 3 17:09, Christian Franke wrote:
> > > > > Unlike (S)ATA and NVMe, the serial number
> > > > > is not available for free in the device identify data block but requires an
> > > > > extra command (SCSI INQUIRY of VPD page 0x80). This might not be supported
> > > > > by the emulated controller or Windows does not use this command.
> > > > AFAICS, only the data from STORAGE_DEVICE_ID_DESCRIPTOR is available
> > > > which is equivalent to the data from VPD page 0x83. As you can see,
> > > > it's part of the STORAGE_DEVICE_UNIQUE_IDENTIFIER data. The data
> > > > returned for the VirtIo device is the identifier string "\x01\x00",
> > > > which is a bit underwhelming.
> > > >
> > > > Would be great if we would learn how to access page 0x80...
> > > Uhm...
> > >
> > > MSDN claims:
> > >
> > > If the storage device is SCSI-compliant, the port driver attempts to
> > > extract the serial number from the optional Unit Serial Number page
> > > (page 0x80) of the VPD.
> > >
> > > Now I'm puzzled.
> >
> > A quick test with a Debian 12 VM in VirtualBox with many virtual
> > controllers+drives shows the same problem:
> > Entries in /dev/disk/by-id appear only for virtual disks behind emulated
> > SATA and NVMe controllers, but not for SCSI and SAS controllers.
> > A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a serial
> > number. In debug mode it prints "Vital Product Data (VPD) INQUIRY failed..."
> > and other messages that suggest limited/buggy support of optional SCSI
> > commands.
> >
> > If a Win11 PE (from install ISO) is run in same VM, the
> > STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe
> > drives not detected), but not for SCSI.
> >
> > Conclusion: The behavior of the current patch is compatible with Linux :-)
>
> Ok, but with the DUID we have a workaround which makes it work even
> better than on Linux, so it would begreat if we used it, unless we find
> out where the UUID in "\GLOBAL??\Disk{<UUID>} comes from...
>
> Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
> even contemplate a 128 bit hash, just to be on the safe side.
Kind of like this
- strcat (name, ioctl_buf + desc->SerialNumberOffset);
+ /* Use SerialNumber in the first place, if available */
+ if (desc->SerialNumberOffset && desc_buf[desc->SerialNumberOffset])
+ strcat (name, desc_buf + desc->SerialNumberOffset);
+ else /* Utilize the DUID as defined by MSDN to generate a hash */
+ {
+ union {
+ unsigned __int128 all;
+ struct {
+ unsigned long high;
+ unsigned long low;
+ };
+ } hash = { 0 };
+
+ for (ULONG i = 0; i < id->Size; ++i)
+ hash.all = ioctl_buf[i] + (hash.all << 6) + (hash.all << 16) - hash.all;
+ __small_sprintf (name + strlen (name), "%X%X", hash.high, hash.low);
+ }
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-04 9:57 ` Corinna Vinschen
@ 2023-11-04 11:34 ` Christian Franke
2023-11-04 15:53 ` Christian Franke
1 sibling, 0 replies; 23+ messages in thread
From: Christian Franke @ 2023-11-04 11:34 UTC (permalink / raw)
To: cygwin-patches
Corinna Vinschen wrote:
> On Nov 4 10:34, Corinna Vinschen wrote:
>> On Nov 3 18:54, Christian Franke wrote:
>>> ...
>>>> MSDN claims:
>>>>
>>>> If the storage device is SCSI-compliant, the port driver attempts to
>>>> extract the serial number from the optional Unit Serial Number page
>>>> (page 0x80) of the VPD.
>>>>
>>>> Now I'm puzzled.
>>> A quick test with a Debian 12 VM in VirtualBox with many virtual
>>> controllers+drives shows the same problem:
>>> Entries in /dev/disk/by-id appear only for virtual disks behind emulated
>>> SATA and NVMe controllers, but not for SCSI and SAS controllers.
>>> A test with "smartctl -i ..." with SCSI/SAS devices doesn't print a serial
>>> number. In debug mode it prints "Vital Product Data (VPD) INQUIRY failed..."
>>> and other messages that suggest limited/buggy support of optional SCSI
>>> commands.
>>>
>>> If a Win11 PE (from install ISO) is run in same VM, the
>>> STORAGE_DEVICE_DESCRIPTOR only provides the serial number for SATA (NVMe
>>> drives not detected), but not for SCSI.
>>>
>>> Conclusion: The behavior of the current patch is compatible with Linux :-)
>> Ok, but with the DUID we have a workaround which makes it work even
>> better than on Linux, so it would begreat if we used it, unless we find
>> out where the UUID in "\GLOBAL??\Disk{<UUID>} comes from...
>>
>> Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
>> even contemplate a 128 bit hash, just to be on the safe side.
> Kind of like this
>
> - strcat (name, ioctl_buf + desc->SerialNumberOffset);
> + /* Use SerialNumber in the first place, if available */
> + if (desc->SerialNumberOffset && desc_buf[desc->SerialNumberOffset])
> + strcat (name, desc_buf + desc->SerialNumberOffset);
> + else /* Utilize the DUID as defined by MSDN to generate a hash */
> + {
> + union {
> + unsigned __int128 all;
> + struct {
> + unsigned long high;
> + unsigned long low;
> + };
> + } hash = { 0 };
> +
> + for (ULONG i = 0; i < id->Size; ++i)
> + hash.all = ioctl_buf[i] + (hash.all << 6) + (hash.all << 16) - hash.all;
> + __small_sprintf (name + strlen (name), "%X%X", hash.high, hash.low);
> + }
>
I agree and will provide a new patch soon.
Christian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-04 9:57 ` Corinna Vinschen
2023-11-04 11:34 ` Christian Franke
@ 2023-11-04 15:53 ` Christian Franke
2023-11-04 20:51 ` Corinna Vinschen
1 sibling, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-11-04 15:53 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]
Corinna Vinschen wrote:
> On Nov 4 10:34, Corinna Vinschen wrote:
>> On Nov 3 18:54, Christian Franke wrote:
>>> ...
>>> Conclusion: The behavior of the current patch is compatible with Linux :-)
>> Ok, but with the DUID we have a workaround which makes it work even
>> better than on Linux, so it would begreat if we used it, unless we find
>> out where the UUID in "\GLOBAL??\Disk{<UUID>} comes from...
>>
>> Given the size of the STORAGE_DEVICE_UNIQUE_IDENTIFIER struct, we could
>> even contemplate a 128 bit hash, just to be on the safe side.
> Kind of like this
>
> - strcat (name, ioctl_buf + desc->SerialNumberOffset);
> + /* Use SerialNumber in the first place, if available */
> + if (desc->SerialNumberOffset && desc_buf[desc->SerialNumberOffset])
> + strcat (name, desc_buf + desc->SerialNumberOffset);
> + else /* Utilize the DUID as defined by MSDN to generate a hash */
> + {
> + union {
> + unsigned __int128 all;
> + struct {
> + unsigned long high;
> + unsigned long low;
> + };
> + } hash = { 0 };
> +
> + for (ULONG i = 0; i < id->Size; ++i)
> + hash.all = ioctl_buf[i] + (hash.all << 6) + (hash.all << 16) - hash.all;
> + __small_sprintf (name + strlen (name), "%X%X", hash.high, hash.low);
> + }
>
New patch attached. Only fhandler/dev_disk.cc changed, devices.cc again
not included.
Christian
[-- Attachment #2: 0001-Cygwin-Add-dev-disk-by-id-symlinks.patch --]
[-- Type: text/plain, Size: 26015 bytes --]
From 5db1cd009e8dc6ff01fabdeb8cddf65d609bfe2f Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sat, 4 Nov 2023 16:49:28 +0100
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
The new directory '/dev/disk/by-id' provides symlinks for each
disk and its partitions:
'BUSTYPE-[VENDOR_]PRODUCT_SERIAL[-partN]' -> '../../sdX[N]'.
This is based on strings provided by STORAGE_DEVICE_DESCRIPTOR.
If this information is too short, a 128-bit hash of the
STORAGE_DEVICE_UNIQUE_IDENTIFIER raw data is added.
Administrator privileges are not required.
Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
winsup/cygwin/Makefile.am | 1 +
winsup/cygwin/devices.in | 4 +
winsup/cygwin/dtable.cc | 3 +
winsup/cygwin/fhandler/dev_disk.cc | 646 ++++++++++++++++++++++++
winsup/cygwin/local_includes/devices.h | 6 +-
winsup/cygwin/local_includes/fhandler.h | 47 ++
winsup/cygwin/mount.cc | 10 +
7 files changed, 716 insertions(+), 1 deletion(-)
create mode 100644 winsup/cygwin/fhandler/dev_disk.cc
diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+ fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
const _device dev_fs_storage =
{"", {FH_FS}, "", exists};
+const _device dev_dev_disk_storage =
+ {"", {FH_DEV_DISK}, "", exists};
+
const _device dev_proc_storage =
{"", {FH_PROC}, "", exists};
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace. */
%%
"/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
"/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
"/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", exists_pty, S_IFCHR, =ptys_dev
":ptym%(0-127)d", BRACK(FHDEV(DEV_PTYM_MAJOR, {$1})), "/dev/ptym{$1}", exists_internal, S_IFCHR, =ptym_dev
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 21d525389..9508f3e0b 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -585,6 +585,9 @@ fh_alloc (path_conv& pc)
case FH_DEV:
fh = cnew (fhandler_dev);
break;
+ case FH_DEV_DISK:
+ fh = cnew (fhandler_dev_disk);
+ break;
case FH_DEV_FD:
fh = cnew (fhandler_dev_fd);
break;
diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
new file mode 100644
index 000000000..2cbe5434e
--- /dev/null
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -0,0 +1,646 @@
+/* fhandler/dev_disk.cc: fhandler for the /dev/disk/by-id/... symlinks.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license. Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "winsup.h"
+#include "path.h"
+#include "fhandler.h"
+#include "tls_pbuf.h"
+#include <storduid.h>
+#include <wctype.h>
+#include <winioctl.h>
+
+/* Replace non-printing and unexpected characters, remove trailing spaces,
+ return remaining string length. */
+static int
+sanitize_id_string (char *s)
+{
+ int lastspace = -1, i;
+ for (i = 0; s[i]; i++)
+ {
+ char c = s[i];
+ if (c != ' ')
+ lastspace = -1;
+ else if (lastspace < 0)
+ lastspace = i;
+ if (('0' <= c && c <= '9') || c == '.' || c == '-'
+ || ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'))
+ continue;
+ s[i] = '_';
+ }
+ if (lastspace >= 0)
+ s[(i = lastspace)] = '\0';
+ return i;
+}
+
+/* Fetch storage properties and create the ID string.
+ returns: 1: success, 0: device ignored, -1: IoControl error. */
+static int
+storprop_to_id_name (HANDLE devhdl, const UNICODE_STRING *upath,
+ char *ioctl_buf, char (& name)[NAME_MAX + 1])
+{
+ DWORD bytes_read;
+ STORAGE_PROPERTY_QUERY descquery =
+ { StorageDeviceProperty, PropertyStandardQuery, { 0 } };
+ if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
+ &descquery, sizeof (descquery),
+ ioctl_buf, NT_MAX_PATH,
+ &bytes_read, nullptr))
+ {
+ __seterrno_from_win_error (GetLastError ());
+ debug_printf ("DeviceIoControl (%S, IOCTL_STORAGE_QUERY_PROPERTY,"
+ " {StorageDeviceProperty}): %E", upath);
+ return -1;
+ }
+
+ const STORAGE_DEVICE_DESCRIPTOR *desc =
+ reinterpret_cast<const STORAGE_DEVICE_DESCRIPTOR *>(ioctl_buf);
+ int vendor_len = 0, product_len = 0, serial_len = 0;
+ if (desc->VendorIdOffset)
+ vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
+ if (desc->ProductIdOffset)
+ product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
+ if (desc->SerialNumberOffset)
+ serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
+
+ /* Ignore drive if information is empty or too long (should not happen). */
+ if (!((vendor_len || product_len) && (20/*bustype*/ + vendor_len + 1 +
+ product_len + 1 + serial_len + 1 + 34/*hash*/ + 1 + 10/*partN*/
+ < (int) sizeof (name))))
+ return 0;
+
+ /* Translate bus types. */
+ const char *bus;
+ switch (desc->BusType)
+ {
+ case BusTypeAta: bus = "ata-"; break;
+ case BusTypeFibre: bus = "fibre-"; break;
+ case BusTypeNvme: bus = "nvme-"; break;
+ case BusTypeRAID: bus = "raid-"; break;
+ case BusTypeSas: bus = "sas-"; break;
+ case BusTypeSata: bus = "sata-"; break;
+ case BusTypeScsi: bus = "scsi-"; break;
+ case BusTypeUsb: bus = "usb-"; break;
+ case BusTypeVirtual: bus = "virtual-"; break;
+ default: bus = nullptr; break;
+ }
+
+ /* Create "BUSTYPE-[VENDOR_]PRODUCT[_SERIAL]" string. */
+ char * cp = name;
+ if (bus)
+ cp = stpcpy (cp, bus);
+ else
+ cp += __small_sprintf (cp, "bustype%02_y-", desc->BusType);
+
+ if (vendor_len)
+ cp = stpcpy (cp, ioctl_buf + desc->VendorIdOffset);
+ if (product_len)
+ {
+ if (vendor_len)
+ cp = stpcpy (cp, "_");
+ cp = stpcpy (cp, ioctl_buf + desc->ProductIdOffset);
+ }
+ if (serial_len)
+ {
+ cp = stpcpy (cp, "_");
+ cp = stpcpy (cp, ioctl_buf + desc->SerialNumberOffset);
+ }
+
+ /* Add hash if information is too short (e.g. missing serial number). */
+ bool add_hash = !(4 <= vendor_len + product_len && 4 <= serial_len);
+ debug_printf ("%S: bustype: %02_y, add_hash: %d, id: '%s' '%s' '%s' ",
+ upath, (unsigned) desc->BusType, (int) add_hash,
+ (vendor_len ? ioctl_buf + desc->VendorIdOffset : ""),
+ (product_len ? ioctl_buf + desc->ProductIdOffset : ""),
+ (serial_len ? ioctl_buf + desc->SerialNumberOffset : ""));
+ if (!add_hash)
+ return 1;
+
+ /* The call below also returns the STORAGE_DEVICE_DESCRIPTOR used above.
+ MSDN documentation for STORAGE_DEVICE_UNIQUE_IDENTIFIER says:
+ "The device descriptor contains IDs that are extracted from non-VPD
+ inquiry data." This may mean that the serial number (part of
+ SCSI/SAS VPD data) may not always be provided. Therefore a separate
+ call to retrieve STORAGE_DEVICE_DESCRIPTOR only is done above. */
+ STORAGE_PROPERTY_QUERY idquery =
+ { StorageDeviceUniqueIdProperty, PropertyStandardQuery, { 0 } };
+ if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
+ &idquery, sizeof (idquery),
+ ioctl_buf, NT_MAX_PATH,
+ &bytes_read, nullptr))
+ {
+ __seterrno_from_win_error (GetLastError ());
+ debug_printf ("DeviceIoControl (%S, IOCTL_STORAGE_QUERY_PROPERTY,"
+ " {StorageDeviceUniqueIdProperty}): %E", upath);
+ return -1;
+ }
+
+ /* Utilize the DUID as defined by MSDN to generate a hash. */
+ const STORAGE_DEVICE_UNIQUE_IDENTIFIER *id =
+ reinterpret_cast<const STORAGE_DEVICE_UNIQUE_IDENTIFIER *>(ioctl_buf);
+ debug_printf ("STORAGE_DEVICE_UNIQUE_IDENTIFIER.Size: %u", id->Size);
+
+ __int128 hash = 0;
+ for (ULONG i = 0; i < id->Size; i++)
+ hash = ioctl_buf[i] + (hash << 6) + (hash << 16) - hash;
+ __small_sprintf (cp, "_%016_Y%016_X", (unsigned long long) (hash >> 64),
+ (unsigned long long) hash);
+ return 1;
+}
+
+struct by_id_entry
+{
+ char name[NAME_MAX + 1];
+ u_int8_t drive;
+ u_int8_t part;
+};
+
+static int
+by_id_compare_name (const void *a, const void *b)
+{
+ const by_id_entry *ap = reinterpret_cast<const by_id_entry *>(a);
+ const by_id_entry *bp = reinterpret_cast<const by_id_entry *>(b);
+ return strcmp (ap->name, bp->name);
+}
+
+static by_id_entry *
+by_id_realloc (by_id_entry *p, size_t n)
+{
+ void *p2 = realloc (p, n * sizeof (*p));
+ if (!p2)
+ free (p);
+ return reinterpret_cast<by_id_entry *>(p2);
+}
+
+/* Create sorted name -> drive mapping table. Must be freed by caller. */
+static int
+get_by_id_table (by_id_entry * &table)
+{
+ table = nullptr;
+
+ /* Open \Device object directory. */
+ wchar_t wpath[MAX_PATH] = L"\\Device";
+ UNICODE_STRING upath = {14, 16, wpath};
+ OBJECT_ATTRIBUTES attr;
+ InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE, nullptr, nullptr);
+ HANDLE dirhdl;
+ NTSTATUS status = NtOpenDirectoryObject (&dirhdl, DIRECTORY_QUERY, &attr);
+ if (!NT_SUCCESS (status))
+ {
+ debug_printf ("NtOpenDirectoryObject, status %y", status);
+ __seterrno_from_nt_status (status);
+ return -1;
+ }
+
+ /* Limit disk and partition numbers like fhandler_dev::readdir (). */
+ const unsigned max_drive_num = 127, max_part_num = 63;
+
+ unsigned alloc_size = 0, table_size = 0;
+ tmp_pathbuf tp;
+ char *ioctl_buf = tp.c_get ();
+ DIRECTORY_BASIC_INFORMATION *dbi_buf =
+ reinterpret_cast<DIRECTORY_BASIC_INFORMATION *>(tp.w_get ());
+
+ /* Traverse \Device directory ... */
+ bool errno_set = false;
+ HANDLE devhdl = INVALID_HANDLE_VALUE;
+ BOOLEAN restart = TRUE;
+ bool last_run = false;
+ ULONG context = 0;
+ while (!last_run)
+ {
+ if (devhdl != INVALID_HANDLE_VALUE)
+ {
+ /* Close handle from previous run. */
+ NtClose(devhdl);
+ devhdl = INVALID_HANDLE_VALUE;
+ }
+
+ status = NtQueryDirectoryObject (dirhdl, dbi_buf, 65536, FALSE, restart,
+ &context, nullptr);
+ if (!NT_SUCCESS (status))
+ {
+ __seterrno_from_nt_status (status);
+ errno_set = true;
+ debug_printf ("NtQueryDirectoryObject, status %y", status);
+ break;
+ }
+ if (status != STATUS_MORE_ENTRIES)
+ last_run = true;
+ restart = FALSE;
+ for (const DIRECTORY_BASIC_INFORMATION *dbi = dbi_buf;
+ dbi->ObjectName.Length > 0;
+ dbi++)
+ {
+ /* ... and check for a "Harddisk[0-9]*" entry. */
+ if (dbi->ObjectName.Length < 9 * sizeof (WCHAR)
+ || wcsncasecmp (dbi->ObjectName.Buffer, L"Harddisk", 8) != 0
+ || !iswdigit (dbi->ObjectName.Buffer[8]))
+ continue;
+ /* Got it. Now construct the path to the entire disk, which is
+ "\\Device\\HarddiskX\\Partition0", and open the disk with
+ minimum permissions. */
+ unsigned long drive_num = wcstoul (dbi->ObjectName.Buffer + 8,
+ nullptr, 10);
+ if (drive_num > max_drive_num)
+ continue;
+ wcscpy (wpath, dbi->ObjectName.Buffer);
+ PWCHAR wpart = wpath + dbi->ObjectName.Length / sizeof (WCHAR);
+ wcpcpy (wpart, L"\\Partition0");
+ upath.Length = dbi->ObjectName.Length + 22;
+ upath.MaximumLength = upath.Length + sizeof (WCHAR);
+ InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE,
+ dirhdl, nullptr);
+ /* SYNCHRONIZE access is required for IOCTL_STORAGE_QUERY_PROPERTY
+ for drives behind some drivers (nvmestor.sys). */
+ IO_STATUS_BLOCK io;
+ status = NtOpenFile (&devhdl, READ_CONTROL | SYNCHRONIZE, &attr, &io,
+ FILE_SHARE_VALID_FLAGS, 0);
+ if (!NT_SUCCESS (status))
+ {
+ devhdl = INVALID_HANDLE_VALUE;
+ __seterrno_from_nt_status (status);
+ errno_set = true;
+ debug_printf ("NtOpenFile(%S), status %y", &upath, status);
+ continue;
+ }
+
+ /* Add table space for drive, partitions and end marker. */
+ if (alloc_size <= table_size + max_part_num)
+ {
+ alloc_size = table_size + max_part_num + 8;
+ table = by_id_realloc (table, alloc_size);
+ if (!table)
+ {
+ NtClose (devhdl);
+ NtClose (dirhdl);
+ return -1;
+ }
+ }
+
+ /* Fetch storage properties and create the ID string. */
+ int rc = storprop_to_id_name (devhdl, &upath, ioctl_buf,
+ table[table_size].name);
+ if (rc <= 0)
+ {
+ if (rc < 0)
+ errno_set = true;
+ continue;
+ }
+ int drive_index = table_size++;
+ size_t drive_len = strlen(table[drive_index].name);
+ table[drive_index].drive = drive_num;
+ table[drive_index].part = 0;
+
+ /* Fetch drive layout info to get size of all partitions on disk. */
+ DWORD part_cnt = 0;
+ PARTITION_INFORMATION_EX *pix = nullptr;
+ PARTITION_INFORMATION *pi = nullptr;
+ DWORD bytes_read;
+ if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 0,
+ ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
+ {
+ DRIVE_LAYOUT_INFORMATION_EX *pdlix =
+ reinterpret_cast<DRIVE_LAYOUT_INFORMATION_EX *>(ioctl_buf);
+ part_cnt = pdlix->PartitionCount;
+ pix = pdlix->PartitionEntry;
+ }
+ else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, nullptr,
+ 0, ioctl_buf, NT_MAX_PATH, &bytes_read,
+ nullptr))
+ {
+ DRIVE_LAYOUT_INFORMATION *pdli =
+ reinterpret_cast<DRIVE_LAYOUT_INFORMATION *>(ioctl_buf);
+ part_cnt = pdli->PartitionCount;
+ pi = pdli->PartitionEntry;
+ }
+ else
+ debug_printf ("DeviceIoControl(%S, "
+ "IOCTL_DISK_GET_DRIVE_LAYOUT{_EX}): %E", &upath);
+
+ /* Loop over partitions. */
+ if (pix || pi)
+ for (DWORD i = 0; i < part_cnt; i++)
+ {
+ DWORD part_num;
+ if (pix)
+ {
+ part_num = pix->PartitionNumber;
+ ++pix;
+ }
+ else
+ {
+ part_num = pi->PartitionNumber;
+ ++pi;
+ }
+ /* A partition number of 0 denotes an extended partition or a
+ filler entry as described in
+ fhandler_dev_floppy::lock_partition. Just skip. */
+ if (part_num == 0)
+ continue;
+ if (part_num > max_part_num)
+ break;
+ table[table_size] = table[drive_index];
+ __small_sprintf(table[table_size].name + drive_len, "-part%u",
+ part_num);
+ table[table_size].part = part_num;
+ table_size++;
+ }
+ }
+ }
+ if (devhdl != INVALID_HANDLE_VALUE)
+ NtClose(devhdl);
+ NtClose (dirhdl);
+
+ if (!table_size && table)
+ {
+ free (table);
+ table = nullptr;
+ }
+ if (!table)
+ return (errno_set ? -1 : 0);
+
+ /* Sort by name and remove duplicates. */
+ qsort (table, table_size, sizeof (*table), by_id_compare_name);
+ for (unsigned i = 0; i < table_size; i++)
+ {
+ unsigned j = i + 1;
+ while (j < table_size && !strcmp (table[i].name, table[j].name))
+ j++;
+ if (j == i + 1)
+ continue;
+ /* Duplicate(s) found, remove all entries with this name. */
+ debug_printf ("removing duplicates %d-%d: '%s'", i, j - 1, table[i].name);
+ if (j < table_size)
+ memmove (table + i, table + j, (table_size - j) * sizeof (*table));
+ table_size -= j - i;
+ i--;
+ }
+
+ debug_printf ("table_size: %d", table_size);
+ return table_size;
+}
+
+const char dev_disk[] = "/dev/disk";
+const size_t dev_disk_len = sizeof (dev_disk) - 1;
+
+fhandler_dev_disk::fhandler_dev_disk ():
+ fhandler_virtual (),
+ loc (unknown_loc),
+ drive_from_id (-1),
+ part_from_id (0)
+{
+}
+
+void
+fhandler_dev_disk::init_dev_disk ()
+{
+ if (loc != unknown_loc)
+ return;
+
+ static const char by_id[] = "/by-id";
+ const size_t by_id_len = sizeof(by_id) - 1;
+
+ /* Determine location. */
+ const char *path = get_name ();
+ if (!path_prefix_p (dev_disk, path, dev_disk_len, false))
+ loc = invalid_loc; // should not happen
+ else if (!path[dev_disk_len])
+ loc = disk_dir; // "/dev/disk"
+ else if (!path_prefix_p (by_id, path + dev_disk_len, by_id_len, false))
+ loc = invalid_loc; // "/dev/disk/invalid"
+ else if (!path[dev_disk_len + by_id_len])
+ loc = by_id_dir; // "/dev/disk/by-id"
+ else if (strchr (path + dev_disk_len + by_id_len + 1, '/'))
+ loc = invalid_loc; // "/dev/disk/by-id/dir/invalid"
+ else
+ loc = by_id_link; // possible "/dev/disk/by-id/LINK"
+ debug_printf ("'%s': loc %d", path, (int)loc);
+
+ /* Done if "/dev/disk", "/dev/disk/by_id" or invalid. */
+ if (loc != by_id_link)
+ return;
+
+ /* Check whether "/dev/disk/by_id/LINK" exists. */
+ by_id_entry *table;
+ int table_size = get_by_id_table (table);
+ if (!table)
+ {
+ loc = invalid_loc;
+ return;
+ }
+
+ by_id_entry key;
+ strcpy (key.name, path + dev_disk_len + by_id_len + 1);
+ const void *found = bsearch (&key, table, table_size, sizeof (*table),
+ by_id_compare_name);
+ if (found)
+ {
+ /* Preserve drive and partition numbers for fillbuf (). */
+ const by_id_entry *e = reinterpret_cast<const by_id_entry *>(found);
+ drive_from_id = e->drive;
+ part_from_id = e->part;
+ }
+ else
+ loc = invalid_loc;
+ free (table);
+}
+
+virtual_ftype_t
+fhandler_dev_disk::exists ()
+{
+ debug_printf ("exists (%s)", get_name ());
+ ensure_inited ();
+ switch (loc)
+ {
+ case disk_dir:
+ case by_id_dir:
+ return virt_directory;
+ case by_id_link:
+ return virt_symlink;
+ default:
+ return virt_none;
+ }
+}
+
+int
+fhandler_dev_disk::fstat (struct stat *buf)
+{
+ debug_printf ("fstat (%s)", get_name ());
+ ensure_inited ();
+ if (loc == invalid_loc)
+ {
+ set_errno (ENOENT);
+ return -1;
+ }
+
+ fhandler_base::fstat (buf);
+ buf->st_mode = (loc == by_id_link ? S_IFLNK | S_IWUSR | S_IWGRP | S_IWOTH
+ : S_IFDIR) | STD_RBITS | STD_XBITS;
+ buf->st_ino = get_ino ();
+ return 0;
+}
+
+static inline by_id_entry **
+dir_id_table (DIR *dir)
+{
+ return reinterpret_cast<by_id_entry **>(&dir->__d_internal);
+}
+
+DIR *
+fhandler_dev_disk::opendir (int fd)
+{
+ ensure_inited ();
+ if (!(loc == disk_dir || loc == by_id_dir))
+ {
+ set_errno (ENOTDIR);
+ return nullptr;
+ }
+
+ by_id_entry *table = nullptr;
+ if (loc == by_id_dir)
+ {
+ int table_size = get_by_id_table (table);
+ if (table_size < 0)
+ return nullptr; /* errno is set. */
+ if (table)
+ {
+ /* Shrink to required table_size. */
+ table = by_id_realloc (table, table_size + 1);
+ if (!table)
+ return nullptr; /* Should not happen. */
+ /* Mark end of table for readdir (). */
+ table[table_size].name[0] = '\0';
+ }
+ }
+
+ DIR *dir = fhandler_virtual::opendir (fd);
+ if (!dir)
+ {
+ free (table);
+ return nullptr;
+ }
+ dir->__flags = dirent_saw_dot | dirent_saw_dot_dot;
+ *dir_id_table (dir) = table;
+ return dir;
+}
+
+int
+fhandler_dev_disk::closedir (DIR *dir)
+{
+ free (*dir_id_table (dir));
+ return fhandler_virtual::closedir (dir);
+}
+
+int
+fhandler_dev_disk::readdir (DIR *dir, dirent *de)
+{
+ int res;
+ if (dir->__d_position < 2)
+ {
+ de->d_name[0] = '.';
+ de->d_name[1] = (dir->__d_position ? '.' : '\0');
+ de->d_name[2] = '\0';
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == disk_dir && dir->__d_position == 2)
+ {
+ strcpy (de->d_name, "by-id");
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == by_id_dir && *dir_id_table (dir))
+ {
+ const char *name = (*dir_id_table (dir))[dir->__d_position - 2].name;
+ if (name[0])
+ {
+ strcpy (de->d_name, name);
+ de->d_type = DT_LNK;
+ dir->__d_position++;
+ res = 0;
+ }
+ else
+ res = ENMFILE;
+ }
+ else
+ res = ENMFILE;
+
+ syscall_printf ("%d = readdir(%p, %p) (%s)", res, dir, de,
+ (!res ? de->d_name : ""));
+ return res;
+}
+
+int
+fhandler_dev_disk::open (int flags, mode_t mode)
+{
+ ensure_inited ();
+ int err = 0;
+ if (!fhandler_virtual::open (flags, mode))
+ err = -1;
+ else if (loc == disk_dir || loc == by_id_dir)
+ {
+ if ((flags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
+ err = EEXIST;
+ else if (flags & O_WRONLY)
+ err = EISDIR;
+ else
+ diropen = true;
+ }
+ /* else if (loc == by_id_link) { } */ /* should not happen */
+ else /* (loc == invalid_loc) */
+ {
+ if (flags & O_CREAT)
+ err = EROFS;
+ else
+ err = ENOENT;
+ }
+
+ int res;
+ if (!err)
+ {
+ nohandle (true);
+ set_open_status ();
+ res = 1;
+ }
+ else
+ {
+ if (err > 0)
+ set_errno (err);
+ res = 0;
+ }
+
+ syscall_printf ("%d = fhandler_dev_disk::open(%y, 0%o)", res, flags, mode);
+ return res;
+}
+
+bool
+fhandler_dev_disk::fill_filebuf ()
+{
+ debug_printf ("fill_filebuf (%s)", get_name ());
+ ensure_inited ();
+ if (!(loc == by_id_link && drive_from_id >= 0))
+ return false;
+
+ char buf[32];
+ int len;
+ if (drive_from_id + 'a' <= 'z')
+ len = __small_sprintf (buf, "../../sd%c", drive_from_id + 'a');
+ else
+ len = __small_sprintf (buf, "../../sd%c%c",
+ drive_from_id / ('z' - 'a' + 1) - 1 + 'a',
+ drive_from_id % ('z' - 'a' + 1) + 'a');
+ if (part_from_id)
+ __small_sprintf (buf + len, "%d", part_from_id);
+
+ if (filebuf)
+ cfree (filebuf);
+ filebuf = cstrdup (buf);
+ return true;
+}
diff --git a/winsup/cygwin/local_includes/devices.h b/winsup/cygwin/local_includes/devices.h
index 10035263d..1e035f9d6 100644
--- a/winsup/cygwin/local_includes/devices.h
+++ b/winsup/cygwin/local_includes/devices.h
@@ -71,6 +71,7 @@ enum fh_devices
FH_DEV = FHDEV (DEV_VIRTFS_MAJOR, 193),
FH_CYGDRIVE= FHDEV (DEV_VIRTFS_MAJOR, 192),
FH_DEV_FD = FHDEV (DEV_VIRTFS_MAJOR, 191),
+ FH_DEV_DISK= FHDEV (DEV_VIRTFS_MAJOR, 190),
FH_SIGNALFD= FHDEV (DEV_VIRTFS_MAJOR, 13),
FH_TIMERFD = FHDEV (DEV_VIRTFS_MAJOR, 14),
@@ -415,6 +416,8 @@ extern const _device dev_piper_storage;
#define piper_dev ((device *) &dev_piper_storage)
extern const _device dev_pipew_storage;
#define pipew_dev ((device *) &dev_pipew_storage)
+extern const _device dev_dev_disk_storage;
+#define dev_disk_dev ((device *) &dev_dev_disk_storage)
extern const _device dev_proc_storage;
#define proc_dev ((device *) &dev_proc_storage)
extern const _device dev_dev_storage;
@@ -439,7 +442,8 @@ extern const _device dev_fs_storage;
#define isprocsys_dev(devn) (devn == FH_PROCSYS)
#define isvirtual_dev(devn) \
- (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE || devn == FH_DEV_FD)
+ (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE \
+ || devn == FH_DEV_FD || devn == FH_DEV_DISK)
#define iscons_dev(n) \
((device::major ((dev_t) (n)) == DEV_CONS_MAJOR) \
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index 212c22344..cd61aadf8 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -40,6 +40,8 @@ details. */
extern const char *windows_device_names[];
extern struct __cygwin_perfile *perfile_table;
#define __fmode (*(user_data->fmode_ptr))
+extern const char dev_disk[];
+extern const size_t dev_disk_len;
extern const char proc[];
extern const size_t proc_len;
extern const char procsys[];
@@ -3190,6 +3192,50 @@ class fhandler_procnet: public fhandler_proc
}
};
+class fhandler_dev_disk: public fhandler_virtual
+{
+ enum dev_disk_location {
+ unknown_loc, invalid_loc, disk_dir, by_id_dir, by_id_link
+ };
+ dev_disk_location loc;
+
+ void init_dev_disk ();
+ void ensure_inited ()
+ {
+ if (loc == unknown_loc)
+ init_dev_disk ();
+ }
+
+ int drive_from_id;
+ int part_from_id;
+
+ public:
+ fhandler_dev_disk ();
+ fhandler_dev_disk (void *) {}
+ virtual_ftype_t exists();
+ DIR *opendir (int fd);
+ int closedir (DIR *);
+ int readdir (DIR *, dirent *);
+ int open (int flags, mode_t mode = 0);
+ int fstat (struct stat *buf);
+ bool fill_filebuf ();
+
+ void copy_from (fhandler_base *x)
+ {
+ pc.free_strings ();
+ *this = *reinterpret_cast<fhandler_dev_disk *> (x);
+ _copy_from_reset_helper ();
+ }
+
+ fhandler_dev_disk *clone (cygheap_types malloc_type = HEAP_FHANDLER)
+ {
+ void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_dev_disk));
+ fhandler_dev_disk *fh = new (ptr) fhandler_dev_disk (ptr);
+ fh->copy_from (this);
+ return fh;
+ }
+};
+
class fhandler_dev_fd: public fhandler_virtual
{
public:
@@ -3416,6 +3462,7 @@ typedef union
char __dev_raw[sizeof (fhandler_dev_raw)];
char __dev_tape[sizeof (fhandler_dev_tape)];
char __dev_zero[sizeof (fhandler_dev_zero)];
+ char __dev_disk[sizeof (fhandler_dev_disk)];
char __dev_fd[sizeof (fhandler_dev_fd)];
char __disk_file[sizeof (fhandler_disk_file)];
char __fifo[sizeof (fhandler_fifo)];
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index 36ab042a7..13ace6250 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -35,6 +35,9 @@ details. */
(path[mount_table->cygdrive_len + 1] == '/' || \
!path[mount_table->cygdrive_len + 1]))
+#define isdev_disk(path) \
+ (path_prefix_p (dev_disk, (path), dev_disk_len, false))
+
#define isproc(path) \
(path_prefix_p (proc, (path), proc_len, false))
@@ -685,6 +688,13 @@ mount_info::conv_to_win32_path (const char *src_path, char *dst, device& dev,
/* Go through chroot check */
goto out;
}
+ if (isdev_disk (src_path))
+ {
+ dev = *dev_disk_dev;
+ *flags = 0;
+ strcpy (dst, src_path);
+ goto out;
+ }
if (isproc (src_path))
{
dev = *proc_dev;
--
2.42.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-04 15:53 ` Christian Franke
@ 2023-11-04 20:51 ` Corinna Vinschen
2023-11-05 15:45 ` Christian Franke
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-04 20:51 UTC (permalink / raw)
To: cygwin-patches
Hi Christian,
patch looks pretty good to me. Just two minor points:
> + /* Traverse \Device directory ... */
> + bool errno_set = false;
> + HANDLE devhdl = INVALID_HANDLE_VALUE;
INVALID_HANDLE_VALUE is a Win32 API concept and is only returned by
CreateFile and friends. The native NT API doesn't know
INVALID_HANDLE_VALUE and, fortunately, only uses NULL. I think it's
puzzeling to use INVALID_HANDLE_VALUE in a native NT context. So, would
you mind to just use NULL (or nullptr) instead? As in...
> + if (devhdl != INVALID_HANDLE_VALUE)
if (devhdl)
Sounds easier to me :)
> + /* Fetch drive layout info to get size of all partitions on disk. */
> + DWORD part_cnt = 0;
> + PARTITION_INFORMATION_EX *pix = nullptr;
> + PARTITION_INFORMATION *pi = nullptr;
> + DWORD bytes_read;
> + if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 0,
> + ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
> + {
> + DRIVE_LAYOUT_INFORMATION_EX *pdlix =
> + reinterpret_cast<DRIVE_LAYOUT_INFORMATION_EX *>(ioctl_buf);
> + part_cnt = pdlix->PartitionCount;
> + pix = pdlix->PartitionEntry;
> + }
> + else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, nullptr,
Do we really still need IOCTL_DISK_GET_DRIVE_LAYOUT w/o _EX?
fhandler_proc still uses it, too, but the EX variation is available
since XP. I can't imagine that a fallback to the non-EX-version is still
necessary. If you like you could drop it immediately, or we can drop
both usages as a followup patch.
Last, but not least, do you see a chance to add any other /dev/disk
subdir? by-partuuid, perhaps?
Thanks,
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-04 20:51 ` Corinna Vinschen
@ 2023-11-05 15:45 ` Christian Franke
2023-11-05 19:59 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-11-05 15:45 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 2575 bytes --]
Hi Corinna,
Corinna Vinschen wrote:
> Hi Christian,
>
> patch looks pretty good to me. Just two minor points:
>
>> + /* Traverse \Device directory ... */
>> + bool errno_set = false;
>> + HANDLE devhdl = INVALID_HANDLE_VALUE;
> INVALID_HANDLE_VALUE is a Win32 API concept and is only returned by
> CreateFile and friends. The native NT API doesn't know
> INVALID_HANDLE_VALUE and, fortunately, only uses NULL. I think it's
> puzzeling to use INVALID_HANDLE_VALUE in a native NT context. So, would
> you mind to just use NULL (or nullptr) instead? As in...
>
>> + if (devhdl != INVALID_HANDLE_VALUE)
> if (devhdl)
>
> Sounds easier to me :)
Yes...done.
>> + /* Fetch drive layout info to get size of all partitions on disk. */
>> + DWORD part_cnt = 0;
>> + PARTITION_INFORMATION_EX *pix = nullptr;
>> + PARTITION_INFORMATION *pi = nullptr;
>> + DWORD bytes_read;
>> + if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 0,
>> + ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
>> + {
>> + DRIVE_LAYOUT_INFORMATION_EX *pdlix =
>> + reinterpret_cast<DRIVE_LAYOUT_INFORMATION_EX *>(ioctl_buf);
>> + part_cnt = pdlix->PartitionCount;
>> + pix = pdlix->PartitionEntry;
>> + }
>> + else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, nullptr,
> Do we really still need IOCTL_DISK_GET_DRIVE_LAYOUT w/o _EX?
Possibly not. I borrowed this code from code behind /proc/partitions.
> fhandler_proc still uses it, too, but the EX variation is available
> since XP. I can't imagine that a fallback to the non-EX-version is still
> necessary. If you like you could drop it immediately, or we can drop
> both usages as a followup patch.
Old IOCTL dropped and code simplified.
> Last, but not least, do you see a chance to add any other /dev/disk
> subdir? by-partuuid, perhaps?
Possibly, but not very soon. I'm not yet sure which API functions could
be used.
Some early draft ideas:
/dev/disk/by-partuid (Partition UUID -> device)
GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)
/dev/disk/by-uuid (Windows Volume UUID -> device)
Vol_UUID1 -> ../../sdXN (disk volume)
Vol_UUID2 -> ../../scd0 (CD/DVD drive volume)
Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID} (others, e.g. VeraCrypt
volume)
/dev/disk/by-drive (Cygwin specific: drive letter -> volume)
c -> ../by-uuid/UUID (if UUID available)
x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume
Shadow Copy)
Christian
[-- Attachment #2: 0001-Cygwin-Add-dev-disk-by-id-symlinks.patch --]
[-- Type: text/plain, Size: 25330 bytes --]
From aa8c35e041ffe5a4b06104c122d9ba1fdc492683 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sun, 5 Nov 2023 15:54:23 +0100
Subject: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
The new directory '/dev/disk/by-id' provides symlinks for each
disk and its partitions:
'BUSTYPE-[VENDOR_]PRODUCT_SERIAL[-partN]' -> '../../sdX[N]'.
This is based on strings provided by STORAGE_DEVICE_DESCRIPTOR.
If this information is too short, a 128-bit hash of the
STORAGE_DEVICE_UNIQUE_IDENTIFIER raw data is added.
Administrator privileges are not required.
Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
winsup/cygwin/Makefile.am | 1 +
winsup/cygwin/devices.in | 4 +
winsup/cygwin/dtable.cc | 3 +
winsup/cygwin/fhandler/dev_disk.cc | 621 ++++++++++++++++++++++++
winsup/cygwin/local_includes/devices.h | 6 +-
winsup/cygwin/local_includes/fhandler.h | 47 ++
winsup/cygwin/mount.cc | 10 +
7 files changed, 691 insertions(+), 1 deletion(-)
create mode 100644 winsup/cygwin/fhandler/dev_disk.cc
diff --git a/winsup/cygwin/Makefile.am b/winsup/cygwin/Makefile.am
index 64b252a22..376c79fc3 100644
--- a/winsup/cygwin/Makefile.am
+++ b/winsup/cygwin/Makefile.am
@@ -84,6 +84,7 @@ FHANDLER_FILES= \
fhandler/console.cc \
fhandler/cygdrive.cc \
fhandler/dev.cc \
+ fhandler/dev_disk.cc \
fhandler/dev_fd.cc \
fhandler/disk_file.cc \
fhandler/dsp.cc \
diff --git a/winsup/cygwin/devices.in b/winsup/cygwin/devices.in
index 2545dd85e..48d3843fe 100644
--- a/winsup/cygwin/devices.in
+++ b/winsup/cygwin/devices.in
@@ -115,6 +115,9 @@ const _device dev_cygdrive_storage =
const _device dev_fs_storage =
{"", {FH_FS}, "", exists};
+const _device dev_dev_disk_storage =
+ {"", {FH_DEV_DISK}, "", exists};
+
const _device dev_proc_storage =
{"", {FH_PROC}, "", exists};
@@ -173,6 +176,7 @@ const _device dev_error_storage =
the POSIX namespace. */
%%
"/dev", BRACK(FH_DEV), "", exists, S_IFDIR
+"/dev/disk", BRACK(FH_DEV_DISK), "", exists, S_IFDIR
"/dev/tty", BRACK(FH_TTY), "/dev/tty", exists, S_IFCHR
"/dev/pty%(0-127)d", BRACK(FHDEV(DEV_PTYS_MAJOR, {$1})), "/dev/pty{$1}", exists_pty, S_IFCHR, =ptys_dev
":ptym%(0-127)d", BRACK(FHDEV(DEV_PTYM_MAJOR, {$1})), "/dev/ptym{$1}", exists_internal, S_IFCHR, =ptym_dev
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 21d525389..9508f3e0b 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -585,6 +585,9 @@ fh_alloc (path_conv& pc)
case FH_DEV:
fh = cnew (fhandler_dev);
break;
+ case FH_DEV_DISK:
+ fh = cnew (fhandler_dev_disk);
+ break;
case FH_DEV_FD:
fh = cnew (fhandler_dev_fd);
break;
diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
new file mode 100644
index 000000000..9a1cae5eb
--- /dev/null
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -0,0 +1,621 @@
+/* fhandler/dev_disk.cc: fhandler for the /dev/disk/by-id/... symlinks.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license. Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "winsup.h"
+#include "path.h"
+#include "fhandler.h"
+#include "tls_pbuf.h"
+#include <storduid.h>
+#include <wctype.h>
+#include <winioctl.h>
+
+/* Replace non-printing and unexpected characters, remove trailing spaces,
+ return remaining string length. */
+static int
+sanitize_id_string (char *s)
+{
+ int lastspace = -1, i;
+ for (i = 0; s[i]; i++)
+ {
+ char c = s[i];
+ if (c != ' ')
+ lastspace = -1;
+ else if (lastspace < 0)
+ lastspace = i;
+ if (('0' <= c && c <= '9') || c == '.' || c == '-'
+ || ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'))
+ continue;
+ s[i] = '_';
+ }
+ if (lastspace >= 0)
+ s[(i = lastspace)] = '\0';
+ return i;
+}
+
+/* Fetch storage properties and create the ID string.
+ returns: 1: success, 0: device ignored, -1: IoControl error. */
+static int
+storprop_to_id_name (HANDLE devhdl, const UNICODE_STRING *upath,
+ char *ioctl_buf, char (& name)[NAME_MAX + 1])
+{
+ DWORD bytes_read;
+ STORAGE_PROPERTY_QUERY descquery =
+ { StorageDeviceProperty, PropertyStandardQuery, { 0 } };
+ if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
+ &descquery, sizeof (descquery),
+ ioctl_buf, NT_MAX_PATH,
+ &bytes_read, nullptr))
+ {
+ __seterrno_from_win_error (GetLastError ());
+ debug_printf ("DeviceIoControl (%S, IOCTL_STORAGE_QUERY_PROPERTY,"
+ " {StorageDeviceProperty}): %E", upath);
+ return -1;
+ }
+
+ const STORAGE_DEVICE_DESCRIPTOR *desc =
+ reinterpret_cast<const STORAGE_DEVICE_DESCRIPTOR *>(ioctl_buf);
+ int vendor_len = 0, product_len = 0, serial_len = 0;
+ if (desc->VendorIdOffset)
+ vendor_len = sanitize_id_string (ioctl_buf + desc->VendorIdOffset);
+ if (desc->ProductIdOffset)
+ product_len = sanitize_id_string (ioctl_buf + desc->ProductIdOffset);
+ if (desc->SerialNumberOffset)
+ serial_len = sanitize_id_string (ioctl_buf + desc->SerialNumberOffset);
+
+ /* Ignore drive if information is empty or too long (should not happen). */
+ if (!((vendor_len || product_len) && (20/*bustype*/ + vendor_len + 1 +
+ product_len + 1 + serial_len + 1 + 34/*hash*/ + 1 + 10/*partN*/
+ < (int) sizeof (name))))
+ return 0;
+
+ /* Translate bus types. */
+ const char *bus;
+ switch (desc->BusType)
+ {
+ case BusTypeAta: bus = "ata-"; break;
+ case BusTypeFibre: bus = "fibre-"; break;
+ case BusTypeNvme: bus = "nvme-"; break;
+ case BusTypeRAID: bus = "raid-"; break;
+ case BusTypeSas: bus = "sas-"; break;
+ case BusTypeSata: bus = "sata-"; break;
+ case BusTypeScsi: bus = "scsi-"; break;
+ case BusTypeUsb: bus = "usb-"; break;
+ case BusTypeVirtual: bus = "virtual-"; break;
+ default: bus = nullptr; break;
+ }
+
+ /* Create "BUSTYPE-[VENDOR_]PRODUCT[_SERIAL]" string. */
+ char * cp = name;
+ if (bus)
+ cp = stpcpy (cp, bus);
+ else
+ cp += __small_sprintf (cp, "bustype%02_y-", desc->BusType);
+
+ if (vendor_len)
+ cp = stpcpy (cp, ioctl_buf + desc->VendorIdOffset);
+ if (product_len)
+ {
+ if (vendor_len)
+ cp = stpcpy (cp, "_");
+ cp = stpcpy (cp, ioctl_buf + desc->ProductIdOffset);
+ }
+ if (serial_len)
+ {
+ cp = stpcpy (cp, "_");
+ cp = stpcpy (cp, ioctl_buf + desc->SerialNumberOffset);
+ }
+
+ /* Add hash if information is too short (e.g. missing serial number). */
+ bool add_hash = !(4 <= vendor_len + product_len && 4 <= serial_len);
+ debug_printf ("%S: bustype: %02_y, add_hash: %d, id: '%s' '%s' '%s' ",
+ upath, (unsigned) desc->BusType, (int) add_hash,
+ (vendor_len ? ioctl_buf + desc->VendorIdOffset : ""),
+ (product_len ? ioctl_buf + desc->ProductIdOffset : ""),
+ (serial_len ? ioctl_buf + desc->SerialNumberOffset : ""));
+ if (!add_hash)
+ return 1;
+
+ /* The call below also returns the STORAGE_DEVICE_DESCRIPTOR used above.
+ MSDN documentation for STORAGE_DEVICE_UNIQUE_IDENTIFIER says:
+ "The device descriptor contains IDs that are extracted from non-VPD
+ inquiry data." This may mean that the serial number (part of
+ SCSI/SAS VPD data) may not always be provided. Therefore a separate
+ call to retrieve STORAGE_DEVICE_DESCRIPTOR only is done above. */
+ STORAGE_PROPERTY_QUERY idquery =
+ { StorageDeviceUniqueIdProperty, PropertyStandardQuery, { 0 } };
+ if (!DeviceIoControl (devhdl, IOCTL_STORAGE_QUERY_PROPERTY,
+ &idquery, sizeof (idquery),
+ ioctl_buf, NT_MAX_PATH,
+ &bytes_read, nullptr))
+ {
+ __seterrno_from_win_error (GetLastError ());
+ debug_printf ("DeviceIoControl (%S, IOCTL_STORAGE_QUERY_PROPERTY,"
+ " {StorageDeviceUniqueIdProperty}): %E", upath);
+ return -1;
+ }
+
+ /* Utilize the DUID as defined by MSDN to generate a hash. */
+ const STORAGE_DEVICE_UNIQUE_IDENTIFIER *id =
+ reinterpret_cast<const STORAGE_DEVICE_UNIQUE_IDENTIFIER *>(ioctl_buf);
+ debug_printf ("STORAGE_DEVICE_UNIQUE_IDENTIFIER.Size: %u", id->Size);
+
+ __int128 hash = 0;
+ for (ULONG i = 0; i < id->Size; i++)
+ hash = ioctl_buf[i] + (hash << 6) + (hash << 16) - hash;
+ __small_sprintf (cp, "_%016_Y%016_X", (unsigned long long) (hash >> 64),
+ (unsigned long long) hash);
+ return 1;
+}
+
+struct by_id_entry
+{
+ char name[NAME_MAX + 1];
+ u_int8_t drive;
+ u_int8_t part;
+};
+
+static int
+by_id_compare_name (const void *a, const void *b)
+{
+ const by_id_entry *ap = reinterpret_cast<const by_id_entry *>(a);
+ const by_id_entry *bp = reinterpret_cast<const by_id_entry *>(b);
+ return strcmp (ap->name, bp->name);
+}
+
+static by_id_entry *
+by_id_realloc (by_id_entry *p, size_t n)
+{
+ void *p2 = realloc (p, n * sizeof (*p));
+ if (!p2)
+ free (p);
+ return reinterpret_cast<by_id_entry *>(p2);
+}
+
+/* Create sorted name -> drive mapping table. Must be freed by caller. */
+static int
+get_by_id_table (by_id_entry * &table)
+{
+ table = nullptr;
+
+ /* Open \Device object directory. */
+ wchar_t wpath[MAX_PATH] = L"\\Device";
+ UNICODE_STRING upath = {14, 16, wpath};
+ OBJECT_ATTRIBUTES attr;
+ InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE, nullptr, nullptr);
+ HANDLE dirhdl;
+ NTSTATUS status = NtOpenDirectoryObject (&dirhdl, DIRECTORY_QUERY, &attr);
+ if (!NT_SUCCESS (status))
+ {
+ debug_printf ("NtOpenDirectoryObject, status %y", status);
+ __seterrno_from_nt_status (status);
+ return -1;
+ }
+
+ /* Limit disk and partition numbers like fhandler_dev::readdir (). */
+ const unsigned max_drive_num = 127, max_part_num = 63;
+
+ unsigned alloc_size = 0, table_size = 0;
+ tmp_pathbuf tp;
+ char *ioctl_buf = tp.c_get ();
+ DIRECTORY_BASIC_INFORMATION *dbi_buf =
+ reinterpret_cast<DIRECTORY_BASIC_INFORMATION *>(tp.w_get ());
+
+ /* Traverse \Device directory ... */
+ bool errno_set = false;
+ HANDLE devhdl = nullptr;
+ BOOLEAN restart = TRUE;
+ bool last_run = false;
+ ULONG context = 0;
+ while (!last_run)
+ {
+ if (devhdl)
+ {
+ /* Close handle from previous run. */
+ NtClose(devhdl);
+ devhdl = nullptr;
+ }
+
+ status = NtQueryDirectoryObject (dirhdl, dbi_buf, 65536, FALSE, restart,
+ &context, nullptr);
+ if (!NT_SUCCESS (status))
+ {
+ __seterrno_from_nt_status (status);
+ errno_set = true;
+ debug_printf ("NtQueryDirectoryObject, status %y", status);
+ break;
+ }
+ if (status != STATUS_MORE_ENTRIES)
+ last_run = true;
+ restart = FALSE;
+ for (const DIRECTORY_BASIC_INFORMATION *dbi = dbi_buf;
+ dbi->ObjectName.Length > 0;
+ dbi++)
+ {
+ /* ... and check for a "Harddisk[0-9]*" entry. */
+ if (dbi->ObjectName.Length < 9 * sizeof (WCHAR)
+ || wcsncasecmp (dbi->ObjectName.Buffer, L"Harddisk", 8) != 0
+ || !iswdigit (dbi->ObjectName.Buffer[8]))
+ continue;
+ /* Got it. Now construct the path to the entire disk, which is
+ "\\Device\\HarddiskX\\Partition0", and open the disk with
+ minimum permissions. */
+ unsigned long drive_num = wcstoul (dbi->ObjectName.Buffer + 8,
+ nullptr, 10);
+ if (drive_num > max_drive_num)
+ continue;
+ wcscpy (wpath, dbi->ObjectName.Buffer);
+ PWCHAR wpart = wpath + dbi->ObjectName.Length / sizeof (WCHAR);
+ wcpcpy (wpart, L"\\Partition0");
+ upath.Length = dbi->ObjectName.Length + 22;
+ upath.MaximumLength = upath.Length + sizeof (WCHAR);
+ InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE,
+ dirhdl, nullptr);
+ /* SYNCHRONIZE access is required for IOCTL_STORAGE_QUERY_PROPERTY
+ for drives behind some drivers (nvmestor.sys). */
+ IO_STATUS_BLOCK io;
+ status = NtOpenFile (&devhdl, READ_CONTROL | SYNCHRONIZE, &attr, &io,
+ FILE_SHARE_VALID_FLAGS, 0);
+ if (!NT_SUCCESS (status))
+ {
+ devhdl = nullptr;
+ __seterrno_from_nt_status (status);
+ errno_set = true;
+ debug_printf ("NtOpenFile(%S), status %y", &upath, status);
+ continue;
+ }
+
+ /* Add table space for drive, partitions and end marker. */
+ if (alloc_size <= table_size + max_part_num)
+ {
+ alloc_size = table_size + max_part_num + 8;
+ table = by_id_realloc (table, alloc_size);
+ if (!table)
+ {
+ NtClose (devhdl);
+ NtClose (dirhdl);
+ return -1;
+ }
+ }
+
+ /* Fetch storage properties and create the ID string. */
+ int rc = storprop_to_id_name (devhdl, &upath, ioctl_buf,
+ table[table_size].name);
+ if (rc <= 0)
+ {
+ if (rc < 0)
+ errno_set = true;
+ continue;
+ }
+ int drive_index = table_size++;
+ size_t drive_len = strlen(table[drive_index].name);
+ table[drive_index].drive = drive_num;
+ table[drive_index].part = 0;
+
+ /* Fetch drive layout info to get size of all partitions on disk. */
+ DWORD bytes_read;
+ if (!DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 0,
+ ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
+ {
+ debug_printf ("DeviceIoControl(%S, "
+ "IOCTL_DISK_GET_DRIVE_LAYOUT_EX): %E", &upath);
+ continue;
+ }
+
+ /* Loop over partitions. */
+ const DRIVE_LAYOUT_INFORMATION_EX *dlix =
+ reinterpret_cast<const DRIVE_LAYOUT_INFORMATION_EX *>(ioctl_buf);
+ for (DWORD i = 0; i < dlix->PartitionCount; i++)
+ {
+ DWORD part_num = dlix->PartitionEntry[i].PartitionNumber;
+ /* A partition number of 0 denotes an extended partition or a
+ filler entry as described in
+ fhandler_dev_floppy::lock_partition. Just skip. */
+ if (part_num == 0)
+ continue;
+ if (part_num > max_part_num)
+ break;
+ table[table_size] = table[drive_index];
+ __small_sprintf(table[table_size].name + drive_len, "-part%u",
+ part_num);
+ table[table_size].part = part_num;
+ table_size++;
+ }
+ }
+ }
+ if (devhdl)
+ NtClose(devhdl);
+ NtClose (dirhdl);
+
+ if (!table_size && table)
+ {
+ free (table);
+ table = nullptr;
+ }
+ if (!table)
+ return (errno_set ? -1 : 0);
+
+ /* Sort by name and remove duplicates. */
+ qsort (table, table_size, sizeof (*table), by_id_compare_name);
+ for (unsigned i = 0; i < table_size; i++)
+ {
+ unsigned j = i + 1;
+ while (j < table_size && !strcmp (table[i].name, table[j].name))
+ j++;
+ if (j == i + 1)
+ continue;
+ /* Duplicate(s) found, remove all entries with this name. */
+ debug_printf ("removing duplicates %d-%d: '%s'", i, j - 1, table[i].name);
+ if (j < table_size)
+ memmove (table + i, table + j, (table_size - j) * sizeof (*table));
+ table_size -= j - i;
+ i--;
+ }
+
+ debug_printf ("table_size: %d", table_size);
+ return table_size;
+}
+
+const char dev_disk[] = "/dev/disk";
+const size_t dev_disk_len = sizeof (dev_disk) - 1;
+
+fhandler_dev_disk::fhandler_dev_disk ():
+ fhandler_virtual (),
+ loc (unknown_loc),
+ drive_from_id (-1),
+ part_from_id (0)
+{
+}
+
+void
+fhandler_dev_disk::init_dev_disk ()
+{
+ if (loc != unknown_loc)
+ return;
+
+ static const char by_id[] = "/by-id";
+ const size_t by_id_len = sizeof(by_id) - 1;
+
+ /* Determine location. */
+ const char *path = get_name ();
+ if (!path_prefix_p (dev_disk, path, dev_disk_len, false))
+ loc = invalid_loc; // should not happen
+ else if (!path[dev_disk_len])
+ loc = disk_dir; // "/dev/disk"
+ else if (!path_prefix_p (by_id, path + dev_disk_len, by_id_len, false))
+ loc = invalid_loc; // "/dev/disk/invalid"
+ else if (!path[dev_disk_len + by_id_len])
+ loc = by_id_dir; // "/dev/disk/by-id"
+ else if (strchr (path + dev_disk_len + by_id_len + 1, '/'))
+ loc = invalid_loc; // "/dev/disk/by-id/dir/invalid"
+ else
+ loc = by_id_link; // possible "/dev/disk/by-id/LINK"
+ debug_printf ("'%s': loc %d", path, (int)loc);
+
+ /* Done if "/dev/disk", "/dev/disk/by_id" or invalid. */
+ if (loc != by_id_link)
+ return;
+
+ /* Check whether "/dev/disk/by_id/LINK" exists. */
+ by_id_entry *table;
+ int table_size = get_by_id_table (table);
+ if (!table)
+ {
+ loc = invalid_loc;
+ return;
+ }
+
+ by_id_entry key;
+ strcpy (key.name, path + dev_disk_len + by_id_len + 1);
+ const void *found = bsearch (&key, table, table_size, sizeof (*table),
+ by_id_compare_name);
+ if (found)
+ {
+ /* Preserve drive and partition numbers for fillbuf (). */
+ const by_id_entry *e = reinterpret_cast<const by_id_entry *>(found);
+ drive_from_id = e->drive;
+ part_from_id = e->part;
+ }
+ else
+ loc = invalid_loc;
+ free (table);
+}
+
+virtual_ftype_t
+fhandler_dev_disk::exists ()
+{
+ debug_printf ("exists (%s)", get_name ());
+ ensure_inited ();
+ switch (loc)
+ {
+ case disk_dir:
+ case by_id_dir:
+ return virt_directory;
+ case by_id_link:
+ return virt_symlink;
+ default:
+ return virt_none;
+ }
+}
+
+int
+fhandler_dev_disk::fstat (struct stat *buf)
+{
+ debug_printf ("fstat (%s)", get_name ());
+ ensure_inited ();
+ if (loc == invalid_loc)
+ {
+ set_errno (ENOENT);
+ return -1;
+ }
+
+ fhandler_base::fstat (buf);
+ buf->st_mode = (loc == by_id_link ? S_IFLNK | S_IWUSR | S_IWGRP | S_IWOTH
+ : S_IFDIR) | STD_RBITS | STD_XBITS;
+ buf->st_ino = get_ino ();
+ return 0;
+}
+
+static inline by_id_entry **
+dir_id_table (DIR *dir)
+{
+ return reinterpret_cast<by_id_entry **>(&dir->__d_internal);
+}
+
+DIR *
+fhandler_dev_disk::opendir (int fd)
+{
+ ensure_inited ();
+ if (!(loc == disk_dir || loc == by_id_dir))
+ {
+ set_errno (ENOTDIR);
+ return nullptr;
+ }
+
+ by_id_entry *table = nullptr;
+ if (loc == by_id_dir)
+ {
+ int table_size = get_by_id_table (table);
+ if (table_size < 0)
+ return nullptr; /* errno is set. */
+ if (table)
+ {
+ /* Shrink to required table_size. */
+ table = by_id_realloc (table, table_size + 1);
+ if (!table)
+ return nullptr; /* Should not happen. */
+ /* Mark end of table for readdir (). */
+ table[table_size].name[0] = '\0';
+ }
+ }
+
+ DIR *dir = fhandler_virtual::opendir (fd);
+ if (!dir)
+ {
+ free (table);
+ return nullptr;
+ }
+ dir->__flags = dirent_saw_dot | dirent_saw_dot_dot;
+ *dir_id_table (dir) = table;
+ return dir;
+}
+
+int
+fhandler_dev_disk::closedir (DIR *dir)
+{
+ free (*dir_id_table (dir));
+ return fhandler_virtual::closedir (dir);
+}
+
+int
+fhandler_dev_disk::readdir (DIR *dir, dirent *de)
+{
+ int res;
+ if (dir->__d_position < 2)
+ {
+ de->d_name[0] = '.';
+ de->d_name[1] = (dir->__d_position ? '.' : '\0');
+ de->d_name[2] = '\0';
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == disk_dir && dir->__d_position == 2)
+ {
+ strcpy (de->d_name, "by-id");
+ de->d_type = DT_DIR;
+ dir->__d_position++;
+ res = 0;
+ }
+ else if (loc == by_id_dir && *dir_id_table (dir))
+ {
+ const char *name = (*dir_id_table (dir))[dir->__d_position - 2].name;
+ if (name[0])
+ {
+ strcpy (de->d_name, name);
+ de->d_type = DT_LNK;
+ dir->__d_position++;
+ res = 0;
+ }
+ else
+ res = ENMFILE;
+ }
+ else
+ res = ENMFILE;
+
+ syscall_printf ("%d = readdir(%p, %p) (%s)", res, dir, de,
+ (!res ? de->d_name : ""));
+ return res;
+}
+
+int
+fhandler_dev_disk::open (int flags, mode_t mode)
+{
+ ensure_inited ();
+ int err = 0;
+ if (!fhandler_virtual::open (flags, mode))
+ err = -1;
+ else if (loc == disk_dir || loc == by_id_dir)
+ {
+ if ((flags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
+ err = EEXIST;
+ else if (flags & O_WRONLY)
+ err = EISDIR;
+ else
+ diropen = true;
+ }
+ /* else if (loc == by_id_link) { } */ /* should not happen */
+ else /* (loc == invalid_loc) */
+ {
+ if (flags & O_CREAT)
+ err = EROFS;
+ else
+ err = ENOENT;
+ }
+
+ int res;
+ if (!err)
+ {
+ nohandle (true);
+ set_open_status ();
+ res = 1;
+ }
+ else
+ {
+ if (err > 0)
+ set_errno (err);
+ res = 0;
+ }
+
+ syscall_printf ("%d = fhandler_dev_disk::open(%y, 0%o)", res, flags, mode);
+ return res;
+}
+
+bool
+fhandler_dev_disk::fill_filebuf ()
+{
+ debug_printf ("fill_filebuf (%s)", get_name ());
+ ensure_inited ();
+ if (!(loc == by_id_link && drive_from_id >= 0))
+ return false;
+
+ char buf[32];
+ int len;
+ if (drive_from_id + 'a' <= 'z')
+ len = __small_sprintf (buf, "../../sd%c", drive_from_id + 'a');
+ else
+ len = __small_sprintf (buf, "../../sd%c%c",
+ drive_from_id / ('z' - 'a' + 1) - 1 + 'a',
+ drive_from_id % ('z' - 'a' + 1) + 'a');
+ if (part_from_id)
+ __small_sprintf (buf + len, "%d", part_from_id);
+
+ if (filebuf)
+ cfree (filebuf);
+ filebuf = cstrdup (buf);
+ return true;
+}
diff --git a/winsup/cygwin/local_includes/devices.h b/winsup/cygwin/local_includes/devices.h
index 10035263d..1e035f9d6 100644
--- a/winsup/cygwin/local_includes/devices.h
+++ b/winsup/cygwin/local_includes/devices.h
@@ -71,6 +71,7 @@ enum fh_devices
FH_DEV = FHDEV (DEV_VIRTFS_MAJOR, 193),
FH_CYGDRIVE= FHDEV (DEV_VIRTFS_MAJOR, 192),
FH_DEV_FD = FHDEV (DEV_VIRTFS_MAJOR, 191),
+ FH_DEV_DISK= FHDEV (DEV_VIRTFS_MAJOR, 190),
FH_SIGNALFD= FHDEV (DEV_VIRTFS_MAJOR, 13),
FH_TIMERFD = FHDEV (DEV_VIRTFS_MAJOR, 14),
@@ -415,6 +416,8 @@ extern const _device dev_piper_storage;
#define piper_dev ((device *) &dev_piper_storage)
extern const _device dev_pipew_storage;
#define pipew_dev ((device *) &dev_pipew_storage)
+extern const _device dev_dev_disk_storage;
+#define dev_disk_dev ((device *) &dev_dev_disk_storage)
extern const _device dev_proc_storage;
#define proc_dev ((device *) &dev_proc_storage)
extern const _device dev_dev_storage;
@@ -439,7 +442,8 @@ extern const _device dev_fs_storage;
#define isprocsys_dev(devn) (devn == FH_PROCSYS)
#define isvirtual_dev(devn) \
- (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE || devn == FH_DEV_FD)
+ (isproc_dev (devn) || devn == FH_CYGDRIVE || devn == FH_NETDRIVE \
+ || devn == FH_DEV_FD || devn == FH_DEV_DISK)
#define iscons_dev(n) \
((device::major ((dev_t) (n)) == DEV_CONS_MAJOR) \
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index 212c22344..cd61aadf8 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -40,6 +40,8 @@ details. */
extern const char *windows_device_names[];
extern struct __cygwin_perfile *perfile_table;
#define __fmode (*(user_data->fmode_ptr))
+extern const char dev_disk[];
+extern const size_t dev_disk_len;
extern const char proc[];
extern const size_t proc_len;
extern const char procsys[];
@@ -3190,6 +3192,50 @@ class fhandler_procnet: public fhandler_proc
}
};
+class fhandler_dev_disk: public fhandler_virtual
+{
+ enum dev_disk_location {
+ unknown_loc, invalid_loc, disk_dir, by_id_dir, by_id_link
+ };
+ dev_disk_location loc;
+
+ void init_dev_disk ();
+ void ensure_inited ()
+ {
+ if (loc == unknown_loc)
+ init_dev_disk ();
+ }
+
+ int drive_from_id;
+ int part_from_id;
+
+ public:
+ fhandler_dev_disk ();
+ fhandler_dev_disk (void *) {}
+ virtual_ftype_t exists();
+ DIR *opendir (int fd);
+ int closedir (DIR *);
+ int readdir (DIR *, dirent *);
+ int open (int flags, mode_t mode = 0);
+ int fstat (struct stat *buf);
+ bool fill_filebuf ();
+
+ void copy_from (fhandler_base *x)
+ {
+ pc.free_strings ();
+ *this = *reinterpret_cast<fhandler_dev_disk *> (x);
+ _copy_from_reset_helper ();
+ }
+
+ fhandler_dev_disk *clone (cygheap_types malloc_type = HEAP_FHANDLER)
+ {
+ void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_dev_disk));
+ fhandler_dev_disk *fh = new (ptr) fhandler_dev_disk (ptr);
+ fh->copy_from (this);
+ return fh;
+ }
+};
+
class fhandler_dev_fd: public fhandler_virtual
{
public:
@@ -3416,6 +3462,7 @@ typedef union
char __dev_raw[sizeof (fhandler_dev_raw)];
char __dev_tape[sizeof (fhandler_dev_tape)];
char __dev_zero[sizeof (fhandler_dev_zero)];
+ char __dev_disk[sizeof (fhandler_dev_disk)];
char __dev_fd[sizeof (fhandler_dev_fd)];
char __disk_file[sizeof (fhandler_disk_file)];
char __fifo[sizeof (fhandler_fifo)];
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index 36ab042a7..13ace6250 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -35,6 +35,9 @@ details. */
(path[mount_table->cygdrive_len + 1] == '/' || \
!path[mount_table->cygdrive_len + 1]))
+#define isdev_disk(path) \
+ (path_prefix_p (dev_disk, (path), dev_disk_len, false))
+
#define isproc(path) \
(path_prefix_p (proc, (path), proc_len, false))
@@ -685,6 +688,13 @@ mount_info::conv_to_win32_path (const char *src_path, char *dst, device& dev,
/* Go through chroot check */
goto out;
}
+ if (isdev_disk (src_path))
+ {
+ dev = *dev_disk_dev;
+ *flags = 0;
+ strcpy (dst, src_path);
+ goto out;
+ }
if (isproc (src_path))
{
dev = *proc_dev;
--
2.42.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-05 15:45 ` Christian Franke
@ 2023-11-05 19:59 ` Corinna Vinschen
2023-11-07 10:10 ` Christian Franke
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-05 19:59 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
Hi Christian,
On Nov 5 16:45, Christian Franke wrote:
> Corinna Vinschen wrote:
> > Do we really still need IOCTL_DISK_GET_DRIVE_LAYOUT w/o _EX?
>
> Possibly not. I borrowed this code from code behind /proc/partitions.
>
> > fhandler_proc still uses it, too, but the EX variation is available
> > since XP. I can't imagine that a fallback to the non-EX-version is still
> > necessary. If you like you could drop it immediately, or we can drop
> > both usages as a followup patch.
>
> Old IOCTL dropped and code simplified.
Great. I pushed your patch.
However, while I was just happily removing the non-EX ioctl's from
fhandler/proc.cc I found that this isn't feasible:
The EX calls are not supported by the floppy driver. D'uh.
So for /proc/partitions emulation we still need them.
> > Last, but not least, do you see a chance to add any other /dev/disk
> > subdir? by-partuuid, perhaps?
>
> Possibly, but not very soon. I'm not yet sure which API functions could be
> used.
> Some early draft ideas:
>
> /dev/disk/by-partuid (Partition UUID -> device)
> GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
> MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)
That should only require IOCTL_DISK_GET_PARTITION_INFO_EX, I think.
> /dev/disk/by-uuid (Windows Volume UUID -> device)
> Vol_UUID1 -> ../../sdXN (disk volume)
> Vol_UUID2 -> ../../scd0 (CD/DVD drive volume)
> Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID} (others, e.g. VeraCrypt
> volume)
Yeah, tricky. These are not the partition GUIDs but the filesystem
GUIDs or serial numbers. AFAICS, Windows filesystems (FAT*, NTFS)
don't maintain a filesystem GUID, as, e. g., ext4 or xfs, but only
serial numbers you can fetch via NtQueryVolumeInformationFile.
A Linux example of that is the serial number from a FAT32 filesytem
as the EFI boot partition in by-uuid:
lrwxrwxrwx 1 root root 10 Oct 30 10:20 DC38-0407 -> ../../sda1
On second thought, maybe that's sufficient for our by-uuid emulation.
> /dev/disk/by-drive (Cygwin specific: drive letter -> volume)
> c -> ../by-uuid/UUID (if UUID available)
> x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume
> Shadow Copy)
Ah, good idea. That's what my extension in /proc/partition already
provides, but a /dev/disk/by-drive sounds like a great idea.
Thanks,
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-05 19:59 ` Corinna Vinschen
@ 2023-11-07 10:10 ` Christian Franke
2023-11-07 13:29 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-11-07 10:10 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
Corinna Vinschen wrote:
> Hi Christian,
>
> On Nov 5 16:45, Christian Franke wrote:
>> ...
>> Old IOCTL dropped and code simplified.
> Great. I pushed your patch.
Thanks.
> ...
>>> Last, but not least, do you see a chance to add any other /dev/disk
>>> subdir? by-partuuid, perhaps?
>> Possibly, but not very soon. I'm not yet sure which API functions could be
>> used.
>> Some early draft ideas:
>>
>> /dev/disk/by-partuid (Partition UUID -> device)
>> GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
>> MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)
> That should only require IOCTL_DISK_GET_PARTITION_INFO_EX, I think.
Easier than expected: DRIVE_LAYOUT_INFORMATION_EX already contains
PARTITION_INFORMATION_EX so existing scanning function could be
enhanced. Patch attached.
>
>> /dev/disk/by-uuid (Windows Volume UUID -> device)
>> Vol_UUID1 -> ../../sdXN (disk volume)
>> Vol_UUID2 -> ../../scd0 (CD/DVD drive volume)
>> Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID} (others, e.g. VeraCrypt
>> volume)
> Yeah, tricky. These are not the partition GUIDs but the filesystem
> GUIDs or serial numbers. AFAICS, Windows filesystems (FAT*, NTFS)
> don't maintain a filesystem GUID, as, e. g., ext4 or xfs, but only
> serial numbers you can fetch via NtQueryVolumeInformationFile.
> A Linux example of that is the serial number from a FAT32 filesytem
> as the EFI boot partition in by-uuid:
>
> lrwxrwxrwx 1 root root 10 Oct 30 10:20 DC38-0407 -> ../../sda1
>
> On second thought, maybe that's sufficient for our by-uuid emulation.
>
>> /dev/disk/by-drive (Cygwin specific: drive letter -> volume)
>> c -> ../by-uuid/UUID (if UUID available)
>> x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume
>> Shadow Copy)
> Ah, good idea. That's what my extension in /proc/partition already
> provides, but a /dev/disk/by-drive sounds like a great idea.
Left for later :-)
Christian
[-- Attachment #2: 0001-Cygwin-Add-dev-disk-by-partuuid-symlinks.patch --]
[-- Type: text/plain, Size: 11491 bytes --]
From e9c9d2a1a1df9ddecd815300c62321a480f0de9b Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Tue, 7 Nov 2023 10:57:15 +0100
Subject: [PATCH] Cygwin: Add /dev/disk/by-partuuid symlinks
The new directory '/dev/disk/by-partuuid' provides symlinks for each
MBR or GPT disk partition:
'MBR_SERIAL-OFFSET' -> '../../sdXN'
'GPT_GUID' -> '../../sdXN'
Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
winsup/cygwin/fhandler/dev_disk.cc | 173 ++++++++++++++++--------
winsup/cygwin/local_includes/fhandler.h | 7 +-
2 files changed, 125 insertions(+), 55 deletions(-)
diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
index 9a1cae5eb..fcd0de651 100644
--- a/winsup/cygwin/fhandler/dev_disk.cc
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -176,9 +176,38 @@ by_id_realloc (by_id_entry *p, size_t n)
return reinterpret_cast<by_id_entry *>(p2);
}
+static bool
+format_partuuid (char *name, const PARTITION_INFORMATION_EX *pix)
+{
+ const GUID *guid;
+ switch (pix->PartitionStyle)
+ {
+ case PARTITION_STYLE_MBR: guid = &pix->Mbr.PartitionId; break;
+ case PARTITION_STYLE_GPT: guid = &pix->Gpt.PartitionId; break;
+ default: return false;
+ }
+
+ if (pix->PartitionStyle == PARTITION_STYLE_MBR && !guid->Data2
+ && !guid->Data3 && !guid->Data4[6] && !guid->Data4[7])
+ /* MBR "GUID": SERIAL-0000-0000-00NN-NNNNNNN00000
+ Byte offset in LE order -----^^^^-^^^^^^^
+ Print as: SERIAL-OFFSET */
+ __small_sprintf(name,
+ "%08_x-%02_x%02_x%02_x%02_x%02_x%02_x",
+ guid->Data1, guid->Data4[5], guid->Data4[4], guid->Data4[3],
+ guid->Data4[2], guid->Data4[1], guid->Data4[0]);
+ else
+ __small_sprintf(name,
+ "%08_x-%04_x-%04_x-%02_x%02_x-%02_x%02_x%02_x%02_x%02_x%02_x",
+ guid->Data1, guid->Data2, guid->Data3,
+ guid->Data4[0], guid->Data4[1], guid->Data4[2], guid->Data4[3],
+ guid->Data4[4], guid->Data4[5], guid->Data4[6], guid->Data4[7]);
+ return true;
+}
+
/* Create sorted name -> drive mapping table. Must be freed by caller. */
static int
-get_by_id_table (by_id_entry * &table)
+get_by_id_table (by_id_entry * &table, fhandler_dev_disk::dev_disk_location loc)
{
table = nullptr;
@@ -282,25 +311,31 @@ get_by_id_table (by_id_entry * &table)
}
}
- /* Fetch storage properties and create the ID string. */
- int rc = storprop_to_id_name (devhdl, &upath, ioctl_buf,
- table[table_size].name);
- if (rc <= 0)
+ const char *drive_name = "";
+ if (loc == fhandler_dev_disk::disk_by_id)
{
- if (rc < 0)
- errno_set = true;
- continue;
+ /* Fetch storage properties and create the ID string. */
+ int rc = storprop_to_id_name (devhdl, &upath, ioctl_buf,
+ table[table_size].name);
+ if (rc <= 0)
+ {
+ if (rc < 0)
+ errno_set = true;
+ continue;
+ }
+ drive_name = table[table_size].name;
+ table[table_size].drive = drive_num;
+ table[table_size].part = 0;
+ table_size++;
}
- int drive_index = table_size++;
- size_t drive_len = strlen(table[drive_index].name);
- table[drive_index].drive = drive_num;
- table[drive_index].part = 0;
- /* Fetch drive layout info to get size of all partitions on disk. */
+ /* Fetch drive layout info to information of all partitions on disk. */
DWORD bytes_read;
if (!DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 0,
ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
{
+ __seterrno_from_win_error (GetLastError ());
+ errno_set = true;
debug_printf ("DeviceIoControl(%S, "
"IOCTL_DISK_GET_DRIVE_LAYOUT_EX): %E", &upath);
continue;
@@ -311,7 +346,8 @@ get_by_id_table (by_id_entry * &table)
reinterpret_cast<const DRIVE_LAYOUT_INFORMATION_EX *>(ioctl_buf);
for (DWORD i = 0; i < dlix->PartitionCount; i++)
{
- DWORD part_num = dlix->PartitionEntry[i].PartitionNumber;
+ const PARTITION_INFORMATION_EX *pix = &dlix->PartitionEntry[i];
+ DWORD part_num = pix->PartitionNumber;
/* A partition number of 0 denotes an extended partition or a
filler entry as described in
fhandler_dev_floppy::lock_partition. Just skip. */
@@ -319,9 +355,22 @@ get_by_id_table (by_id_entry * &table)
continue;
if (part_num > max_part_num)
break;
- table[table_size] = table[drive_index];
- __small_sprintf(table[table_size].name + drive_len, "-part%u",
- part_num);
+
+ char *name = table[table_size].name;
+ switch (loc)
+ {
+ case fhandler_dev_disk::disk_by_id:
+ __small_sprintf (name, "%s-part%u", drive_name, part_num);
+ break;
+
+ case fhandler_dev_disk::disk_by_partuuid:
+ if (!format_partuuid (name, pix))
+ continue;
+ break;
+
+ default: continue; /* Should not happen. */
+ }
+ table[table_size].drive = drive_num;
table[table_size].part = part_num;
table_size++;
}
@@ -362,10 +411,15 @@ get_by_id_table (by_id_entry * &table)
const char dev_disk[] = "/dev/disk";
const size_t dev_disk_len = sizeof (dev_disk) - 1;
+static const char by_id[] = "/by-id";
+const size_t by_id_len = sizeof(by_id) - 1;
+static const char by_partuuid[] = "/by-partuuid";
+const size_t by_partuuid_len = sizeof(by_partuuid) - 1;
fhandler_dev_disk::fhandler_dev_disk ():
fhandler_virtual (),
loc (unknown_loc),
+ loc_is_link (false),
drive_from_id (-1),
part_from_id (0)
{
@@ -377,32 +431,46 @@ fhandler_dev_disk::init_dev_disk ()
if (loc != unknown_loc)
return;
- static const char by_id[] = "/by-id";
- const size_t by_id_len = sizeof(by_id) - 1;
-
/* Determine location. */
const char *path = get_name ();
+ size_t dirlen = 0;
if (!path_prefix_p (dev_disk, path, dev_disk_len, false))
loc = invalid_loc; // should not happen
else if (!path[dev_disk_len])
loc = disk_dir; // "/dev/disk"
- else if (!path_prefix_p (by_id, path + dev_disk_len, by_id_len, false))
- loc = invalid_loc; // "/dev/disk/invalid"
- else if (!path[dev_disk_len + by_id_len])
- loc = by_id_dir; // "/dev/disk/by-id"
- else if (strchr (path + dev_disk_len + by_id_len + 1, '/'))
- loc = invalid_loc; // "/dev/disk/by-id/dir/invalid"
+ else if (path_prefix_p (by_id, path + dev_disk_len, by_id_len, false))
+ {
+ loc = disk_by_id; // "/dev/disk/by-id.."
+ dirlen = dev_disk_len + by_id_len;
+ }
+ else if (path_prefix_p (by_partuuid, path + dev_disk_len, by_partuuid_len, false))
+ {
+ loc = disk_by_partuuid; // "/dev/disk/by-partuuid..."
+ dirlen = dev_disk_len + by_partuuid_len;
+ }
else
- loc = by_id_link; // possible "/dev/disk/by-id/LINK"
- debug_printf ("'%s': loc %d", path, (int)loc);
+ loc = invalid_loc; // "/dev/disk/invalid"
+
+ loc_is_link = false;
+ if (dirlen)
+ {
+ if (!path[dirlen])
+ ; // "/dev/disk/by-..."
+ else if (!strchr (path + dirlen + 1, '/'))
+ loc_is_link = true; // "/dev/disk/by-.../LINK"
+ else
+ loc = invalid_loc; // "/dev/disk/by-.../dir/invalid"
+ }
- /* Done if "/dev/disk", "/dev/disk/by_id" or invalid. */
- if (loc != by_id_link)
+ debug_printf ("%s: loc %d, loc_is_link %d", path, (int) loc, (int) loc_is_link);
+
+ /* Done if directory or invalid. */
+ if (!loc_is_link)
return;
- /* Check whether "/dev/disk/by_id/LINK" exists. */
+ /* Check whether "/dev/disk/by-.../LINK" exists. */
by_id_entry *table;
- int table_size = get_by_id_table (table);
+ int table_size = get_by_id_table (table, loc);
if (!table)
{
loc = invalid_loc;
@@ -410,7 +478,7 @@ fhandler_dev_disk::init_dev_disk ()
}
by_id_entry key;
- strcpy (key.name, path + dev_disk_len + by_id_len + 1);
+ strcpy (key.name, path + dirlen + 1);
const void *found = bsearch (&key, table, table_size, sizeof (*table),
by_id_compare_name);
if (found)
@@ -430,16 +498,12 @@ fhandler_dev_disk::exists ()
{
debug_printf ("exists (%s)", get_name ());
ensure_inited ();
- switch (loc)
- {
- case disk_dir:
- case by_id_dir:
- return virt_directory;
- case by_id_link:
- return virt_symlink;
- default:
- return virt_none;
- }
+ if (loc == invalid_loc)
+ return virt_none;
+ else if (loc_is_link)
+ return virt_symlink;
+ else
+ return virt_directory;
}
int
@@ -454,7 +518,7 @@ fhandler_dev_disk::fstat (struct stat *buf)
}
fhandler_base::fstat (buf);
- buf->st_mode = (loc == by_id_link ? S_IFLNK | S_IWUSR | S_IWGRP | S_IWOTH
+ buf->st_mode = (loc_is_link ? S_IFLNK | S_IWUSR | S_IWGRP | S_IWOTH
: S_IFDIR) | STD_RBITS | STD_XBITS;
buf->st_ino = get_ino ();
return 0;
@@ -470,16 +534,16 @@ DIR *
fhandler_dev_disk::opendir (int fd)
{
ensure_inited ();
- if (!(loc == disk_dir || loc == by_id_dir))
+ if (loc_is_link)
{
set_errno (ENOTDIR);
return nullptr;
}
by_id_entry *table = nullptr;
- if (loc == by_id_dir)
+ if (loc != disk_dir)
{
- int table_size = get_by_id_table (table);
+ int table_size = get_by_id_table (table, loc);
if (table_size < 0)
return nullptr; /* errno is set. */
if (table)
@@ -524,14 +588,15 @@ fhandler_dev_disk::readdir (DIR *dir, dirent *de)
dir->__d_position++;
res = 0;
}
- else if (loc == disk_dir && dir->__d_position == 2)
+ else if (loc == disk_dir && dir->__d_position < 2 + 2)
{
- strcpy (de->d_name, "by-id");
+ static const char * const names[2] {by_id, by_partuuid};
+ strcpy (de->d_name, names[dir->__d_position - 2] + 1);
de->d_type = DT_DIR;
dir->__d_position++;
res = 0;
}
- else if (loc == by_id_dir && *dir_id_table (dir))
+ else if (*dir_id_table (dir))
{
const char *name = (*dir_id_table (dir))[dir->__d_position - 2].name;
if (name[0])
@@ -559,7 +624,8 @@ fhandler_dev_disk::open (int flags, mode_t mode)
int err = 0;
if (!fhandler_virtual::open (flags, mode))
err = -1;
- else if (loc == disk_dir || loc == by_id_dir)
+ /* else if (loc_is_link) {} */ /* should not happen. */
+ else if (loc != invalid_loc)
{
if ((flags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
err = EEXIST;
@@ -568,8 +634,7 @@ fhandler_dev_disk::open (int flags, mode_t mode)
else
diropen = true;
}
- /* else if (loc == by_id_link) { } */ /* should not happen */
- else /* (loc == invalid_loc) */
+ else
{
if (flags & O_CREAT)
err = EROFS;
@@ -600,7 +665,7 @@ fhandler_dev_disk::fill_filebuf ()
{
debug_printf ("fill_filebuf (%s)", get_name ());
ensure_inited ();
- if (!(loc == by_id_link && drive_from_id >= 0))
+ if (!(loc_is_link && drive_from_id >= 0))
return false;
char buf[32];
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index cd61aadf8..6013496d5 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -3194,10 +3194,15 @@ class fhandler_procnet: public fhandler_proc
class fhandler_dev_disk: public fhandler_virtual
{
+public:
enum dev_disk_location {
- unknown_loc, invalid_loc, disk_dir, by_id_dir, by_id_link
+ unknown_loc, invalid_loc, disk_dir,
+ disk_by_id, disk_by_partuuid
};
+
+private:
dev_disk_location loc;
+ bool loc_is_link;
void init_dev_disk ();
void ensure_inited ()
--
2.42.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-07 10:10 ` Christian Franke
@ 2023-11-07 13:29 ` Corinna Vinschen
2023-11-07 14:30 ` Christian Franke
0 siblings, 1 reply; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-07 13:29 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 7 11:10, Christian Franke wrote:
> Corinna Vinschen wrote:
> > Hi Christian,
> >
> > On Nov 5 16:45, Christian Franke wrote:
> > > ...
> > > Old IOCTL dropped and code simplified.
> > Great. I pushed your patch.
>
> Thanks.
>
>
> > ...
> > > > Last, but not least, do you see a chance to add any other /dev/disk
> > > > subdir? by-partuuid, perhaps?
> > > Possibly, but not very soon. I'm not yet sure which API functions could be
> > > used.
> > > Some early draft ideas:
> > >
> > > /dev/disk/by-partuid (Partition UUID -> device)
> > > GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
> > > MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)
> > That should only require IOCTL_DISK_GET_PARTITION_INFO_EX, I think.
>
> Easier than expected: DRIVE_LAYOUT_INFORMATION_EX already contains
> PARTITION_INFORMATION_EX so existing scanning function could be enhanced.
> Patch attached.
Nice, pushed!
> > > /dev/disk/by-uuid (Windows Volume UUID -> device)
> > > Vol_UUID1 -> ../../sdXN (disk volume)
> > > Vol_UUID2 -> ../../scd0 (CD/DVD drive volume)
> > > Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID} (others, e.g. VeraCrypt
> > > volume)
> > Yeah, tricky. These are not the partition GUIDs but the filesystem
> > GUIDs or serial numbers. AFAICS, Windows filesystems (FAT*, NTFS)
> > don't maintain a filesystem GUID, as, e. g., ext4 or xfs, but only
> > serial numbers you can fetch via NtQueryVolumeInformationFile.
> > A Linux example of that is the serial number from a FAT32 filesytem
> > as the EFI boot partition in by-uuid:
> >
> > lrwxrwxrwx 1 root root 10 Oct 30 10:20 DC38-0407 -> ../../sda1
> >
> > On second thought, maybe that's sufficient for our by-uuid emulation.
> >
> > > /dev/disk/by-drive (Cygwin specific: drive letter -> volume)
> > > c -> ../by-uuid/UUID (if UUID available)
> > > x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume
> > > Shadow Copy)
> > Ah, good idea. That's what my extension in /proc/partition already
> > provides, but a /dev/disk/by-drive sounds like a great idea.
>
> Left for later :-)
Looking forward to it. We'll just need an entry for the release text
in winsup/cygwin/release/3.5.0 and doc/new-features.xml in the end :)
Thanks,
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-07 13:29 ` Corinna Vinschen
@ 2023-11-07 14:30 ` Christian Franke
2023-11-07 15:23 ` Corinna Vinschen
0 siblings, 1 reply; 23+ messages in thread
From: Christian Franke @ 2023-11-07 14:30 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 294 bytes --]
Corinna Vinschen wrote:
> ..
> Looking forward to it. We'll just need an entry for the release text
> in winsup/cygwin/release/3.5.0 and doc/new-features.xml in the end :)
Attached for now as implementing the remaining subdirs is not yet
scheduled. Docbook formatting not tested.
Christian
[-- Attachment #2: 0001-Cygwin-Document-dev-disk-by-id-and-dev-disk-by-partu.patch --]
[-- Type: text/plain, Size: 2253 bytes --]
From b07de21461207a2b57465d3dd8f7db2b36d886c0 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Tue, 7 Nov 2023 15:25:54 +0100
Subject: [PATCH] Cygwin: Document /dev/disk/by-id and /dev/disk/by-partuuid
Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
winsup/cygwin/release/3.5.0 | 6 ++++++
winsup/doc/new-features.xml | 14 ++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/winsup/cygwin/release/3.5.0 b/winsup/cygwin/release/3.5.0
index dbbf8009d..2d59818b5 100644
--- a/winsup/cygwin/release/3.5.0
+++ b/winsup/cygwin/release/3.5.0
@@ -17,6 +17,12 @@ What's new:
class expressions, and collating symbols in the search pattern, i.e.,
[:alnum:], [=a=], [.aa.].
+- Introduce /dev/disk directory with subdirectories by-id and by-partuuid.
+ The by-id directory provides symlinks for each disk and its partitions:
+ BUSTYPE-[VENDOR_]PRODUCT_[SERIAL|HASH][-partN] -> ../../sdX[N].
+ The by-partuuid directory provides symlinks for each MBR and GPT disk
+ partition: MBR_SERIAL-OFFSET -> ../../sdXN, GPT_GUID -> ../../sdXN.
+
- Introduce /proc/codesets and /proc/locales with information on
supported codesets and locales for all interested parties. Locale(1)
opens these files and uses the info for printing locale info like any
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 78b2dbafd..a8e8a7991 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -34,6 +34,20 @@ class expressions, and collating symbols in the search pattern, i.e.,
[:alnum:], [=a=], [.aa.].
</para></listitem>
+<listitem><para>
+Introduce /dev/disk directory with subdirectories by-id and by-partuuid.
+The by-id directory provides symlinks for each disk and its partitions:
+ <screen>
+ BUSTYPE-[VENDOR_]PRODUCT_[SERIAL|0xHASH][-partN] -> ../../sdX[N]
+ </screen>
+The by-partuuid directory provides symlinks for each MBR and GPT disk
+partition:
+ <screen>
+ MBR_SERIAL-OFFSET -> ../../sdXN
+ GPT_GUID -> ../../sdXN
+ </screen>
+</para></listitem>
+
<listitem><para>
Introduce /proc/codesets and /proc/locales with information on supported
codesets and locales for all interested parties. Locale(1) opens these
--
2.42.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks
2023-11-07 14:30 ` Christian Franke
@ 2023-11-07 15:23 ` Corinna Vinschen
0 siblings, 0 replies; 23+ messages in thread
From: Corinna Vinschen @ 2023-11-07 15:23 UTC (permalink / raw)
To: Christian Franke; +Cc: cygwin-patches
On Nov 7 15:30, Christian Franke wrote:
> Corinna Vinschen wrote:
> > ..
> > Looking forward to it. We'll just need an entry for the release text
> > in winsup/cygwin/release/3.5.0 and doc/new-features.xml in the end :)
>
> Attached for now as implementing the remaining subdirs is not yet scheduled.
> Docbook formatting not tested.
Looks good, pushed.
Thanks,
Corinna
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2023-11-07 15:23 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-25 11:57 [PATCH] Cygwin: Add /dev/disk/by-id symlinks Christian Franke
2023-10-03 12:39 ` Christian Franke
2023-11-03 9:55 ` Corinna Vinschen
2023-11-03 10:09 ` Corinna Vinschen
2023-11-03 11:06 ` Christian Franke
2023-11-03 11:11 ` Corinna Vinschen
2023-11-03 11:10 ` Corinna Vinschen
2023-11-03 13:27 ` Corinna Vinschen
2023-11-03 16:09 ` Christian Franke
2023-11-03 16:27 ` Corinna Vinschen
2023-11-03 16:30 ` Corinna Vinschen
2023-11-03 17:54 ` Christian Franke
2023-11-04 9:34 ` Corinna Vinschen
2023-11-04 9:57 ` Corinna Vinschen
2023-11-04 11:34 ` Christian Franke
2023-11-04 15:53 ` Christian Franke
2023-11-04 20:51 ` Corinna Vinschen
2023-11-05 15:45 ` Christian Franke
2023-11-05 19:59 ` Corinna Vinschen
2023-11-07 10:10 ` Christian Franke
2023-11-07 13:29 ` Corinna Vinschen
2023-11-07 14:30 ` Christian Franke
2023-11-07 15:23 ` Corinna Vinschen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).