Re: API break, sysfs "capability" file

[Jens Axboe]

On 4/16/24 8:25 AM, Lennart Poettering wrote:
> On Di, 16.04.24 08:22, Jens Axboe (axboe@xxxxxxxxx) wrote:
> 
>> On 4/16/24 8:18 AM, Lennart Poettering wrote:
>>> On Di, 09.04.24 09:17, Jens Axboe (axboe@xxxxxxxxx) wrote:
>>>
>>>> On 4/9/24 8:15 AM, Christoph Hellwig wrote:
>>>>> On Tue, Apr 09, 2024 at 10:19:09AM +0200, Lennart Poettering wrote:
>>>>>> All I am looking for is a very simple test that returns me a boolean:
>>>>>> is there kernel-level partition scanning enabled on this device or
>>>>>> not.
>>>>>
>>>>> And we can add a trivial sysfs attribute for that.
>>>>
>>>> And I think we should. I don't know what was being smoked adding a sysfs
>>>> interface that exposed internal flag values - and honestly what was
>>>> being smoked to rely on that, but I think it's fair to say that the
>>>> majority of the fuckup here is on the kernel side.
>>>
>>> Yeah, it's a shitty interface, the kernel is rich in that. But it was
>>> excessively well documented, better in fact than almost all other
>>> kernel interfaces:
>>>
>>> ? https://www.kernel.org/doc/html/v5.16/block/capability.html ?
>>>
>>> If you document something on so much detail in the API docs, how do
>>> you expect this *not* to be relied on by userspace.
>>
>> This is _internal_ documentation, not user ABI documentation. The fact
>> that it's talking about internal flag values should make that clear,
>> though I can definitely see how that's just badly exposed along with
>> other things that document things that users/admins could care about.
> 
> The text begins with:
> 
>     "This file documents the sysfs file block/<disk>/capability."
> 
> So it makes very clear this is about the sysfs interface.
> 
> Are you saying that sysfs of the block layer should be considered an
> *internal* kernel API? That's a wild definition, if I may say so.

No I missed that - to me it's clearly internal documentation as it's
talking about the flags, but yeah you are right it's being presented as
sysfs documentation for the 'capability' file. That should never have
gone into the tree as ABI documentation.

Doesn't really change my conclusion from earlier. As mentioned, this is
clearly a kernel fuckup, and honestly since it's being presented as ABI,
we definitely need to rectify this and provide an alternative. Even
though I'm not a huge fan of it, might just be best to re-introduce
'capability' and just have conversions of the flags so we retain the
user side of it the same. That can then also go into stable, so we'll
end up with something that at least looks cohesive on the user side.

-- 
Jens Axboe