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?
> 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.
...
> @@ -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)?
> 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?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
