Hello David, thanks for the patch!
I'm curious how the application is using this information, and if it makes sense to just fake it instead, for simplicity?
I have some further comments inlined below.
On 4/27/21 4:31 PM, David Koolhoven wrote:
Allows programs to query if disk has a seek penalty.
Signed-off-by: David Koolhoven david@koolhoven-home.net
You could add a Wine-Bug tag for this; see e.g. https://source.winehq.org/patches/data/204879 for an example.
v3: Linux side/sysfs logic is contained file.
dlls/mountmgr.sys/Makefile.in | 1 + dlls/mountmgr.sys/device.c | 24 +++ dlls/mountmgr.sys/unix/device_seek_penalty.c | 145 +++++++++++++++++++ include/ntddstor.h | 9 +- 4 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 dlls/mountmgr.sys/unix/device_seek_penalty.c
diff --git a/dlls/mountmgr.sys/Makefile.in b/dlls/mountmgr.sys/Makefile.in index 83204e66504..61b462ddd7b 100644 --- a/dlls/mountmgr.sys/Makefile.in +++ b/dlls/mountmgr.sys/Makefile.in @@ -7,6 +7,7 @@ EXTRALIBS = $(DISKARBITRATION_LIBS) $(SYSTEMCONFIGURATION_LIBS) $(CORESERVICES_L
C_SRCS = \ dbus.c \
- unix/device_seek_penalty.c \ device.c \ diskarb.c \ mountmgr.c
I don't think there's a good enough reason to do this; leaving it in device.c would be better.
diff --git a/dlls/mountmgr.sys/device.c b/dlls/mountmgr.sys/device.c index 04e8fe3c0f5..90c73b13139 100644 --- a/dlls/mountmgr.sys/device.c +++ b/dlls/mountmgr.sys/device.c @@ -1894,6 +1894,30 @@ static NTSTATUS query_property( struct disk_device *device, IRP *irp )
break; }
- case StorageDeviceSeekPenaltyProperty:
- {
DEVICE_SEEK_PENALTY_DESCRIPTOR *descriptor;- int ret;
Please use consistent four-space indentation, and avoid tabs.
memset( irp->AssociatedIrp.SystemBuffer, 0, irpsp->Parameters.DeviceIoControl.OutputBufferLength );descriptor = irp->AssociatedIrp.SystemBuffer;descriptor->Version = sizeof(DEVICE_SEEK_PENALTY_DESCRIPTOR);
That seems wrong.
descriptor->Size = sizeof(DEVICE_SEEK_PENALTY_DESCRIPTOR);irp->IoStatus.Information = sizeof(DEVICE_SEEK_PENALTY_DESCRIPTOR);ret = device_seek_penalty(device->unix_mount);
This function is badly named, especially considering that it's a predicate.
if (ret < 0) {FIXME( "Unsupported property StorageDeviceSeekPenalty\n" );status = STATUS_NOT_SUPPORTED;} else if (ret == 0) {descriptor->IncursSeekPenalty = FALSE;status = STATUS_SUCCESS;} else if (ret == 1) {descriptor->IncursSeekPenalty = TRUE;status = STATUS_SUCCESS;- }
break;- } default: FIXME( "Unsupported property %#x\n", query->PropertyId ); status = STATUS_NOT_SUPPORTED;
diff --git a/dlls/mountmgr.sys/unix/device_seek_penalty.c b/dlls/mountmgr.sys/unix/device_seek_penalty.c new file mode 100644 index 00000000000..eefb4d67e65 --- /dev/null +++ b/dlls/mountmgr.sys/unix/device_seek_penalty.c @@ -0,0 +1,145 @@ +/*
- System information APIs
- Copyright 1996-1998 Marcus Meissner
- This library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
- This library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
- You should have received a copy of the GNU Lesser General Public
- License along with this library; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
- */
+#include "config.h" +#include "wine/port.h"
+#ifdef linux +#include <assert.h> +#include <errno.h> +#include <stdarg.h> +#include <stdio.h> +#include <sys/stat.h> +#include <sys/sysmacros.h>
+#define NONAMELESSUNION +#include "ntstatus.h" +#include "mountmgr.h" +#define WIN32_NO_STATUS +#include "windef.h" +#include "winternl.h" +#endif +#include "wine/debug.h"
+WINE_DEFAULT_DEBUG_CHANNEL(mountmgr);
+int device_seek_penalty (char *unix_mount) +{ +#ifdef linux
- FILE *fp;
- char isrotapathstr[260];
- char evpath[260];
- char ueventbufstr[260];
- char isrotastrbuf[2];
- char *fgetsret = NULL;
- int ret = 0;
- char *rptr = NULL;
- char *sptr = NULL;
- char *devstr_edited = NULL;
- char mountpath[260];
- char *part_dev = NULL;
- int c = 0;
- struct stat statbuf;
- int seekpen;
- const char *home = getenv( "HOME" );
This function is full of inconsistent spacing; please try to match the surrounding code style.
- const char *prefix = getenv( "WINEPREFIX" );
- size_t len = (prefix ? strlen(prefix) : strlen(home) + strlen("/.wine")) + sizeof("/dosdevices/com256");
- char *path = HeapAlloc( GetProcessHeap(), 0, len );
- if (path)
- {
if (prefix) strcpy( path, prefix );else{strcpy( path, home );strcat( path, "/.wine" );}strcat( path, "/dosdevices/" );- }
- if (!unix_mount) {
return -1;- }
- snprintf (mountpath, 260, "%s%s", path, unix_mount);
- ret = stat (mountpath, &statbuf);
This doesn't make any sense; unix_mount is the absolute Unix mount point. Did you test this code?
- if (ret == -1) {
return -1;- }
- HeapFree( GetProcessHeap(), 0, path );
- sprintf (evpath, "/sys/dev/block/%d:%d/uevent", major(statbuf.st_dev), minor(statbuf.st_dev));
- fp = fopen(evpath, "r");
- if (!fp) {
return -1;- }
- while ((rptr = fgets (ueventbufstr, 260, fp))) {
sptr = strstr (rptr, "DEVNAME=");if (sptr) {sptr += strlen ("DEVNAME=");break;}- }
- fclose (fp);
- devstr_edited = sptr;
- if (!devstr_edited) {
return -1;- }
- /* Find first character after forwardslash '/' */
- part_dev = strrchr (devstr_edited, '/');
- if (!part_dev || (part_dev == devstr_edited)) part_dev = devstr_edited;
- else part_dev++;
- /* Trim off trailing digits and whitespace */
- c = strlen (devstr_edited);
- c--;
- while ((devstr_edited[c] >= '0' && devstr_edited[c] <= '9')
|| (devstr_edited[c] == '\n' || devstr_edited[c] == '\r'))devstr_edited[c--] = '\0';
This isn't reliable; there's no guarantee that partition devices are named in that scheme (and I've seen partitions that aren't in practice).
- ret = snprintf (isrotapathstr, 260, "/sys/block/%s/queue/rotational", part_dev);
- if (ret < 1 || ret == 260) {
return -1;- }
"ret >= 260"
- fp = fopen(isrotapathstr, "r");
- if (!fp) {
return -1;- }
- fgetsret = fgets(isrotastrbuf, 2, fp);
- if (!fgetsret) {
return -1;- }
fgetc() seems even easier.
- fclose (fp);
- if (isrotastrbuf[0] == '1') {
seekpen = 1;- } else if (isrotastrbuf[0] == '0') {
seekpen = 0;- } else {
return -1;- }
- return seekpen;
+#else
- return -1;
+#endif +} diff --git a/include/ntddstor.h b/include/ntddstor.h index b8c4bb73b0d..836def413fe 100644 --- a/include/ntddstor.h +++ b/include/ntddstor.h @@ -214,7 +214,8 @@ typedef enum _STORAGE_QUERY_TYPE {
typedef enum _STORAGE_PROPERTY_ID { StorageDeviceProperty = 0,
- StorageAdapterProperty
- StorageAdapterProperty = 1,
- StorageDeviceSeekPenaltyProperty = 7,
} STORAGE_PROPERTY_ID, *PSTORAGE_PROPERTY_ID;
typedef struct _STORAGE_PROPERTY_QUERY { @@ -272,6 +273,12 @@ typedef struct _STORAGE_ADAPTER_DESCRIPTOR { USHORT BusMinorVersion; } STORAGE_ADAPTER_DESCRIPTOR, *PSTORAGE_ADAPTER_DESCRIPTOR;
+typedef struct _DEVICE_SEEK_PENALTY_DESCRIPTOR {
- ULONG Version;
- ULONG Size;
- BOOLEAN IncursSeekPenalty;
+} DEVICE_SEEK_PENALTY_DESCRIPTOR, *PDEVICE_SEEK_PENALTY_DESCRIPTOR;
#ifdef __cplusplus } #endif