Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 22 Jul 2022 10:31:29 -0700
From:   "Darrick J. Wong" <djwong@kernel.org>
To:     Jeremy Bongio <bongiojp@gmail.com>
Cc:     Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org,
        linux-man@vger.kernel.org
Subject: Re: [PATCH v3] Add manpage for get/set fsuuid ioctl for ext4
 filesystem.
Message-ID: <Ytre8bHCS+S12/8M@magnolia>
References: <20220722063732.466621-1-bongiojp@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220722063732.466621-1-bongiojp@gmail.com>
Precedence: bulk

On Thu, Jul 21, 2022 at 11:37:32PM -0700, Jeremy Bongio wrote:
> Signed-off-by: Jeremy Bongio <bongiojp@gmail.com>
> ---
> 
> Changes in v3:
> 
> Removed LIBRARY section since this ioctl won't be wrapped by a system library.
> 
> Code now uses 4-space indent.
> 
> Updated for semantic new lines. Please call out if I still break this rule.
> 
> Reworded based on Darrick's suggestions.
> 
> Unrecognized fsu_flags now results in EOPNOTSUPP error.
> 
> Added definitions of EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID which are not
> contained in a library.
> 
>  man2/ioctl_fsuuid.2 | 113 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 113 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..c9e2789b9
> --- /dev/null
> +++ b/man2/ioctl_fsuuid.2
> @@ -0,0 +1,113 @@
> +.\" Copyright (c) 2022 Google, Inc., written by Jeremy Bongio <bongiojp@gmail.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 SYNOPSIS
> +.nf
> +.B #include <sys/ioctl.h>
> +.BR "#include <linux/fs.h>" "         /* Definition of " _IOR " and " _IOW " */"
> +.PP
> +.BI "#define EXT4_IOC_GETFSUUID      _IOR('f', 44, struct fsuuid)"
> +.BI "#define EXT4_IOC_SETFSUUID      _IOW('f', 44, struct fsuuid)"
> +.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.
> +.PP
> +If
> +.BR EXT4_IOC_GETFSUUID
> +is called with
> +.I fsu_len
> +set to 0,
> +.I fsu_len
> +will be set to the number of bytes in an ext4 filesystem uuid
> +and the return code will be EINVAL.
> +.PP
> +If
> +.BR EXT4_IOC_GETFSUUID
> +is called with
> +.I fsu_len
> +matching the length of the ext4 filesystem uuid,
> +then that uuid will be written to
> +.I fsu_uuid[]
> +and the return value will be zero.
> +If
> +.I fsu_len
> +does not match, the return value will be
> +.B EINVAL.
> +.PP
> +If
> +.BR EXT4_IOC_SETFSUUID
> +is called with
> +.I fsu_len
> +matching the length of the ext4 filesystem uuid,
> +then the filesystem uuid will be set to the contents of
> +.I fsu_uuid[]
> +and  the return value will reflect the outcome of the update operation.
> +If 

Nit: trailing space.

> +.I fsu_len
> +does not match, the return value will be
> +.B EINVAL.
> +.PP
> +The
> +.BR FS_IOC_SETFSUUID
> +operation requires privilege
> +.RB ( CAP_SYS_ADMIN ).
> +If the filesystem is busy, an
> +.BR 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
> +Could not copy to/from userspace.
> +.TP
> +.B EINVAL
> +.I fsu_len
> +did not match the filesystem uuid length.
> +.TP
> +.B ENOTTY
> +The filesystem does not support the ioctl.
> +.TP
> +.B EOPNOTSUPP
> +The filesystem does not support changing the uuid through this ioctl.
> +This may be due to incompatible filesystem feature flags or
> +.I fsu_flags
> +has bits set that are not recognized.

Hm, I think I miscommunicated last time.  fsu_flags bits that are not
recognized by the implementation should return EINVAL.  fsu_flag bits
that *are* recognized but impossible to service should return
EOPNOTSUPP.

.TP
.B EINVAL
.I fsu_len
did not match the filesystem uuid length, or
.I fsu_flags
had bits set that the implementation does not understand.

.TP
.B EOPNOTSUPP
The filesystem does not support changing the uuid through this ioctl.
This may be due to incompatible filesystem feature flags or
.I fsu_flags
had bits set that make the operation impossible.

FWIW I agree with the manpages maintainer that this should have a
section with example code.  I don't know if he requires the example code
be embedded in the manpage, or if it would be sufficient to point
readers at a real implementation (e.g. uuid_ioctl.c in fstests, or the
relevant helper functions in tune2fs.c in e2fsprogs).

--D

> +.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.1.359.gd136c6c3e2-goog
>