[v2] documentation: fix Generic Block Device Capability
Commit Message
While adding documentation for blkfilter, new lines of documentation was
added in the file include/linux/blkdev.h. Sergei Shtepa spotted that
this changed Documentation/block/capability.rst output.
Checking the documentation of previous versions, for example
https://www.kernel.org/doc/html/v5.10/block/capability.html I also saw
that the output of recent versions was already "broken" before.
This patch restore the output only relate to generic block device
capability and also added the hexadecimal values that was removed from
doc comment in commit 430cc5d (block: cleanup the GENHD_FL_*
definitions).
This will make this documentation page return to "documents the sysfs
file block/<disk>/capability" including the printed value in hexadecimal.
Co-developed-by: Sergei Shtepa <sergei.shtepa@veeam.com>
Signed-off-by: Sergei Shtepa <sergei.shtepa@veeam.com>
Signed-off-by: Fabio Fantoni <fantonifabio@tiscali.it>
---
Note: v1 was included in blksnap v2 serie
Changes from v1:
- Documentation/block/capability.rst: removed other blkdev.h functions
and keep only ":DOC: genhd capability flags"
- added changes of include/linux/blkdev.h
Documentation/block/capability.rst | 1 +
include/linux/blkdev.h | 10 +++++-----
2 files changed, 6 insertions(+), 5 deletions(-)
Comments
On Tue, Jan 10, 2023 at 02:21:04PM +0100, Fabio Fantoni wrote:
> - * ``GENHD_FL_REMOVABLE``: indicates that the block device gives access to
> + * ``GENHD_FL_REMOVABLE`` (0x01): indicates that the block device gives access to
The numberic values really do not belong into the documentation. They
are just implementation details.
Il 16/01/2023 18:53, Christoph Hellwig ha scritto:
> On Tue, Jan 10, 2023 at 02:21:04PM +0100, Fabio Fantoni wrote:
>> - * ``GENHD_FL_REMOVABLE``: indicates that the block device gives access to
>> + * ``GENHD_FL_REMOVABLE`` (0x01): indicates that the block device gives access to
> The numberic values really do not belong into the documentation. They
> are just implementation details.
>
Thanks for reply, if values are not into the documentation see from
/sys/block/<disk>/capability output what flags are enabled will require
look to source code of include/linux/blkdev.h and
Documentation/block/capability.rst will be less useful, or I'm wrong?
On Mon, Jan 16, 2023 at 07:14:41PM +0100, Fabio Fantoni wrote:
> Il 16/01/2023 18:53, Christoph Hellwig ha scritto:
> > On Tue, Jan 10, 2023 at 02:21:04PM +0100, Fabio Fantoni wrote:
> > > - * ``GENHD_FL_REMOVABLE``: indicates that the block device gives access to
> > > + * ``GENHD_FL_REMOVABLE`` (0x01): indicates that the block device gives access to
> > The numberic values really do not belong into the documentation. They
> > are just implementation details.
> >
> Thanks for reply, if values are not into the documentation see from
> /sys/block/<disk>/capability output what flags are enabled will require look
> to source code of include/linux/blkdev.h and
> Documentation/block/capability.rst will be less useful, or I'm wrong?
Urgg. I think this file is generally just a bad idea. The flags are
kernel internal and not isolated from userspace. It seems nothing broke
with the various renumbering lately, but we need to isolate it from
the implementation details. And if it really should be a user API
we need a text version of it.
Il 17/01/2023 07:05, Christoph Hellwig ha scritto:
> On Mon, Jan 16, 2023 at 07:14:41PM +0100, Fabio Fantoni wrote:
>> Il 16/01/2023 18:53, Christoph Hellwig ha scritto:
>>> On Tue, Jan 10, 2023 at 02:21:04PM +0100, Fabio Fantoni wrote:
>>>> - * ``GENHD_FL_REMOVABLE``: indicates that the block device gives access to
>>>> + * ``GENHD_FL_REMOVABLE`` (0x01): indicates that the block device gives access to
>>> The numberic values really do not belong into the documentation. They
>>> are just implementation details.
>>>
>> Thanks for reply, if values are not into the documentation see from
>> /sys/block/<disk>/capability output what flags are enabled will require look
>> to source code of include/linux/blkdev.h and
>> Documentation/block/capability.rst will be less useful, or I'm wrong?
> Urgg. I think this file is generally just a bad idea. The flags are
> kernel internal and not isolated from userspace. It seems nothing broke
> with the various renumbering lately, but we need to isolate it from
> the implementation details. And if it really should be a user API
> we need a text version of it.
Up to 5.13 the page was "ok"
(https://www.kernel.org/doc/html/v5.13/block/capability.html) from 5.14
doc comments of other functions was appaired, from 5.17 the flags had
"major" changes and values was removed from doc comment
(https://www.kernel.org/doc/html/v5.17/block/capability.html)
With this patch return to have only block capability flags in this file
and including the values: https://snipboard.io/QrN6hg.jpg
But as you tell that this file is a bad idea,
Documentation/block/capability.rst should be removed?
About if should be a user API, I don't have enough experience to be able
to say if and in which cases it could be useful on the user side. From
what I could see with the documentation page correct is that you can at
least immediately see (after a very fast search) for example if the
block device is removable and if it is partitionable. And even if the
values was changed pointing to the doc page of the used kernel version
and having the values in doc is still possible see what flags are
enabled from doc page.
@@ -8,3 +8,4 @@ This file documents the sysfs file ``block/<disk>/capability``.
capabilities a specific block device supports:
.. kernel-doc:: include/linux/blkdev.h
+ :DOC: genhd capability flags
@@ -74,16 +74,16 @@ struct partition_meta_info {
/**
* DOC: genhd capability flags
*
- * ``GENHD_FL_REMOVABLE``: indicates that the block device gives access to
+ * ``GENHD_FL_REMOVABLE`` (0x01): indicates that the block device gives access to
* removable media. When set, the device remains present even when media is not
* inserted. Shall not be set for devices which are removed entirely when the
* media is removed.
*
- * ``GENHD_FL_HIDDEN``: the block device is hidden; it doesn't produce events,
- * doesn't appear in sysfs, and can't be opened from userspace or using
- * blkdev_get*. Used for the underlying components of multipath devices.
+ * ``GENHD_FL_HIDDEN`` (0x02): the block device is hidden; it doesn't produce
+ * events, doesn't appear in sysfs, and can't be opened from userspace or
+ * using blkdev_get*. Used for the underlying components of multipath devices.
*
- * ``GENHD_FL_NO_PART``: partition support is disabled. The kernel will not
+ * ``GENHD_FL_NO_PART`` (0x04): partition support is disabled. The kernel will not
* scan for partitions from add_disk, and users can't add partitions manually.
*
*/