[PATCH v3 0/2] MR5976: Acquire filesystem labels from dbus
-- v3: kernelbase: Silence 'read access denied' message if the device has a label, or if it's not expected to have one. https://gitlab.winehq.org/wine/wine/-/merge_requests/5976
From: Alfred Agrell <floating(a)muncher.se> --- dlls/mountmgr.sys/dbus.c | 24 +++++++++++++++--------- dlls/mountmgr.sys/device.c | 14 ++++++++------ dlls/mountmgr.sys/diskarb.c | 6 +++--- dlls/mountmgr.sys/mountmgr.c | 6 +++--- dlls/mountmgr.sys/mountmgr.h | 4 ++-- dlls/mountmgr.sys/unixlib.c | 4 +++- dlls/mountmgr.sys/unixlib.h | 4 +++- 7 files changed, 37 insertions(+), 25 deletions(-) diff --git a/dlls/mountmgr.sys/dbus.c b/dlls/mountmgr.sys/dbus.c index 6fe6984c218..52b5c203660 100644 --- a/dlls/mountmgr.sys/dbus.c +++ b/dlls/mountmgr.sys/dbus.c @@ -191,6 +191,7 @@ static void udisks_new_device( const char *udi ) const char *device = NULL; const char *mount_point = NULL; const char *type = NULL; + const char *label = NULL; GUID guid, *guid_ptr = NULL; int removable = FALSE; enum device_type drive_type = DEVICE_UNKNOWN; @@ -227,6 +228,8 @@ static void udisks_new_device( const char *udi ) p_dbus_message_iter_get_basic( &variant, &removable ); else if (!strcmp( name, "IdType" )) p_dbus_message_iter_get_basic( &variant, &type ); + else if (!strcmp( name, "IdLabel" )) + p_dbus_message_iter_get_basic( &variant, &label ); else if (!strcmp( name, "DriveMediaCompatibility" )) drive_type = udisks_parse_media_compatibility( &variant ); else if (!strcmp( name, "DeviceMountPaths" )) @@ -245,9 +248,9 @@ static void udisks_new_device( const char *udi ) } } - TRACE( "udi %s device %s mount point %s uuid %s type %s removable %u\n", + TRACE( "udi %s device %s mount point %s uuid %s type %s label %s removable %u\n", debugstr_a(udi), debugstr_a(device), debugstr_a(mount_point), - debugstr_guid(guid_ptr), debugstr_a(type), removable ); + debugstr_guid(guid_ptr), debugstr_a(type), debugstr_a(label), removable ); if (type) { @@ -265,8 +268,8 @@ static void udisks_new_device( const char *udi ) if (device) { - if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, NULL, NULL ); - else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, NULL ); + if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, NULL, label, NULL ); + else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, label, NULL ); } p_dbus_message_unref( reply ); @@ -276,7 +279,7 @@ static void udisks_new_device( const char *udi ) static void udisks_removed_device( const char *udi ) { TRACE( "removed %s\n", wine_dbgstr_a(udi) ); - queue_device_op( REMOVE_DEVICE, udi, NULL, NULL, 0, NULL, NULL, NULL ); + queue_device_op( REMOVE_DEVICE, udi, NULL, NULL, 0, NULL, NULL, NULL, NULL ); } /* UDisks callback for changed device */ @@ -366,6 +369,7 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess const char *type = NULL; const char *drive = NULL; const char *id = NULL; + const char *label = NULL; GUID guid, *guid_ptr = NULL; const char *iface, *name; int removable = FALSE; @@ -396,6 +400,8 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess device = udisks2_string_from_array( &variant ); else if (!strcmp( name, "IdType" )) p_dbus_message_iter_get_basic( &variant, &type ); + else if (!strcmp( name, "IdLabel" )) + p_dbus_message_iter_get_basic( &variant, &label ); else if (!strcmp( name, "Drive" )) { p_dbus_message_iter_get_basic( &variant, &drive ); @@ -414,9 +420,9 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess } } - TRACE( "udi %s device %s mount point %s uuid %s type %s removable %u\n", + TRACE( "udi %s device %s mount point %s uuid %s type %s label %s removable %u\n", debugstr_a(udi), debugstr_a(device), debugstr_a(mount_point), - debugstr_guid(guid_ptr), debugstr_a(type), removable ); + debugstr_guid(guid_ptr), debugstr_a(type), debugstr_a(label), removable ); if (type) { @@ -433,8 +439,8 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess } if (device) { - if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, id, NULL ); - else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, id, NULL ); + if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, id, label, NULL ); + else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, id, label, NULL ); } } diff --git a/dlls/mountmgr.sys/device.c b/dlls/mountmgr.sys/device.c index 48ee8d9189a..2aef41dad0f 100644 --- a/dlls/mountmgr.sys/device.c +++ b/dlls/mountmgr.sys/device.c @@ -949,7 +949,7 @@ static void set_dos_devices_disk_serial( struct disk_device *device ) /* change the information for an existing volume */ static NTSTATUS set_volume_info( struct volume *volume, struct dos_drive *drive, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const char *disk_serial ) + const char *label, const char *disk_serial ) { void *id = NULL; unsigned int id_len = 0; @@ -995,6 +995,8 @@ static NTSTATUS set_volume_info( struct volume *volume, struct dos_drive *drive, get_filesystem_label( volume ); get_filesystem_serial( volume ); } + if (label && label[0] && !volume->label[0]) + RtlMultiByteToUnicodeN(volume->label, ARRAY_SIZE(volume->label), NULL, label, strlen(label)+1); TRACE("fs_type %#x, label %s, serial %08lx\n", volume->fs_type, debugstr_w(volume->label), volume->serial); @@ -1098,7 +1100,7 @@ static void create_drive_devices(void) { /* don't reset uuid if we used an existing volume */ const GUID *guid = volume ? NULL : get_default_uuid(i); - set_volume_info( drive->volume, drive, device, link, drive_type, guid, NULL ); + set_volume_info( drive->volume, drive, device, link, drive_type, guid, NULL, NULL ); } if (volume) release_volume( volume ); } @@ -1198,7 +1200,7 @@ static void create_scsi_entry( struct volume *volume, const struct scsi_info *in /* create a new disk volume */ NTSTATUS add_volume( const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, const char *disk_serial, - const struct scsi_info *scsi_info ) + const char *label, const struct scsi_info *scsi_info ) { struct volume *volume; NTSTATUS status = STATUS_SUCCESS; @@ -1219,7 +1221,7 @@ NTSTATUS add_volume( const char *udi, const char *device, const char *mount_poin else status = create_volume( udi, type, &volume ); found: - if (!status) status = set_volume_info( volume, NULL, device, mount_point, type, guid, disk_serial ); + if (!status) status = set_volume_info( volume, NULL, device, mount_point, type, guid, label, disk_serial ); if (!status && scsi_info) create_scsi_entry( volume, scsi_info ); if (volume) release_volume( volume ); LeaveCriticalSection( &device_section ); @@ -1248,7 +1250,7 @@ NTSTATUS remove_volume( const char *udi ) /* create a new dos drive */ NTSTATUS add_dos_device( int letter, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const struct scsi_info *scsi_info ) + const char *label, const struct scsi_info *scsi_info ) { HKEY hkey; NTSTATUS status = STATUS_SUCCESS; @@ -1305,7 +1307,7 @@ found: struct set_dosdev_symlink_params params = { dosdev, mount_point }; MOUNTMGR_CALL( set_dosdev_symlink, ¶ms ); } - set_volume_info( volume, drive, device, mount_point, type, guid, NULL ); + set_volume_info( volume, drive, device, mount_point, type, guid, label, NULL ); TRACE( "added device %c: udi %s for %s on %s type %u\n", 'a' + drive->drive, wine_dbgstr_a(udi), wine_dbgstr_a(device), diff --git a/dlls/mountmgr.sys/diskarb.c b/dlls/mountmgr.sys/diskarb.c index d33f6c60afb..5a46da7f599 100644 --- a/dlls/mountmgr.sys/diskarb.c +++ b/dlls/mountmgr.sys/diskarb.c @@ -180,9 +180,9 @@ static void appeared_callback( DADiskRef disk, void *context ) } if (removable) - queue_device_op( ADD_DOS_DEVICE, device, device, mount_point, type, guid_ptr, NULL, &scsi_info ); + queue_device_op( ADD_DOS_DEVICE, device, device, mount_point, type, guid_ptr, NULL, NULL, &scsi_info ); else - if (guid_ptr) queue_device_op( ADD_VOLUME, device, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, &scsi_info ); + if (guid_ptr) queue_device_op( ADD_VOLUME, device, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, NULL, &scsi_info ); done: CFRelease( dict ); @@ -208,7 +208,7 @@ static void disappeared_callback( DADiskRef disk, void *context ) TRACE( "got unmount notification for '%s'\n", device ); - queue_device_op( REMOVE_DEVICE, device, NULL, NULL, 0, NULL, NULL, NULL ); + queue_device_op( REMOVE_DEVICE, device, NULL, NULL, 0, NULL, NULL, NULL, NULL ); done: CFRelease( dict ); diff --git a/dlls/mountmgr.sys/mountmgr.c b/dlls/mountmgr.sys/mountmgr.c index ad3c034fe0f..8d42fb36bad 100644 --- a/dlls/mountmgr.sys/mountmgr.c +++ b/dlls/mountmgr.sys/mountmgr.c @@ -257,7 +257,7 @@ static NTSTATUS define_unix_drive( const void *in_buff, SIZE_T insize ) case DRIVE_RAMDISK: type = DEVICE_RAMDISK; break; case DRIVE_FIXED: type = DEVICE_HARDDISK_VOL; break; } - return add_dos_device( letter - 'a', NULL, device, mount_point, type, NULL, NULL ); + return add_dos_device( letter - 'a', NULL, device, mount_point, type, NULL, NULL, NULL ); } else { @@ -444,11 +444,11 @@ static void CALLBACK device_op( ULONG_PTR arg1, ULONG_PTR arg2, ULONG_PTR arg3 ) { case ADD_DOS_DEVICE: add_dos_device( -1, info.udi, info.device, info.mount_point, - info.type, info.guid, info.scsi_info ); + info.type, info.guid, info.label, info.scsi_info ); break; case ADD_VOLUME: add_volume( info.udi, info.device, info.mount_point, DEVICE_HARDDISK_VOL, - info.guid, info.serial, info.scsi_info ); + info.guid, info.serial, info.label, info.scsi_info ); break; case REMOVE_DEVICE: if (!remove_dos_device( -1, info.udi )) remove_volume( info.udi ); diff --git a/dlls/mountmgr.sys/mountmgr.h b/dlls/mountmgr.sys/mountmgr.h index b1de312dffb..ae6927335b1 100644 --- a/dlls/mountmgr.sys/mountmgr.h +++ b/dlls/mountmgr.sys/mountmgr.h @@ -90,11 +90,11 @@ struct scsi_info extern NTSTATUS add_volume( const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, const char *disk_serial, - const struct scsi_info *info ); + const char *label, const struct scsi_info *info ); extern NTSTATUS remove_volume( const char *udi ); extern NTSTATUS add_dos_device( int letter, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const struct scsi_info *info ); + const char *label, const struct scsi_info *info ); extern NTSTATUS remove_dos_device( int letter, const char *udi ); extern NTSTATUS query_unix_drive( void *buff, SIZE_T insize, SIZE_T outsize, IO_STATUS_BLOCK *iosb ); diff --git a/dlls/mountmgr.sys/unixlib.c b/dlls/mountmgr.sys/unixlib.c index 47c42d939c6..36f9f82dad8 100644 --- a/dlls/mountmgr.sys/unixlib.c +++ b/dlls/mountmgr.sys/unixlib.c @@ -133,7 +133,7 @@ static void detect_devices( const char **paths, char *names, ULONG size ) void queue_device_op( enum device_op op, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const char *serial, const struct scsi_info *scsi_info ) + const char *serial, const char *label, const struct scsi_info *scsi_info ) { struct device_info *info; char *str, *end; @@ -152,6 +152,7 @@ void queue_device_op( enum device_op op, const char *udi, const char *device, ADD_STR(device); ADD_STR(mount_point); ADD_STR(serial); + ADD_STR(label); #undef ADD_STR if (guid) { @@ -188,6 +189,7 @@ static NTSTATUS dequeue_device_op( void *args ) if (dst->device) dst->device = (char *)dst + (src->device - (char *)src); if (dst->mount_point) dst->mount_point = (char *)dst + (src->mount_point - (char *)src); if (dst->serial) dst->serial = (char *)dst + (src->serial - (char *)src); + if (dst->label) dst->label = (char *)dst + (src->label - (char *)src); if (dst->guid) dst->guid = &dst->guid_buffer; if (dst->scsi_info) dst->scsi_info = &dst->scsi_buffer; diff --git a/dlls/mountmgr.sys/unixlib.h b/dlls/mountmgr.sys/unixlib.h index 41f64251e22..b40b323ac80 100644 --- a/dlls/mountmgr.sys/unixlib.h +++ b/dlls/mountmgr.sys/unixlib.h @@ -36,6 +36,7 @@ struct device_info const char *device; const char *mount_point; const char *serial; + const char *label; GUID *guid; struct scsi_info *scsi_info; @@ -179,7 +180,8 @@ enum mountmgr_funcs extern void queue_device_op( enum device_op op, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const char *disk_serial, const struct scsi_info *info ); + const char *disk_serial, const char *label, + const struct scsi_info *info ); extern void run_dbus_loop(void); extern void run_diskarbitration_loop(void); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5976
From: Alfred Agrell <floating(a)muncher.se> --- dlls/kernelbase/volume.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/kernelbase/volume.c b/dlls/kernelbase/volume.c index 944c275460c..33321fa81b2 100644 --- a/dlls/kernelbase/volume.c +++ b/dlls/kernelbase/volume.c @@ -199,8 +199,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label, if (status) { TRACE( "cannot open device %s: %lx\n", debugstr_w(nt_name.Buffer), status ); - if (status == STATUS_ACCESS_DENIED) - MESSAGE( "wine: Read access denied for device %s, FS volume label and serial are not available.\n", debugstr_w(nt_name.Buffer) ); status = NtOpenFile( &handle, SYNCHRONIZE, &attr, &io, 0, FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ); } @@ -209,6 +207,13 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label, ret = GetVolumeInformationByHandleW( handle, label, label_len, serial, filename_len, flags, fsname, fsname_len ); + + if ((label && !*label) || (serial && !*serial)) + { + UINT drive_type = GetDriveTypeW(root); + if (drive_type == DRIVE_REMOVABLE || drive_type == DRIVE_CDROM) + ERR( "wine: FS volume label and serial are not available for device %s.\n", debugstr_w(nt_name.Buffer) ); + } NtClose( handle ); done: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5976
On Wed Jul 3 00:03:30 2024 +0000, Alfred Agrell wrote:
The message shows up if I start some random program in Wine that just wants to show the label somewhere (like GetOpenFileName), or if the program just wants other information (for example https://github.com/mirror/make/blob/d66a65ad5a0e31b287f53930b0f09e31801f1613...). Having to add WINEDEBUG to every program with a file dialog sounds tedious, so I would quite appreciate changing it somehow. But you're right, that volume_label_warn variable wasn't the best idea I've had. I can drop the message completely if you'd prefer that, but mac doesn't run dbus, so I suspect the warning still has a few usecases. If you ask me, I'd prefer to not touch the message (until it is fixed in a way so that the serial and label are always available like on Windows) or turn it into FIXME. I think WINEDEBUG=-all is the right and only answer for unwanted debug message output. Our, I don't know, WINEDEBUG=-all by default but it would be a global change not specific to this place. Some things might be printed excessively of course but in general notifying on some missing stuff which is harmless in the most of cases is a common Wine practice, currently WINEDEBUG=-all and / or redirecting stderr is the proper answer.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74963
On Wed Jul 3 00:08:35 2024 +0000, Paul Gofman wrote:
If you ask me, I'd prefer to not touch the message (until it is fixed in a way so that the serial and label are always available like on Windows) or turn it into FIXME. I think WINEDEBUG=-all is the right and only answer for unwanted debug message output. Our, I don't know, WINEDEBUG=-all by default but it would be a global change not specific to this place. Some things might be printed excessively of course but in general notifying on some missing stuff which is harmless in the most of cases is a common Wine practice, currently WINEDEBUG=-all and / or redirecting stderr is the proper answer. I don't think that kernelbase is actually doing anything useful by trying to open the file with GENERIC_READ anymore? It certainly isn't helping it retrieve the label or serial. That used to be the case, but it's not anymore; we always depend on having a mountmgr device.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75029
On Wed Jul 3 16:54:42 2024 +0000, Elizabeth Figura wrote:
I don't think that kernelbase is actually doing anything useful by trying to open the file with GENERIC_READ anymore? It certainly isn't helping it retrieve the label or serial. That used to be the case, but it's not anymore; we always depend on having a mountmgr device. I had a brief look yesterday and there at least regardless of label and serial NtQueryVolumeInformationFile(FileFsAttributeInformation) in GetVolumeInformationByHandleW(). And NtQueryVolumeInformationFile(FileFsAttributeInformation) has fallback handling.
The other thing is, I am not sure if there is any scenario when opening with GENERIC_READ will fail and without GENERIC_READ will succeed? Both should end up in Unix fd open and there is probably no way (device, or any) file can be opened without read access. Maybe that is the fallback path without GENERIC_READ should be removed instead? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75032
On Wed Jul 3 17:20:22 2024 +0000, Paul Gofman wrote:
I had a brief look yesterday and there at least regardless of label and serial NtQueryVolumeInformationFile(FileFsAttributeInformation) in GetVolumeInformationByHandleW(). And NtQueryVolumeInformationFile(FileFsAttributeInformation) has fallback handling. The other thing is, I am not sure if there is any scenario when opening with GENERIC_READ will fail and without GENERIC_READ will succeed? Both should end up in Unix fd open and there is probably no way (device, or any) file can be opened without read access. Maybe that is the fallback path without GENERIC_READ should be removed instead? Nvm the last suggestion probably, there should be the difference for Wine device file which doesn't map to Unix fd.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75033
On Wed Jul 3 17:28:22 2024 +0000, Paul Gofman wrote:
Nvm the last suggestion probably, there should be the difference for Wine device file which doesn't map to Unix fd. It fails for devfs block devices, which is what that code operates on. Note that "A:\\" is the only valid type of path to pass to GetVolumeInformation(); our kernelbase internally strips the final slash and opens "\\??\\A:", and opening a drive root without a slash in ntdll causes us to open $WINEPREFIX\dosdevices\a:: (with the double colon) which points to the drive block device.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75047
It fails for devfs block devices, which is what that code operates on.
Sure, but I specifically checked here that for, e. g., /dev/sda1 (brw-rw---- 1 root disk) open() just fails without read permissions for user on file with EACCESS, with or without O_RDONLY (which should be present always btw, 'man open' says "The argument flags must include one of the following access modes: O_RDONLY, O_WRONLY, or O_RDWR"). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75049
On Wed Jul 3 19:44:30 2024 +0000, Paul Gofman wrote:
It fails for devfs block devices, which is what that code operates on. Sure, but I specifically checked here that for, e. g., /dev/sda1 (brw-rw---- 1 root disk) open() just fails without read permissions for user on file with EACCESS, with or without O_RDONLY (which should be present always btw, 'man open' says "The argument flags must include one of the following access modes: O_RDONLY, O_WRONLY, or O_RDWR"). Actually, the difference happens when $WINEPREFIX\\dosdevices\\c:: or z:: point to a directory (which is the case for common drives like C: and Z:).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75052
On Wed Jul 3 20:06:14 2024 +0000, Paul Gofman wrote:
Actually, the difference happens when $WINEPREFIX\\dosdevices\\c:: or z:: point to a directory (which is the case for common drives like C: and Z:). a:: should never point to a directory. If there's no corresponding block device (like for C) then a:: just doesn't exist. In that case we end up opening the real NT symlink \??\C:, which resolves to the mountmgr device, and in that case we do a real kernel ioctl instead of using the ntdll code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75055
On Wed Jul 3 20:22:56 2024 +0000, Elizabeth Figura wrote:
a:: should never point to a directory. If there's no corresponding block device (like for C) then a:: just doesn't exist. In that case we end up opening the real NT symlink \??\C:, which resolves to the mountmgr device, and in that case we do a real kernel ioctl instead of using the ntdll code. Oh yeah, right. And the major difference between first NtOpenFile attempt and the second are different file names ("\\??\\D:" on first attempt vs "\\??\\D:\\" on second attempt) which you mentioned. Sorry I got tripped by that access flag difference which is probably essentially irrelevant.
But still, the is a fallback path in NtQueryVolumeInformationFile(FileFsAttributeInformation) which tries to read from block device directly. If there is a certainty that everything should go through dbus now (including all the archs), then probably that path should be removed? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75056
Oh yeah, right. And the major difference between first NtOpenFile attempt and the second are different file names ("\\??\\D:" on first attempt vs "\\??\\D:\\" on second attempt) which you mentioned. Sorry I got tripped by that access flag difference which is probably essentially irrelevant.
Oops, you're right. I actually did *not* notice that and completely forgot it worked that way.
But still, the is a fallback path in NtQueryVolumeInformationFile(FileFsAttributeInformation) which tries to read from block device directly. If there is a certainty that everything should go through dbus now (including all the archs), then probably that path should be removed?
I don't know the answer to whether we want to depend on mountmgr being able to enumerate all devices. It'd certainly be nice if we can. Either way, though, the message "unable to read serial and label" is inaccurate, since we *do* require mountmgr for that one, and mountmgr requires having an actual volume enumerated (it doesn't try to read from the block device like the old code did). There's a fallback for drive type, but none for label and serial. So we should probably just remove it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75080
On Wed Jul 3 21:48:26 2024 +0000, Elizabeth Figura wrote:
Oh yeah, right. And the major difference between first NtOpenFile attempt and the second are different file names ("\\??\\D:" on first attempt vs "\\??\\D:\\" on second attempt) which you mentioned. Sorry I got tripped by that access flag difference which is probably essentially irrelevant. Oops, you're right. I actually did *not* notice that and completely forgot it worked that way. But still, the is a fallback path in NtQueryVolumeInformationFile(FileFsAttributeInformation) which tries to read from block device directly. If there is a certainty that everything should go through dbus now (including all the archs), then probably that path should be removed? I don't know the answer to whether we want to depend on mountmgr being able to enumerate all devices. It'd certainly be nice if we can. Either way, though, the message "unable to read serial and label" is inaccurate, since we *do* require mountmgr for that one, and mountmgr requires having an actual volume enumerated (it doesn't try to read from the block device like the old code did). There's a fallback for drive type, but none for label and serial. So we should probably just remove it. Delete that message, sure, can do
(sorry for lateness, waited until this convo finished then forgot it) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_75476
participants (4)
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
Elizabeth Figura (＠zfigura)
-
Paul Gofman (＠gofman)