Hello, apologies for the very late reply.
On 5/1/21 6:54 PM, David Koolhoven wrote:
On Sat, May 01, 2021 at 02:53:57PM -0500, Zebediah Figura (she/her) wrote:
Hello David, thanks for the patch!
Hello thanks for the review Zebediah!
I'm curious how the application is using this information, and if it makes sense to just fake it instead, for simplicity?
It's not certain how it's being used, but it solves this bug: https://bugs.winehq.org/show_bug.cgi?id=51065
It could be used to collect performance statistics, or to deploy an application level policy on data access and memory management. I'm uncertain.
I was recently informed this patch doesn't function on LVM partitians, so it'll need a new version anyway.
Spoofing a response would certainly make it easier to implement and maintain, it sort of depends on the goals of the WINE project for a feature like this, which leads to a question:
Would it be better for this to spoof an answer to the query if platform specific testing fails, rather than reporting a failure to the Windows program?
I think there's nothing wrong with implementing things like this properly in general, but stubs do tend to be easier.
The awkward thing in this case is that mountmgr is kind of a mess wrt how we track disks and partitions; that should probably be cleaned up to some degree first.
If spoofing is better, should it spoof a seek penalty exists or not? I think it should spoof that it does exist, but I'm open to any implementation.
I don't have strong opinions here.
The Linux platform logic was put into it's own file because I wanted to attempt a FreeBSD version of the same logic too. Would renaming the function with a prefix of "wineint_" work to clarify what it is?
There's no reason to split platform-specific logic into their own files; we leave them in the same file almost everywhere else in Wine.
Wrt naming, I mean that "device_seek_penalty" does not semantically communicate what the function does. A predicate should generally be named like "device_has_seek_penalty".
I'll submit a spoofing only version and float a testing implementation later.
Thank you!
I'll work out the rest.