Re: [PATCH v2 3/3] loop.4: Document loop_configure ioctl

["Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>]

Hello Yang Xu,

On 8/6/20 11:48 AM, Yang Xu wrote:
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>  man4/loop.4 | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/man4/loop.4 b/man4/loop.4
> index 41abe0122..44dfd9a8c 100644
> --- a/man4/loop.4
> +++ b/man4/loop.4
> @@ -181,6 +181,41 @@ This value must be a power of two in the range
>  otherwise, an
>  .B EINVAL
>  error results.
> +.TP
> +.BR LOOP_CONFIGURE " (since Linux 5.8)"
> +.\" commit 3448914e8cc550ba792d4ccc74471d1ca4293aae
> +Setup and configure all loop device parameters using the (third)
> +.BR ioctl (2)
> +argument at once.

I suggest rewording as

[[
Setup and configure all loop device parameters in a single step using
the (third)
.BR ioctl (2)
argument.
]]

> +This argument is a pointer to

s/to/to a/

> +.I loop_config
> +structure, defined in
> +.I <linux/loop.h>
> +as:
> +.IP
> +.in +4n
> +.EX
> +struct loop_config {
> +     __u32               fd;
> +     __u32               block_size;
> +     struct loop_info64  info;
> +     __u64               __reserved[8];
> +};
> +.EE
> +.in
> +.IP
> +In addition to doing what
> +.BR LOOP_SET_STATUS
> +can do,
> +.BR LOOP_CONFIGURE
> +can also be used to set the correct block size immediately
> +by setting loop_config.block_size. Explicitly request
> +direct I/O mode by setting
> +.BR LO_FLAGS_DIRECT_IO
> +in loop_config.info.lo_flags. Explicitly request read-only
> +mode by setting
> +.BR LO_FLAGS_READ_ONLY
> +in loop_config.info.lo_flags.

On an earlier version of this patch, I commented that this piece is
unclear (the language is difficult to understand). If I correctly
guess what you are trying to say, then I think it would be better
worded as something like:

[[
In addition to doing what
.BR LOOP_SET_STATUS
can do,
.BR LOOP_CONFIGURE
can also be used to the following:
.IP * 2
Set the correct block size immediately by setting
.IR loop_config.block_size ;
.IP *
explicitly request direct I/O mode by setting
.BR LO_FLAGS_DIRECT_IO
in loop_config.info.lo_flags;
.IP *
explicitly request read-only mode by setting
.BR LO_FLAGS_READ_ONLY
in
.IR loop_config.info.lo_flags .
]]

In other words, I think you mean to say that LOOP_CONFIGURE can be used
to any one of the things in the list, right?

If I'm right, could you redraft the patch please. If I'm not right, I
think we better discuss some more how the text should look :-).

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/