On Tue, Aug 27, 2019 at 04:30:54PM +1000, Dave Chinner wrote:
> On Tue, Aug 20, 2019 at 01:30:38PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Refactor all the open-coded XFS_IOC_FSGEOMETRY queries into a single
> > helper that we can use to standardize behaviors across mixed xfslibs
> > versions. This is the prelude to introducing a new FSGEOMETRY version
> > in 5.2 and needing to fix the (relatively few) client programs.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > Makefile | 1 +
> > fsr/xfs_fsr.c | 26 ++++----------------------
>
> Ok.
>
> > growfs/xfs_growfs.c | 25 +++++++++----------------
>
> Nit.
>
> > include/xfrog.h | 22 ++++++++++++++++++++++
>
> Copyright issue.
>
> > io/bmap.c | 3 ++-
> > io/fsmap.c | 3 ++-
> > io/open.c | 3 ++-
> > io/stat.c | 5 +++--
>
> ok.
>
> > libfrog/fsgeom.c | 18 ++++++++++++++++++
>
> fallback question.
>
> > quota/free.c | 6 +++---
> > repair/xfs_repair.c | 5 +++--
> > rtcp/Makefile | 3 +++
>
> Linker question
>
> > rtcp/xfs_rtcp.c | 7 ++++---
> > scrub/phase1.c | 5 +++--
> > spaceman/file.c | 3 ++-
> > spaceman/info.c | 27 +++++++++------------------
>
> OK.
>
>
> > diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
> > index 20089d2b..86b1d542 100644
> > --- a/growfs/xfs_growfs.c
> > +++ b/growfs/xfs_growfs.c
> > @@ -7,6 +7,7 @@
> > #include "libxfs.h"
> > #include "path.h"
> > #include "fsgeom.h"
> > +#include "xfrog.h"
> >
> > static void
> > usage(void)
> > @@ -165,22 +166,14 @@ main(int argc, char **argv)
> > }
> >
> > /* get the current filesystem size & geometry */
> > - if (xfsctl(fname, ffd, XFS_IOC_FSGEOMETRY, &geo) < 0) {
> > - /*
> > - * OK, new xfsctl barfed - back off and try earlier version
> > - * as we're probably running an older kernel version.
> > - * Only field added in the v2 geometry xfsctl is "logsunit"
> > - * so we'll zero that out for later display (as zero).
> > - */
> > - geo.logsunit = 0;
> > - if (xfsctl(fname, ffd, XFS_IOC_FSGEOMETRY_V1, &geo) < 0) {
> > - fprintf(stderr, _(
> > - "%s: cannot determine geometry of filesystem"
> > - " mounted at %s: %s\n"),
> > - progname, fname, strerror(errno));
> > - exit(1);
> > - }
> > + if (xfrog_geometry(ffd, &geo) < 0) {
> > + fprintf(stderr, _(
> > + "%s: cannot determine geometry of filesystem"
> > + " mounted at %s: %s\n"),
> > + progname, fname, strerror(errno));
> > + exit(1);
>
> Can you run the format string into a single line?
ok.
> > diff --git a/include/xfrog.h b/include/xfrog.h
> > new file mode 100644
> > index 00000000..5420b47c
> > --- /dev/null
> > +++ b/include/xfrog.h
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2000-2002 Silicon Graphics, Inc.
> > + * All Rights Reserved.
> > + */
>
> I don't think that copyright is valid/appropriate for a new header
> file with new contents.
Hrmmm ok I'll uhh fix it.
> ...
>
> > @@ -67,3 +68,20 @@ xfs_report_geom(
> > geo->rtextsize * geo->blocksize, (unsigned long long)geo->rtblocks,
> > (unsigned long long)geo->rtextents);
> > }
> > +
> > +/* Try to obtain the xfs geometry. */
> > +int
> > +xfrog_geometry(
> > + int fd,
> > + struct xfs_fsop_geom *fsgeo)
> > +{
> > + int ret;
> > +
> > + memset(fsgeo, 0, sizeof(*fsgeo));
> > +
> > + ret = ioctl(fd, XFS_IOC_FSGEOMETRY, fsgeo);
> > + if (!ret)
> > + return 0;
> > +
> > + return ioctl(fd, XFS_IOC_FSGEOMETRY_V1, fsgeo);
> > +}
>
> Should this fall back to XFS_IOC_FSGEOMETRY_V4, then V1 if that
> fails (which it shouldn't on any supported kernel)?
It should; there's a patch in the next series to do that, but I might as
well merge them (this series was originally targeting 5.1).
>
> > diff --git a/rtcp/Makefile b/rtcp/Makefile
> > index 808b5378..264b4f27 100644
> > --- a/rtcp/Makefile
> > +++ b/rtcp/Makefile
> > @@ -9,6 +9,9 @@ LTCOMMAND = xfs_rtcp
> > CFILES = xfs_rtcp.c
> > LLDFLAGS = -static
> >
> > +LLDLIBS = $(LIBFROG)
> > +LTDEPENDENCIES = $(LIBFROG)
>
> Does this work correctly given LLDFLAGS sets -static and not
> -libtools-static-libs?
It seems to, at least on Ubuntu 18.04.
--D
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
