Re: [PATCH] Cygwin: Add /dev/disk/by-id symlinks

[Christian Franke <Christian.Franke@t-online.de>]

[-- 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