Message-ID: <YtiZ+gOmOFTpiAjW@magnolia>
Date:   Wed, 20 Jul 2022 17:12:42 -0700
From:   "Darrick J. Wong" <djwong@...nel.org>
To:     Jeremy Bongio <bongiojp@...il.com>
Cc:     Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org,
        linux-man@...r.kernel.org
Subject: Re: [PATCH v2] Add manpage for get/set fsuuid ioctl for ext4
 filesystem.

On Wed, Jul 20, 2022 at 04:45:12PM -0700, Jeremy Bongio wrote:
> Signed-off-by: Jeremy Bongio <bongiojp@...il.com>
> ---
> 
> This is a ext4 filesystem specific ioctl. However, this ioctl will
> likely be implemented for multiple filesystems at which point this
> manpage will be updated.
> 
>  man2/ioctl_fsuuid.2 | 115 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 115 insertions(+)
>  create mode 100644 man2/ioctl_fsuuid.2
> 
> diff --git a/man2/ioctl_fsuuid.2 b/man2/ioctl_fsuuid.2
> new file mode 100644
> index 000000000..53747684f
> --- /dev/null
> +++ b/man2/ioctl_fsuuid.2
> @@ -0,0 +1,115 @@
> +.\" Copyright (c) 2022 Google, Inc., written by Jeremy Bongio <bongiojp@...il.com>
> +.\"
> +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> +.TH IOCTL_FSUUID 2 2022-07-20 "Linux" "Linux Programmer's Manual"
> +.SH NAME
> +ioctl_fsuuid \- get or set an ext4 filesystem uuid
> +.SH LIBRARY
> +Standard C library
> +.RI ( libc ", " \-lc )

I'm not sure if libc will actually wrap this one, they often won't do
that for ioctls.

> +.SH SYNOPSIS
> +.nf
> +.B #include <sys/ioctl.h>
> +.PP
> +.BI "int ioctl(int " fd ", EXT4_IOC_GETFSUUID, struct " fsuuid ");"
> +.BI "int ioctl(int " fd ", EXT4_IOC_SETFSUUID, struct " fsuuid ");"
> +.fi
> +.SH DESCRIPTION
> +If an ext4 filesystem supports uuid manipulation, these
> +.BR ioctl (2)
> +operations can be used to get or set the uuid for the ext4 filesystem
> +on which
> +.I fd
> +resides.
> +.PP
> +The argument to these operations should be a pointer to a
> +.IR "struct fsuuid" ":"
> +.PP
> +.in +4n
> +.EX
> +struct fsuuid {
> +       __u32 fsu_len;      /* Number of bytes in a uuid */
> +       __u32 fsu_flags;    /* Mapping flags */
> +       __u8  fsu_uuid[];   /* Byte array for uuid */
> +};
> +.EE
> +.PP
> +The
> +.I fsu_flags
> +field must be set to 0. 

Nit: whitespace at the end of the line.

> +.PP
> +If an
> +.BR EXT4_IOC_GETFSUUID
> +operation is called with
> +.I fsu_len
> +set to 0,
> +.I fsu_len
> +will be reassigned the number of bytes in an ext4 filesystem uuid

"...will be set to the number of bytes..." ?

> +and the return code will be -EINVAL.
> +.PP
> +If an
> +.BR EXT4_IOC_GETFSUUID
> +operation is called with
> +.I fsu_len
> +set to the number of bytes in an ext4 filesystem uuid and
> +.I fsu_uuid
> +is allocated at least that many bytes, then
> +the filesystem uuid will be written to
> +.I fsu_uuid.

Hm.  It's not like the kernel actually checks the allocation -- if
fsu_len is set to the length of the filesystem's volume uuid, then
the that volume uuid will be written to fsu_uuid[].  How about:

"If EXT4_IOC_GETFSUUID is called with fsu_len matching the length of the
ext4 filesystem uuid, then that uuid will be written to fsu_uuid[] and
the return value will be zero.
If fsu_len does not match, the return value will be -EINVAL."

> +.PP
> +If an
> +.BR EXT4_IOC_SETFSUUID
> +operation is called with
> +.I fsu_len
> +set to the number of bytes in an ext4 filesystem uuid and
> +.I fsu_uuid
> +contains a uuid with 

Nit: whitespace at EOL.

> +.I fsu_uuid
> +bytes, then
> +the filesystem uuid will be set to
> +.I fsu_uuid.

"If EXT4_IOC_SETFSUUID is called with fsu_len matching the length of the
ext4 filesystem uuid, then the filesystem uuid will be set to the
contents of fsu_uuid[] and the return value will reflect the outcome of
the update operation.
If fsu_len does not match, the return value will be -EINVAL."

> +.PP
> +The
> +.B FS_IOC_SETFSUUID
> +operation requires privilege
> +.RB ( CAP_SYS_ADMIN ).
> +If the filesystem is currently being resized, an
> +.B EXT4_IOC_SETFSUUID
> +operation will wait until the resize is finished and the uuid can safely be set.
> +This may take a long time.

Why is resize called out here specifically?  Won't setfsuuid block on
/any/ operation that has tied up the filesystem superblocks?  I think
this could be more general:

"If the filesystem is busy, an EXT4_IOC_SETFSUUID operation will wait
until it can apply the uuid changes.
This may take a long time."

> +.PP
> +.SH RETURN VALUE
> +On success zero is returned.
> +On error, \-1 is returned, and
> +.I errno
> +is set to indicate the error.
> +.SH ERRORS
> +Possible errors include (but are not limited to) the following:
> +.TP
> +.B EFAULT
> +Either the pointer to the
> +.I fsuuid
> +structure is invalid or
> +.I fsu_uuid
> +has not been initialized properly.

Invalid?  Isn't that what EINVAL is for?

I think EFAULT is for "could not copy to/from userspace".

> +.TP
> +.B EINVAL
> +The specified arguments are invalid.
> +.I fsu_len
> +did not match the filesystem uuid length or
> +.I fsu_flags
> +has bits set that are not implemented.

"...not recognized."

If they're not implemented, shouldn't that be EOPNOTSUPP?

--D

> +.TP
> +.B ENOTTY
> +The filesystem does not support the ioctl.
> +.TP
> +.B EOPNOTSUPP
> +The filesystem does not currently support changing the uuid through this
> +ioctl. This may be due to incompatible feature flags.
> +.TP
> +.B EPERM
> +The calling process does not have sufficient permissions to set the uuid.
> +.SH CONFORMING TO
> +This API is Linux-specific.
> +.SH SEE ALSO
> +.BR ioctl (2)
> -- 
> 2.37.0.170.g444d1eabd0-goog
> 

Powered by blists - more mailing lists