Re: [PATCH 06/12] man: document the new allocation group geometry ioctl

Darrick J. Wong
2019-08-30T20:48:59Z

On Fri, Aug 30, 2019 at 03:53:47PM +1000, Dave Chinner wrote:
> On Tue, Aug 20, 2019 at 01:31:48PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Document the new ioctl to describe an allocation group's geometry.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > man/man2/ioctl_xfs_ag_geometry.2 | 74 ++++++++++++++++++++++++++++++++++++++
> > man/man3/xfsctl.3 | 6 +++
> > 2 files changed, 80 insertions(+)
> > create mode 100644 man/man2/ioctl_xfs_ag_geometry.2
> >
> >
> > diff --git a/man/man2/ioctl_xfs_ag_geometry.2 b/man/man2/ioctl_xfs_ag_geometry.2
> > new file mode 100644
> > index 00000000..5dfe0d08
> > --- /dev/null
> > +++ b/man/man2/ioctl_xfs_ag_geometry.2
> > @@ -0,0 +1,74 @@
> > +.\" Copyright (c) 2019, Oracle. All rights reserved.
> > +.\"
> > +.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
> > +.\" SPDX-License-Identifier: GPL-2.0+
> > +.\" %%%LICENSE_END
> > +.TH IOCTL-XFS-AG-GEOMETRY 2 2019-04-11 "XFS"
> > +.SH NAME
> > +ioctl_xfs_ag_geometry \- query XFS allocation group geometry information
> > +.SH SYNOPSIS
> > +.br
> > +.B #include <xfs/xfs_fs.h>
> > +.PP
> > +.BI "int ioctl(int " fd ", XFS_IOC_AG_GEOMETRY, struct xfs_ag_geometry *" arg );
> > +.SH DESCRIPTION
> > +This XFS ioctl retrieves the geometry information for a given allocation group.
> > +The geometry information is conveyed in a structure of the following form:
> > +.PP
> > +.in +4n
> > +.nf
> > +struct xfs_ag_geometry {
> > + uint32_t ag_number;
> > + uint32_t ag_length;
> > + uint32_t ag_freeblks;
> > + uint32_t ag_icount;
> > + uint32_t ag_ifree;
> > + uint32_t ag_sick;
> > + uint32_t ag_checked;
> > + uint32_t ag_reserved32;
> > + uint64_t ag_reserved[12];
>
> Where's the flags field for feature versioning? Please don't tell me
> we merged an ioctl structure without a flags or version field in
> it...

Yes, we did, though the "reserved fields are always zeroed" enables us
to retroactively define this to v0 of the structure.

> > +};
> > +.fi
> > +.in
> > +.TP
> > +.I ag_number
> > +The number of allocation group that the caller wishes to learn about.
>
> "the index of"....
>
> "The number of" is easily confused with a quantity....
>
> Is this an input or an output?

Purely an input.

"The caller must set this field to the index of the allocation group
that the caller wishes to learn about." ?

> > +.TP
> > +.I ag_length
> > +Length of the allocation group, in units of filesystem blocks.
>
> The length of the AG is returned in this field, in units....
>
> Same for the rest...

Ok.

> > +.TP
> > +.I ag_freeblks
> > +Number of free blocks in the allocation group, in units of filesystem blocks.
> > +.TP
> > +.I ag_icount
> > +Number of inode records allocated in this allocation group.
> > +.TP
> > +.I ag_ifree
> > +Number of unused inode records (of the space allocated) in this allocation
> > +group.
> > +.TP
> > +.IR ag_reserved " and " ag_reserved32
> > +Will be set to zero.
>
> It would be better to say "all reserved feilds will be set to zero
> on return" so that we don't have to change this every time we rev
> the structure....

Ok.

--D

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com