[PATCH v2 0/2] MR5976: Acquire filesystem labels from dbus
-- v2: kernelbase: Silence 'read access denied' message if the device has a label, or if it's not expected to have one. mountmgr.sys: Read and use disk label from dbus. 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 | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dlls/kernelbase/volume.c b/dlls/kernelbase/volume.c index 944c275460c..9a5deffa922 100644 --- a/dlls/kernelbase/volume.c +++ b/dlls/kernelbase/volume.c @@ -163,6 +163,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label, OBJECT_ATTRIBUTES attr; unsigned int i; BOOL ret = FALSE; + BOOL volume_label_warn = FALSE; if (!root) root = L"\\"; if (!RtlDosPathNameToNtPathName_U( root, &nt_name, NULL, NULL )) @@ -200,7 +201,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label, { 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) ); + volume_label_warn = TRUE; status = NtOpenFile( &handle, SYNCHRONIZE, &attr, &io, 0, FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ); } @@ -209,6 +210,13 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label, ret = GetVolumeInformationByHandleW( handle, label, label_len, serial, filename_len, flags, fsname, fsname_len ); + + if (volume_label_warn && ERR_ON(volume) && ((label && !*label) || (serial && !*serial))) + { + UINT drive_type = GetDriveTypeW(root); + if (drive_type == DRIVE_REMOVABLE || drive_type == DRIVE_CDROM) + ERR( "wine: Read access denied for device %s, FS volume label and serial are not available.\n", debugstr_w(nt_name.Buffer) ); + } NtClose( handle ); done: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5976
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146761 Your paranoid android. === debian11b (64 bit WoW report) === kernel32: comm.c:1574: Test failed: AbortWaitCts hComPortEvent failed comm.c:1586: Test failed: Unexpected time 1001, expected around 500
My main goal (for this entire MR) is to shut up that MESSAGE for Z:. Judging by commit 4907f262, the goal is to print that only if the device is a mounted ISO file or an actual optical drive. Let's check for specifically that.
It certainly doesn't worth to do a lot of things to get rid of a message in the log. I personally doubt shutting it up is too much useful, but maybe that is just me. If this has to be done it is better not to build up heuristics about that but rather make it a FIXME (so it doesn't show up with WINEDEBUG=-all), or WARN or remove it. But anything without building the whole framework around that or otherwise complicating things. But note that the output is needed for debugging only, the best and only sane way to make it silent is to redirect stderr to /dev/null. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74960
On Tue Jul 2 23:31:06 2024 +0000, Paul Gofman wrote:
My main goal (for this entire MR) is to shut up that MESSAGE for Z:. Judging by commit 4907f262, the goal is to print that only if the device is a mounted ISO file or an actual optical drive. Let's check for specifically that. It certainly doesn't worth to do a lot of things to get rid of a message in the log. I personally doubt shutting it up is too much useful, but maybe that is just me. If this has to be done it is better not to build up heuristics about that but rather make it a FIXME (so it doesn't show up with WINEDEBUG=-all), or WARN or remove it. But anything without building the whole framework around that or otherwise complicating things. But note that the output is needed for debugging only, the best and only sane way to make it silent is to redirect stderr to /dev/null. 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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74961
participants (4)
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
Marvin -
Paul Gofman (＠gofman)