[PATCH 00/12] xfsprogs-5.1: fix various problems

[PATCH 00/12] xfsprogs-5.1: fix various problems

[Darrick J. Wong]

Hi all,

Fix various problems in the xfsprogs-specific parts of the libxfs code.

The first two patches fix some minor bugs in libxfs.

Patches 3-6 refactor all utilities to use common libhandle functions to
retrieve the filesystem geometry, bulkstat, and inumbers.  The helpers
will make it easier for newer userspace to fall back to older versions
of ioctls.

Patch 7 fixes the return types on the libfrog bitmap code functions to
be more general and less scrub specific.

Patch 8 ports xfs_repair to use the libxfs dirent and attr name check
functions.

Patch 9 reworks the xfs_scrub throttling function to use named constants
instead of magic values to make it easier to verify that it actually
does what the manpage says.

Patch 10 enables mkfs to set the DAX flag on the root directory.

Patch 11 strengthens mkfs's log alignment checking code.

Patch 12 enables reflink by default in mkfs.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=xfsprogs-5.1-fixes

[PATCH 01/12] libxfs: fix attr include mess

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Remove all the userspace xfs_attr shim cruft so that we have one
definition of ATTR_* macros so that we can actually use xfs_attr.c
routines and include xfs_attr.h without problems.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/libxfs.h         |   10 +---------
 libxfs/libxfs_api_defs.h |    5 +++++
 libxfs/libxfs_priv.h     |    8 --------
 libxfs/xfs_attr.c        |    1 +
 libxfs/xfs_attr_leaf.c   |    1 +
 5 files changed, 8 insertions(+), 17 deletions(-)


diff --git a/include/libxfs.h b/include/libxfs.h
index 230bc3e8..dd5fe542 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -211,14 +211,6 @@ libxfs_bmbt_disk_get_all(
 int libxfs_rtfree_extent(struct xfs_trans *, xfs_rtblock_t, xfs_extlen_t);
 bool libxfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
 
-/* XXX: need parts of xfs_attr.h in userspace */
-#define LIBXFS_ATTR_ROOT	0x0002	/* use attrs in root namespace */
-#define LIBXFS_ATTR_SECURE	0x0008	/* use attrs in security namespace */
-#define LIBXFS_ATTR_CREATE	0x0010	/* create, but fail if attr exists */
-#define LIBXFS_ATTR_REPLACE	0x0020	/* set, but fail if attr not exists */
-
-int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
-int xfs_attr_set(struct xfs_inode *dp, const unsigned char *name,
-		 unsigned char *value, int valuelen, int flags);
+#include "xfs_attr.h"
 
 #endif	/* __LIBXFS_H__ */
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 1150ec93..34bb552d 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -144,4 +144,9 @@
 #define xfs_fs_geometry			libxfs_fs_geometry
 #define xfs_init_local_fork		libxfs_init_local_fork
 
+#define LIBXFS_ATTR_ROOT		ATTR_ROOT
+#define LIBXFS_ATTR_SECURE		ATTR_SECURE
+#define LIBXFS_ATTR_CREATE		ATTR_CREATE
+#define LIBXFS_ATTR_REPLACE		ATTR_REPLACE
+
 #endif /* __LIBXFS_API_DEFS_H__ */
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index d668a157..f60bff06 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -104,14 +104,6 @@ extern char    *progname;
  */
 #define PTR_FMT "%p"
 
-/* XXX: need to push these out to make LIBXFS_ATTR defines */
-#define ATTR_ROOT			0x0002
-#define ATTR_SECURE			0x0008
-#define ATTR_CREATE			0x0010
-#define ATTR_REPLACE			0x0020
-#define ATTR_KERNOTIME			0
-#define ATTR_KERNOVAL			0
-
 #define XFS_IGET_CREATE			0x1
 #define XFS_IGET_UNTRUSTED		0x2
 
diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index b8838302..170e64cf 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -20,6 +20,7 @@
 #include "xfs_trans.h"
 #include "xfs_bmap.h"
 #include "xfs_bmap_btree.h"
+#include "xfs_attr.h"
 #include "xfs_attr_leaf.h"
 #include "xfs_attr_remote.h"
 #include "xfs_trans_space.h"
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index 679c7d0d..1027ca01 100644
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -21,6 +21,7 @@
 #include "xfs_bmap.h"
 #include "xfs_attr_sf.h"
 #include "xfs_attr_remote.h"
+#include "xfs_attr.h"
 #include "xfs_attr_leaf.h"
 #include "xfs_trace.h"
 #include "xfs_cksum.h"

[PATCH 02/12] libxfs: set m_finobt_nores when initializing library

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

We don't generally set up per-ag reservations in userspace, so set this
flag to true to force transactions to set up block reservations.  This
isn't necessary for userspace (since we never touch the finobt) but we
shouldn't leave a logic bomb.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/init.c |    1 +
 1 file changed, 1 insertion(+)


diff --git a/libxfs/init.c b/libxfs/init.c
index 2f6decc8..1baccb31 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -640,6 +640,7 @@ libxfs_mount(
 
 	libxfs_buftarg_init(mp, dev, logdev, rtdev);
 
+	mp->m_finobt_nores = true;
 	mp->m_flags = (LIBXFS_MOUNT_32BITINODES|LIBXFS_MOUNT_32BITINOOPT);
 	mp->m_sb = *sb;
 	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_KERNEL);

[PATCH 03/12] libxfs: refactor online geometry queries

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

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@oracle.com>
---
 Makefile            |    9 +++++----
 fsr/xfs_fsr.c       |   25 +++----------------------
 growfs/Makefile     |    5 +++--
 growfs/xfs_growfs.c |   24 ++++++++----------------
 include/linux.h     |    5 +++++
 io/bmap.c           |    2 +-
 io/fsmap.c          |    2 +-
 io/open.c           |    2 +-
 io/stat.c           |    4 ++--
 libhandle/Makefile  |    2 +-
 libhandle/ioctl.c   |   26 ++++++++++++++++++++++++++
 quota/Makefile      |    4 ++--
 quota/free.c        |    5 ++---
 repair/Makefile     |    6 +++---
 repair/xfs_repair.c |    4 ++--
 rtcp/Makefile       |    3 +++
 rtcp/xfs_rtcp.c     |    6 +++---
 scrub/phase1.c      |    2 +-
 spaceman/Makefile   |    4 ++--
 spaceman/file.c     |    2 +-
 spaceman/info.c     |   24 +++++++-----------------
 21 files changed, 82 insertions(+), 84 deletions(-)
 create mode 100644 libhandle/ioctl.c


diff --git a/Makefile b/Makefile
index 9204bed8..b72a9209 100644
--- a/Makefile
+++ b/Makefile
@@ -99,14 +99,15 @@ $(LIB_SUBDIRS) $(TOOL_SUBDIRS): include libfrog
 $(DLIB_SUBDIRS) $(TOOL_SUBDIRS): libxfs
 db logprint: libxlog
 fsr: libhandle
-growfs: libxcmd
+growfs: libxcmd libhandle
 io: libxcmd libhandle
-quota: libxcmd
-repair: libxlog libxcmd
+quota: libxcmd libhandle
+repair: libxlog libxcmd libhandle
 copy: libxlog
 mkfs: libxcmd
-spaceman: libxcmd
+spaceman: libxcmd libhandle
 scrub: libhandle libxcmd
+rtcp: libhandle
 
 ifeq ($(HAVE_BUILDDEFS), yes)
 include $(BUILDRULES)
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index fef6262c..968d133c 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -83,9 +83,8 @@ int cmp(const void *, const void *);
 static void tmp_init(char *mnt);
 static char * tmp_next(char *mnt);
 static void tmp_close(char *mnt);
-int xfs_getgeom(int , xfs_fsop_geom_v1_t * );
 
-static xfs_fsop_geom_v1_t fsgeom;	/* geometry of active mounted system */
+static struct xfs_fsop_geom fsgeom;	/* geometry of active mounted system */
 
 #define NMOUNT 64
 static int numfs;
@@ -102,12 +101,6 @@ static int	nfrags = 0;	/* Debug option: Coerse into specific number
 				 * of extents */
 static int	openopts = O_CREAT|O_EXCL|O_RDWR|O_DIRECT;
 
-static int
-xfs_fsgeometry(int fd, xfs_fsop_geom_v1_t *geom)
-{
-    return ioctl(fd, XFS_IOC_FSGEOMETRY_V1, geom);
-}
-
 static int
 xfs_bulkstat_single(int fd, xfs_ino_t *lastip, xfs_bstat_t *ubuffer)
 {
@@ -630,7 +623,7 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
 		return -1;
 	}
 
-	if (xfs_getgeom(fsfd, &fsgeom) < 0 ) {
+	if (xfs_fsgeometry(fsfd, &fsgeom) < 0 ) {
 		fsrprintf(_("Skipping %s: could not get XFS geometry\n"),
 			  mntdir);
 		close(fsfd);
@@ -772,7 +765,7 @@ fsrfile(char *fname, xfs_ino_t ino)
 	}
 
 	/* Get the fs geometry */
-	if (xfs_getgeom(fsfd, &fsgeom) < 0 ) {
+	if (xfs_fsgeometry(fsfd, &fsgeom) < 0 ) {
 		fsrprintf(_("Unable to get geom on fs for: %s\n"), fname);
 		goto out;
 	}
@@ -1612,18 +1605,6 @@ getnextents(int fd)
 	return(nextents);
 }
 
-/*
- * Get the fs geometry
- */
-int
-xfs_getgeom(int fd, xfs_fsop_geom_v1_t * fsgeom)
-{
-	if (xfs_fsgeometry(fd, fsgeom) < 0) {
-		return -1;
-	}
-	return 0;
-}
-
 /*
  * Get xfs realtime space information
  */
diff --git a/growfs/Makefile b/growfs/Makefile
index 4104cc0d..e0964587 100644
--- a/growfs/Makefile
+++ b/growfs/Makefile
@@ -9,7 +9,8 @@ LTCOMMAND = xfs_growfs
 
 CFILES = xfs_growfs.c
 
-LLDLIBS = $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
+LLDLIBS = $(LIBHANDLE) $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBUUID) $(LIBRT) \
+	  $(LIBPTHREAD)
 ifeq ($(ENABLE_READLINE),yes)
 LLDLIBS += $(LIBREADLINE) $(LIBTERMCAP)
 endif
@@ -18,7 +19,7 @@ ifeq ($(ENABLE_EDITLINE),yes)
 LLDLIBS += $(LIBEDITLINE) $(LIBTERMCAP)
 endif
 
-LTDEPENDENCIES = $(LIBXFS) $(LIBXCMD) $(LIBFROG)
+LTDEPENDENCIES = $(LIBHANDLE) $(LIBXFS) $(LIBXCMD) $(LIBFROG)
 LLDFLAGS = -static-libtool-libs
 
 default: depend $(LTCOMMAND)
diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index 2af7e5ba..392e4a00 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -165,22 +165,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 (xfs_fsgeometry(ffd, &geo)) {
+		fprintf(stderr, _(
+			"%s: cannot determine geometry of filesystem"
+			" mounted at %s: %s\n"),
+			progname, fname, strerror(errno));
+		exit(1);
 	}
+
 	isint = geo.logstart > 0;
 
 	/*
@@ -359,7 +351,7 @@ main(int argc, char **argv)
 		}
 	}
 
-	if (xfsctl(fname, ffd, XFS_IOC_FSGEOMETRY_V1, &ngeo) < 0) {
+	if (xfs_fsgeometry(ffd, &ngeo)) {
 		fprintf(stderr, _("%s: XFS_IOC_FSGEOMETRY xfsctl failed: %s\n"),
 			progname, strerror(errno));
 		exit(1);
diff --git a/include/linux.h b/include/linux.h
index 8f3c32b0..5fe33117 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -323,4 +323,9 @@ fsmap_advance(
 #include <asm-generic/mman-common.h>
 #endif /* HAVE_MAP_SYNC */
 
+/* ioctl wrappers */
+
+struct xfs_fsop_geom;
+int xfs_fsgeometry(int fd, struct xfs_fsop_geom *fsgeo);
+
 #endif	/* __XFS_LINUX_H__ */
diff --git a/io/bmap.c b/io/bmap.c
index d408826a..9103fe72 100644
--- a/io/bmap.c
+++ b/io/bmap.c
@@ -105,7 +105,7 @@ bmap_f(
 		bmv_iflags &= ~(BMV_IF_PREALLOC|BMV_IF_NO_DMAPI_READ);
 
 	if (vflag) {
-		c = xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo);
+		c = xfs_fsgeometry(file->fd, &fsgeo);
 		if (c < 0) {
 			fprintf(stderr,
 				_("%s: can't get geometry [\"%s\"]: %s\n"),
diff --git a/io/fsmap.c b/io/fsmap.c
index 477c36fc..9c310bd8 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -447,7 +447,7 @@ fsmap_f(
 	}
 
 	if (vflag) {
-		c = ioctl(file->fd, XFS_IOC_FSGEOMETRY, &fsgeo);
+		c = xfs_fsgeometry(file->fd, &fsgeo);
 		if (c < 0) {
 			fprintf(stderr,
 				_("%s: can't get geometry [\"%s\"]: %s\n"),
diff --git a/io/open.c b/io/open.c
index a406ea54..b6aacb83 100644
--- a/io/open.c
+++ b/io/open.c
@@ -118,7 +118,7 @@ openfile(
 	if (flags & IO_PATH) {
 		/* Can't call ioctl() on O_PATH fds */
 		memset(geom, 0, sizeof(*geom));
-	} else if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
+	} else if (xfs_fsgeometry(fd, geom)) {
 		perror("XFS_IOC_FSGEOMETRY");
 		close(fd);
 		return -1;
diff --git a/io/stat.c b/io/stat.c
index 37c0b2e8..5399ed4f 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -194,8 +194,8 @@ statfs_f(
 	}
 	if (file->flags & IO_FOREIGN)
 		return 0;
-	if ((xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo)) < 0) {
-		perror("XFS_IOC_FSGEOMETRY_V1");
+	if (xfs_fsgeometry(file->fd, &fsgeo)) {
+		perror("XFS_IOC_FSGEOMETRY");
 	} else {
 		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
 		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
diff --git a/libhandle/Makefile b/libhandle/Makefile
index f297a59e..a358549c 100644
--- a/libhandle/Makefile
+++ b/libhandle/Makefile
@@ -12,7 +12,7 @@ LT_AGE = 0
 
 LTLDFLAGS += -Wl,--version-script,libhandle.sym
 
-CFILES = handle.c jdm.c
+CFILES = handle.c ioctl.c jdm.c
 LSRCFILES = libhandle.sym
 
 default: ltdepend $(LTLIBRARY)
diff --git a/libhandle/ioctl.c b/libhandle/ioctl.c
new file mode 100644
index 00000000..5c954bd0
--- /dev/null
+++ b/libhandle/ioctl.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2019 Oracle.  All Rights Reserved.
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ */
+#include "xfs.h"
+#include <string.h>
+
+/* Wrappers for Linux ioctls. */
+
+/* Try to obtain the xfs geometry. */
+int
+xfs_fsgeometry(
+	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);
+}
diff --git a/quota/Makefile b/quota/Makefile
index 384f023a..db6174b9 100644
--- a/quota/Makefile
+++ b/quota/Makefile
@@ -10,8 +10,8 @@ HFILES = init.h quota.h
 CFILES = init.c util.c \
 	edit.c free.c linux.c path.c project.c quot.c quota.c report.c state.c
 
-LLDLIBS = $(LIBXCMD) $(LIBFROG)
-LTDEPENDENCIES = $(LIBXCMD) $(LIBFROG)
+LLDLIBS = $(LIBHANDLE) $(LIBXCMD) $(LIBFROG)
+LTDEPENDENCIES = $(LIBHANDLE) $(LIBXCMD) $(LIBFROG)
 LLDFLAGS = -static
 
 ifeq ($(ENABLE_READLINE),yes)
diff --git a/quota/free.c b/quota/free.c
index 1d13006e..e95af8eb 100644
--- a/quota/free.c
+++ b/quota/free.c
@@ -67,9 +67,8 @@ mount_free_space_data(
 	}
 
 	if (!(mount->fs_flags & FS_FOREIGN)) {
-		if ((xfsctl(mount->fs_dir, fd, XFS_IOC_FSGEOMETRY_V1,
-							&fsgeo)) < 0) {
-			perror("XFS_IOC_FSGEOMETRY_V1");
+		if (xfs_fsgeometry(fd, &fsgeo)) {
+			perror("XFS_IOC_FSGEOMETRY");
 			close(fd);
 			return 0;
 		}
diff --git a/repair/Makefile b/repair/Makefile
index 0964499a..a5cab8f1 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -20,9 +20,9 @@ CFILES = agheader.c attr_repair.c avl.c bmap.c btree.c \
 	progress.c prefetch.c rmap.c rt.c sb.c scan.c slab.c threads.c \
 	versions.c xfs_repair.c
 
-LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBXCMD) $(LIBFROG) $(LIBUUID) $(LIBRT) \
-	$(LIBPTHREAD) $(LIBBLKID)
-LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBXCMD) $(LIBFROG)
+LLDLIBS = $(LIBHANDLE) $(LIBXFS) $(LIBXLOG) $(LIBXCMD) $(LIBFROG) $(LIBUUID) \
+	  $(LIBRT) $(LIBPTHREAD) $(LIBBLKID)
+LTDEPENDENCIES = $(LIBHANDLE) $(LIBXFS) $(LIBXLOG) $(LIBXCMD) $(LIBFROG)
 LLDFLAGS = -static-libtool-libs
 
 default: depend $(LTCOMMAND)
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index 9657503f..a569b958 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -636,11 +636,11 @@ check_fs_vs_host_sectsize(
 {
 	int	fd;
 	long	old_flags;
-	struct xfs_fsop_geom_v1 geom = { 0 };
+	struct xfs_fsop_geom	geom = { 0 };
 
 	fd = libxfs_device_to_fd(x.ddev);
 
-	if (ioctl(fd, XFS_IOC_FSGEOMETRY_V1, &geom) < 0) {
+	if (xfs_fsgeometry(fd, &geom)) {
 		do_log(_("Cannot get host filesystem geometry.\n"
 	"Repair may fail if there is a sector size mismatch between\n"
 	"the image and the host filesystem.\n"));
diff --git a/rtcp/Makefile b/rtcp/Makefile
index 808b5378..e51ba165 100644
--- a/rtcp/Makefile
+++ b/rtcp/Makefile
@@ -9,6 +9,9 @@ LTCOMMAND = xfs_rtcp
 CFILES = xfs_rtcp.c
 LLDFLAGS = -static
 
+LLDLIBS = $(LIBHANDLE)
+LTDEPENDENCIES = $(LIBHANDLE)
+
 default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
diff --git a/rtcp/xfs_rtcp.c b/rtcp/xfs_rtcp.c
index f928a86a..3a9e9a2f 100644
--- a/rtcp/xfs_rtcp.c
+++ b/rtcp/xfs_rtcp.c
@@ -368,8 +368,8 @@ rtcp( char *source, char *target, int fextsize)
 int
 xfsrtextsize( char *path)
 {
-	int fd, rval, rtextsize;
-	xfs_fsop_geom_v1_t geo;
+	struct xfs_fsop_geom	geo;
+	int			fd, rval, rtextsize;
 
 	fd = open( path, O_RDONLY );
 	if ( fd < 0 ) {
@@ -377,7 +377,7 @@ xfsrtextsize( char *path)
 			progname, path, strerror(errno));
 		return -1;
 	}
-	rval = xfsctl( path, fd, XFS_IOC_FSGEOMETRY_V1, &geo );
+	rval = xfs_fsgeometry(fd, &geo);
 	close(fd);
 	if ( rval < 0 )
 		return -1;
diff --git a/scrub/phase1.c b/scrub/phase1.c
index 04a5f4a9..a4e9c9af 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -129,7 +129,7 @@ _("Does not appear to be an XFS filesystem!"));
 	}
 
 	/* Retrieve XFS geometry. */
-	error = ioctl(ctx->mnt_fd, XFS_IOC_FSGEOMETRY, &ctx->geo);
+	error = xfs_fsgeometry(ctx->mnt_fd, &ctx->geo);
 	if (error) {
 		str_errno(ctx, ctx->mntpoint);
 		return false;
diff --git a/spaceman/Makefile b/spaceman/Makefile
index b1c1b16d..69c70531 100644
--- a/spaceman/Makefile
+++ b/spaceman/Makefile
@@ -10,8 +10,8 @@ HFILES = init.h space.h
 CFILES = info.c init.c file.c prealloc.c trim.c
 LSRCFILES = xfs_info.sh
 
-LLDLIBS = $(LIBXCMD) $(LIBFROG)
-LTDEPENDENCIES = $(LIBXCMD) $(LIBFROG)
+LLDLIBS = $(LIBHANDLE) $(LIBXCMD) $(LIBFROG)
+LTDEPENDENCIES = $(LIBHANDLE) $(LIBXCMD) $(LIBFROG)
 LLDFLAGS = -static
 
 ifeq ($(ENABLE_READLINE),yes)
diff --git a/spaceman/file.c b/spaceman/file.c
index 98549517..d2acf5db 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -56,7 +56,7 @@ openfile(
 		return -1;
 	}
 
-	if (ioctl(fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
+	if (xfs_fsgeometry(fd, geom)) {
 		if (errno == ENOTTY)
 			fprintf(stderr,
 _("%s: Not on a mounted XFS filesystem.\n"),
diff --git a/spaceman/info.c b/spaceman/info.c
index 01d0744a..bcefdcb8 100644
--- a/spaceman/info.c
+++ b/spaceman/info.c
@@ -37,24 +37,14 @@ info_f(
 	}
 
 	/* get the current filesystem size & geometry */
-	error = ioctl(file->fd, XFS_IOC_FSGEOMETRY, &geo);
+	error = xfs_fsgeometry(file->fd, &geo);
 	if (error) {
-		/*
-		 * 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;
-		error = ioctl(file->fd, XFS_IOC_FSGEOMETRY_V1, &geo);
-		if (error) {
-			fprintf(stderr, _(
-				"%s: cannot determine geometry of filesystem"
-				" mounted at %s: %s\n"),
-				progname, file->name, strerror(errno));
-			exitcode = 1;
-			return 0;
-		}
+		fprintf(stderr, _(
+			"%s: cannot determine geometry of filesystem"
+			" mounted at %s: %s\n"),
+			progname, file->name, strerror(errno));
+		exitcode = 1;
+		return 0;
 	}
 
 	xfs_report_geom(&geo, file->fs_path.fs_name, file->fs_path.fs_log,

[PATCH 04/12] libxfs: refactor open-coded bulkstat calls

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor the BULKSTAT_SINGLE and BULKSTAT ioctl callsites into helper
functions.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fsr/xfs_fsr.c     |   33 ++-------------------------
 include/linux.h   |    5 ++++
 io/open.c         |   66 +++++++++++++++++++++++++----------------------------
 io/swapext.c      |   17 +-------------
 libhandle/ioctl.c |   38 +++++++++++++++++++++++++++++++
 quota/quot.c      |   16 ++++---------
 scrub/inodes.c    |   27 ++++++----------------
 7 files changed, 90 insertions(+), 112 deletions(-)


diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 968d133c..bc5cf9ed 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -101,31 +101,6 @@ static int	nfrags = 0;	/* Debug option: Coerse into specific number
 				 * of extents */
 static int	openopts = O_CREAT|O_EXCL|O_RDWR|O_DIRECT;
 
-static int
-xfs_bulkstat_single(int fd, xfs_ino_t *lastip, xfs_bstat_t *ubuffer)
-{
-    xfs_fsop_bulkreq_t  bulkreq;
-
-    bulkreq.lastip = (__u64 *)lastip;
-    bulkreq.icount = 1;
-    bulkreq.ubuffer = ubuffer;
-    bulkreq.ocount = NULL;
-    return ioctl(fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
-}
-
-static int
-xfs_bulkstat(int fd, xfs_ino_t *lastip, int icount,
-                    xfs_bstat_t *ubuffer, __s32 *ocount)
-{
-    xfs_fsop_bulkreq_t  bulkreq;
-
-    bulkreq.lastip = (__u64 *)lastip;
-    bulkreq.icount = icount;
-    bulkreq.ubuffer = ubuffer;
-    bulkreq.ocount = ocount;
-    return ioctl(fd, XFS_IOC_FSBULKSTAT, &bulkreq);
-}
-
 static int
 xfs_swapext(int fd, xfs_swapext_t *sx)
 {
@@ -599,7 +574,7 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
 	int	fsfd, fd;
 	int	count = 0;
 	int	ret;
-	__s32	buflenout;
+	uint32_t buflenout;
 	xfs_bstat_t buf[GRABSZ];
 	char	fname[64];
 	char	*tname;
@@ -751,7 +726,7 @@ fsrfile(char *fname, xfs_ino_t ino)
 		goto out;
 	}
 
-	if ((xfs_bulkstat_single(fsfd, &ino, &statbuf)) < 0) {
+	if ((xfs_bulkstat_single(fsfd, ino, &statbuf)) < 0) {
 		fsrprintf(_("unable to get bstat on %s: %s\n"),
 			fname, strerror(errno));
 		goto out;
@@ -980,7 +955,6 @@ fsr_setup_attr_fork(
 	i = 0;
 	do {
 		xfs_bstat_t	tbstat;
-		xfs_ino_t	ino;
 		char		name[64];
 
 		/*
@@ -988,8 +962,7 @@ fsr_setup_attr_fork(
 		 * this to compare against the target and determine what we
 		 * need to do.
 		 */
-		ino = tstatbuf.st_ino;
-		if ((xfs_bulkstat_single(tfd, &ino, &tbstat)) < 0) {
+		if ((xfs_bulkstat_single(tfd, tstatbuf.st_ino, &tbstat)) < 0) {
 			fsrprintf(_("unable to get bstat on temp file: %s\n"),
 						strerror(errno));
 			return -1;
diff --git a/include/linux.h b/include/linux.h
index 5fe33117..98750e18 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -328,4 +328,9 @@ fsmap_advance(
 struct xfs_fsop_geom;
 int xfs_fsgeometry(int fd, struct xfs_fsop_geom *fsgeo);
 
+struct xfs_bstat;
+int xfs_bulkstat_single(int fd, uint64_t ino, struct xfs_bstat *ubuffer);
+int xfs_bulkstat(int fd, uint64_t *lastino, uint32_t icount,
+		struct xfs_bstat *ubuffer, uint32_t *ocount);
+
 #endif	/* __XFS_LINUX_H__ */
diff --git a/io/open.c b/io/open.c
index b6aacb83..6ceff18d 100644
--- a/io/open.c
+++ b/io/open.c
@@ -712,19 +712,18 @@ get_last_inode(void)
 
 static int
 inode_f(
-	  int			argc,
-	  char			**argv)
+	int			argc,
+	char			**argv)
 {
-	__s32			count = 0;
-	__u64			result_ino = 0;
-	__u64			userino = NULLFSINO;
+	struct xfs_bstat	bstat;
+	uint32_t		count = 0;
+	uint64_t		result_ino = 0;
+	uint64_t		userino = NULLFSINO;
 	char			*p;
 	int			c;
 	int			verbose = 0;
 	int			ret_next = 0;
-	int			cmd = 0;
-	struct xfs_fsop_bulkreq	bulkreq;
-	struct xfs_bstat	bstat;
+	int			ret;
 
 	while ((c = getopt(argc, argv, "nv")) != EOF) {
 		switch (c) {
@@ -766,35 +765,32 @@ inode_f(
 			exitcode = 1;
 			return 0;
 		}
+	} else if (ret_next) {
+		/* get next inode */
+		ret = xfs_bulkstat(file->fd, &userino, 1, &bstat, &count);
+		if (ret) {
+			perror("xfsctl");
+			exitcode = 1;
+			return 0;
+		}
+
+		/* The next inode in use, or 0 if none */
+		if (count)
+			result_ino = bstat.bs_ino;
+		else
+			result_ino = 0;
 	} else {
-		if (ret_next)	/* get next inode */
-			cmd = XFS_IOC_FSBULKSTAT;
-		else		/* get this inode */
-			cmd = XFS_IOC_FSBULKSTAT_SINGLE;
-
-		bulkreq.lastip = &userino;
-		bulkreq.icount = 1;
-		bulkreq.ubuffer = &bstat;
-		bulkreq.ocount = &count;
-
-		if (xfsctl(file->name, file->fd, cmd, &bulkreq)) {
-			if (!ret_next && errno == EINVAL) {
-				/* Not in use */
-				result_ino = 0;
-			} else {
-				perror("xfsctl");
-				exitcode = 1;
-				return 0;
-			}
-		} else if (ret_next) {
-			/* The next inode in use, or 0 if none */
-			if (*bulkreq.ocount)
-				result_ino = bstat.bs_ino;
-			else
-				result_ino = 0;
+		/* get this inode */
+		ret = xfs_bulkstat_single(file->fd, userino, &bstat);
+		if (ret && errno == EINVAL) {
+			/* Not in use */
+			result_ino = 0;
+		} else if (ret) {
+			perror("bulkstat_single");
+			exitcode = 1;
+			return 0;
 		} else {
-			/* The inode we asked about */
-			result_ino = userino;
+			result_ino = bstat.bs_ino;
 		}
 	}
 
diff --git a/io/swapext.c b/io/swapext.c
index d360c221..8b1b1b18 100644
--- a/io/swapext.c
+++ b/io/swapext.c
@@ -20,21 +20,6 @@ swapext_help(void)
 "\n"));
 }
 
-static int
-xfs_bulkstat_single(
-	int			fd,
-	xfs_ino_t		*lastip,
-	struct xfs_bstat	*ubuffer)
-{
-	struct xfs_fsop_bulkreq	bulkreq;
-
-	bulkreq.lastip = (__u64 *)lastip;
-	bulkreq.icount = 1;
-	bulkreq.ubuffer = ubuffer;
-	bulkreq.ocount = NULL;
-	return ioctl(fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
-}
-
 static int
 swapext_f(
 	int			argc,
@@ -60,7 +45,7 @@ swapext_f(
 		goto out;
 	}
 
-	error = xfs_bulkstat_single(file->fd, &stat.st_ino, &sx.sx_stat);
+	error = xfs_bulkstat_single(file->fd, stat.st_ino, &sx.sx_stat);
 	if (error) {
 		perror("bulkstat");
 		goto out;
diff --git a/libhandle/ioctl.c b/libhandle/ioctl.c
index 5c954bd0..a4676fea 100644
--- a/libhandle/ioctl.c
+++ b/libhandle/ioctl.c
@@ -24,3 +24,41 @@ xfs_fsgeometry(
 
 	return ioctl(fd, XFS_IOC_FSGEOMETRY_V1, fsgeo);
 }
+
+/* Bulkstat a single inode. */
+int
+xfs_bulkstat_single(
+	int			fd,
+	uint64_t		ino,
+	struct xfs_bstat	*ubuffer)
+{
+	__u64			i = ino;
+	struct xfs_fsop_bulkreq	bulkreq = {
+		.lastip		= &i,
+		.icount		= 1,
+		.ubuffer	= ubuffer,
+		.ocount		= NULL,
+	};
+
+	return ioctl(fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
+}
+
+/* Bulkstat a bunch of inodes. */
+int
+xfs_bulkstat(
+	int			fd,
+	uint64_t		*lastino,
+	uint32_t		icount,
+	struct xfs_bstat	*ubuffer,
+	uint32_t		*ocount)
+{
+	struct xfs_fsop_bulkreq	bulkreq = {
+		.lastip		= (__u64 *)lastino,
+		.icount		= icount,
+		.ubuffer	= ubuffer,
+		.ocount		= (__s32 *)ocount,
+	};
+
+	return ioctl(fd, XFS_IOC_FSBULKSTAT, &bulkreq);
+}
+
diff --git a/quota/quot.c b/quota/quot.c
index d60cf4a8..789e4b40 100644
--- a/quota/quot.c
+++ b/quota/quot.c
@@ -124,12 +124,11 @@ quot_bulkstat_add(
 static void
 quot_bulkstat_mount(
 	char			*fsdir,
-	uint			flags)
+	unsigned int		flags)
 {
-	xfs_fsop_bulkreq_t	bulkreq;
-	xfs_bstat_t		*buf;
-	__u64			last = 0;
-	__s32			count;
+	struct xfs_bstat	*buf;
+	uint64_t		last = 0;
+	uint32_t		count;
 	int			i, sts, fsfd;
 	du_t			**dp;
 
@@ -158,12 +157,7 @@ quot_bulkstat_mount(
 		return;
 	}
 
-	bulkreq.lastip = &last;
-	bulkreq.icount = NBSTAT;
-	bulkreq.ubuffer = buf;
-	bulkreq.ocount = &count;
-
-	while ((sts = xfsctl(fsdir, fsfd, XFS_IOC_FSBULKSTAT, &bulkreq)) == 0) {
+	while ((sts = xfs_bulkstat(fsfd, &last, NBSTAT, buf, &count)) == 0) {
 		if (count == 0)
 			break;
 		for (i = 0; i < count; i++)
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 442a5978..702b7d50 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -50,17 +50,10 @@ xfs_iterate_inodes_range_check(
 	struct xfs_inogrp	*inogrp,
 	struct xfs_bstat	*bstat)
 {
-	struct xfs_fsop_bulkreq	onereq = {NULL};
 	struct xfs_bstat	*bs;
-	__u64			oneino;
-	__s32			onelen = 0;
 	int			i;
 	int			error;
 
-	onereq.lastip  = &oneino;
-	onereq.icount  = 1;
-	onereq.ocount  = &onelen;
-
 	for (i = 0, bs = bstat; i < XFS_INODES_PER_CHUNK; i++) {
 		if (!(inogrp->xi_allocmask & (1ULL << i)))
 			continue;
@@ -70,10 +63,8 @@ xfs_iterate_inodes_range_check(
 		}
 
 		/* Load the one inode. */
-		oneino = inogrp->xi_startino + i;
-		onereq.ubuffer = bs;
-		error = ioctl(ctx->mnt_fd, XFS_IOC_FSBULKSTAT_SINGLE,
-				&onereq);
+		error = xfs_bulkstat_single(ctx->mnt_fd,
+				inogrp->xi_startino + i, bs);
 		if (error || bs->bs_ino != inogrp->xi_startino + i) {
 			memset(bs, 0, sizeof(struct xfs_bstat));
 			bs->bs_ino = inogrp->xi_startino + i;
@@ -99,7 +90,6 @@ xfs_iterate_inodes_range(
 	void			*arg)
 {
 	struct xfs_fsop_bulkreq	igrpreq = {NULL};
-	struct xfs_fsop_bulkreq	bulkreq = {NULL};
 	struct xfs_handle	handle;
 	struct xfs_inogrp	inogrp;
 	struct xfs_bstat	bstat[XFS_INODES_PER_CHUNK];
@@ -107,8 +97,8 @@ xfs_iterate_inodes_range(
 	char			buf[DESCR_BUFSZ];
 	struct xfs_bstat	*bs;
 	__u64			igrp_ino;
-	__u64			ino;
-	__s32			bulklen = 0;
+	uint64_t		ino;
+	uint32_t		bulklen = 0;
 	__s32			igrplen = 0;
 	bool			moveon = true;
 	int			i;
@@ -117,10 +107,6 @@ xfs_iterate_inodes_range(
 
 
 	memset(bstat, 0, XFS_INODES_PER_CHUNK * sizeof(struct xfs_bstat));
-	bulkreq.lastip  = &ino;
-	bulkreq.icount  = XFS_INODES_PER_CHUNK;
-	bulkreq.ubuffer = &bstat;
-	bulkreq.ocount  = &bulklen;
 
 	igrpreq.lastip  = &igrp_ino;
 	igrpreq.icount  = 1;
@@ -138,14 +124,15 @@ xfs_iterate_inodes_range(
 	while (!error && igrplen) {
 		/* Load the inodes. */
 		ino = inogrp.xi_startino - 1;
-		bulkreq.icount = inogrp.xi_alloccount;
+
 		/*
 		 * We can have totally empty inode chunks on filesystems where
 		 * there are more than 64 inodes per block.  Skip these.
 		 */
 		if (inogrp.xi_alloccount == 0)
 			goto igrp_retry;
-		error = ioctl(ctx->mnt_fd, XFS_IOC_FSBULKSTAT, &bulkreq);
+		error = xfs_bulkstat(ctx->mnt_fd, &ino, inogrp.xi_alloccount,
+				bstat, &bulklen);
 		if (error)
 			str_info(ctx, descr, "%s", strerror_r(errno,
 						buf, DESCR_BUFSZ));

[PATCH 05/12] libxfs: refactor open-coded INUMBERS calls

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Refactor all the INUMBERS ioctl callsites into helper functions.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/linux.h    |    4 ++++
 io/imap.c          |   30 +++++++++++++-----------------
 io/open.c          |   19 +++++++------------
 libhandle/ioctl.c  |   18 ++++++++++++++++++
 scrub/fscounters.c |   17 ++++++-----------
 scrub/inodes.c     |   21 +++++++--------------
 6 files changed, 55 insertions(+), 54 deletions(-)


diff --git a/include/linux.h b/include/linux.h
index 98750e18..39190e11 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -333,4 +333,8 @@ int xfs_bulkstat_single(int fd, uint64_t ino, struct xfs_bstat *ubuffer);
 int xfs_bulkstat(int fd, uint64_t *lastino, uint32_t icount,
 		struct xfs_bstat *ubuffer, uint32_t *ocount);
 
+struct xfs_inogrp;
+int xfs_inumbers(int fd, uint64_t *lastino, uint32_t icount,
+		struct xfs_inogrp *ubuffer, uint32_t *ocount);
+
 #endif	/* __XFS_LINUX_H__ */
diff --git a/io/imap.c b/io/imap.c
index fbc8e9e1..49917545 100644
--- a/io/imap.c
+++ b/io/imap.c
@@ -14,12 +14,12 @@ static cmdinfo_t imap_cmd;
 static int
 imap_f(int argc, char **argv)
 {
-	int		count;
-	int		nent;
-	int		i;
-	__u64		last = 0;
-	xfs_inogrp_t	*t;
-	xfs_fsop_bulkreq_t bulkreq;
+	struct xfs_inogrp	*t;
+	uint64_t		last = 0;
+	uint32_t		count;
+	uint32_t		nent;
+	int			i;
+	int			error;
 
 	if (argc != 2)
 		nent = 1;
@@ -30,14 +30,8 @@ imap_f(int argc, char **argv)
 	if (!t)
 		return 0;
 
-	bulkreq.lastip  = &last;
-	bulkreq.icount  = nent;
-	bulkreq.ubuffer = (void *)t;
-	bulkreq.ocount  = &count;
-
-	while (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS, &bulkreq) == 0) {
-		if (count == 0)
-			goto out_free;
+	while ((error = xfs_inumbers(file->fd, &last, nent, t, &count)) == 0 &&
+	       count > 0) {
 		for (i = 0; i < count; i++) {
 			printf(_("ino %10llu count %2d mask %016llx\n"),
 				(unsigned long long)t[i].xi_startino,
@@ -45,9 +39,11 @@ imap_f(int argc, char **argv)
 				(unsigned long long)t[i].xi_allocmask);
 		}
 	}
-	perror("xfsctl(XFS_IOC_FSINUMBERS)");
-	exitcode = 1;
-out_free:
+
+	if (error) {
+		perror("xfsctl(XFS_IOC_FSINUMBERS)");
+		exitcode = 1;
+	}
 	free(t);
 	return 0;
 }
diff --git a/io/open.c b/io/open.c
index 6ceff18d..11805cd7 100644
--- a/io/open.c
+++ b/io/open.c
@@ -668,24 +668,19 @@ inode_help(void)
 "\n"));
 }
 
+#define IGROUP_NR	(1024)
 static __u64
 get_last_inode(void)
 {
-	__u64			lastip = 0;
-	__u64			lastgrp = 0;
-	__s32			ocount = 0;
+	uint64_t		lastip = 0;
+	uint32_t		lastgrp = 0;
+	uint32_t		ocount = 0;
 	__u64			last_ino;
-	struct xfs_inogrp	igroup[1024];
-	struct xfs_fsop_bulkreq	bulkreq;
-
-	bulkreq.lastip = &lastip;
-	bulkreq.ubuffer = &igroup;
-	bulkreq.icount = sizeof(igroup) / sizeof(struct xfs_inogrp);
-	bulkreq.ocount = &ocount;
+	struct xfs_inogrp	igroup[IGROUP_NR];
 
 	for (;;) {
-		if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS,
-				&bulkreq)) {
+		if (xfs_inumbers(file->fd, &lastip, IGROUP_NR, igroup,
+					&ocount)) {
 			perror("XFS_IOC_FSINUMBERS");
 			return 0;
 		}
diff --git a/libhandle/ioctl.c b/libhandle/ioctl.c
index a4676fea..599fdf3e 100644
--- a/libhandle/ioctl.c
+++ b/libhandle/ioctl.c
@@ -62,3 +62,21 @@ xfs_bulkstat(
 	return ioctl(fd, XFS_IOC_FSBULKSTAT, &bulkreq);
 }
 
+/* Query inode allocation bitmask information. */
+int
+xfs_inumbers(
+	int			fd,
+	uint64_t		*lastino,
+	uint32_t		icount,
+	struct xfs_inogrp	*ubuffer,
+	uint32_t		*ocount)
+{
+	struct xfs_fsop_bulkreq	bulkreq = {
+		.lastip		= (__u64 *)lastino,
+		.icount		= icount,
+		.ubuffer	= ubuffer,
+		.ocount		= (__s32 *)ocount,
+	};
+
+	return ioctl(fd, XFS_IOC_FSINUMBERS, &bulkreq);
+}
diff --git a/scrub/fscounters.c b/scrub/fscounters.c
index 9e93e2a6..13f46e17 100644
--- a/scrub/fscounters.c
+++ b/scrub/fscounters.c
@@ -41,26 +41,21 @@ xfs_count_inodes_range(
 	uint64_t		last_ino,
 	uint64_t		*count)
 {
-	struct xfs_fsop_bulkreq	igrpreq = {NULL};
 	struct xfs_inogrp	inogrp;
-	__u64			igrp_ino;
+	uint64_t		igrp_ino;
 	uint64_t		nr = 0;
-	__s32			igrplen = 0;
+	uint32_t		igrplen = 0;
 	int			error;
 
 	ASSERT(!(first_ino & (XFS_INODES_PER_CHUNK - 1)));
 	ASSERT((last_ino & (XFS_INODES_PER_CHUNK - 1)));
 
-	igrpreq.lastip  = &igrp_ino;
-	igrpreq.icount  = 1;
-	igrpreq.ubuffer = &inogrp;
-	igrpreq.ocount  = &igrplen;
-
 	igrp_ino = first_ino;
-	error = ioctl(ctx->mnt_fd, XFS_IOC_FSINUMBERS, &igrpreq);
-	while (!error && igrplen && inogrp.xi_startino < last_ino) {
+	while ((error = xfs_inumbers(ctx->mnt_fd, &igrp_ino, 1, &inogrp,
+					&igrplen)) == 0 &&
+	       igrplen > 0 &&
+	       inogrp.xi_startino < last_ino) {
 		nr += inogrp.xi_alloccount;
-		error = ioctl(ctx->mnt_fd, XFS_IOC_FSINUMBERS, &igrpreq);
 	}
 
 	if (error) {
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 702b7d50..b27edef7 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -89,17 +89,16 @@ xfs_iterate_inodes_range(
 	xfs_inode_iter_fn	fn,
 	void			*arg)
 {
-	struct xfs_fsop_bulkreq	igrpreq = {NULL};
 	struct xfs_handle	handle;
 	struct xfs_inogrp	inogrp;
 	struct xfs_bstat	bstat[XFS_INODES_PER_CHUNK];
 	char			idescr[DESCR_BUFSZ];
 	char			buf[DESCR_BUFSZ];
 	struct xfs_bstat	*bs;
-	__u64			igrp_ino;
+	uint64_t		igrp_ino;
 	uint64_t		ino;
 	uint32_t		bulklen = 0;
-	__s32			igrplen = 0;
+	uint32_t		igrplen = 0;
 	bool			moveon = true;
 	int			i;
 	int			error;
@@ -108,11 +107,6 @@ xfs_iterate_inodes_range(
 
 	memset(bstat, 0, XFS_INODES_PER_CHUNK * sizeof(struct xfs_bstat));
 
-	igrpreq.lastip  = &igrp_ino;
-	igrpreq.icount  = 1;
-	igrpreq.ubuffer = &inogrp;
-	igrpreq.ocount  = &igrplen;
-
 	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
 	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
 			sizeof(handle.ha_fid.fid_len);
@@ -120,8 +114,9 @@ xfs_iterate_inodes_range(
 
 	/* Find the inode chunk & alloc mask */
 	igrp_ino = first_ino;
-	error = ioctl(ctx->mnt_fd, XFS_IOC_FSINUMBERS, &igrpreq);
-	while (!error && igrplen) {
+	while ((error = xfs_inumbers(ctx->mnt_fd, &igrp_ino, 1, &inogrp,
+					&igrplen)) == 0 &&
+	       igrplen > 0) {
 		/* Load the inodes. */
 		ino = inogrp.xi_startino - 1;
 
@@ -130,7 +125,7 @@ xfs_iterate_inodes_range(
 		 * there are more than 64 inodes per block.  Skip these.
 		 */
 		if (inogrp.xi_alloccount == 0)
-			goto igrp_retry;
+			continue;
 		error = xfs_bulkstat(ctx->mnt_fd, &ino, inogrp.xi_alloccount,
 				bstat, &bulklen);
 		if (error)
@@ -154,7 +149,7 @@ xfs_iterate_inodes_range(
 				stale_count++;
 				if (stale_count < 30) {
 					igrp_ino = inogrp.xi_startino;
-					goto igrp_retry;
+					continue;
 				}
 				snprintf(idescr, DESCR_BUFSZ, "inode %"PRIu64,
 						(uint64_t)bs->bs_ino);
@@ -176,8 +171,6 @@ _("Changed too many times during scan; giving up."));
 		}
 
 		stale_count = 0;
-igrp_retry:
-		error = ioctl(ctx->mnt_fd, XFS_IOC_FSINUMBERS, &igrpreq);
 	}
 
 err:

[PATCH 06/12] misc: remove all use of xfs_fsop_geom_t

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Remove all the uses of the old xfs_fsop_geom_t typedef.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 growfs/xfs_growfs.c |    4 ++--
 io/init.c           |    2 +-
 io/io.h             |    6 +++---
 io/open.c           |    6 +++---
 man/man3/xfsctl.3   |    2 +-
 spaceman/file.c     |    4 ++--
 spaceman/init.c     |    2 +-
 spaceman/space.h    |    6 +++---
 8 files changed, 16 insertions(+), 16 deletions(-)


diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index 392e4a00..ffd82f95 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -44,7 +44,7 @@ main(int argc, char **argv)
 	int			error;	/* we have hit an error */
 	long			esize;	/* new rt extent size */
 	int			ffd;	/* mount point file descriptor */
-	xfs_fsop_geom_t		geo;	/* current fs geometry */
+	struct xfs_fsop_geom	geo;	/* current fs geometry */
 	int			iflag;	/* -i flag */
 	int			isint;	/* log is currently internal */
 	int			lflag;	/* -l flag */
@@ -52,7 +52,7 @@ main(int argc, char **argv)
 	int			maxpct;	/* -m flag value */
 	int			mflag;	/* -m flag */
 	int			nflag;	/* -n flag */
-	xfs_fsop_geom_t		ngeo;	/* new fs geometry */
+	struct xfs_fsop_geom	ngeo;	/* new fs geometry */
 	int			rflag;	/* -r flag */
 	long long		rsize;	/* new rt size in fs blocks */
 	int			xflag;	/* -x flag */
diff --git a/io/init.c b/io/init.c
index 83f08f2d..7025aea5 100644
--- a/io/init.c
+++ b/io/init.c
@@ -133,7 +133,7 @@ init(
 	int		c, flags = 0;
 	char		*sp;
 	mode_t		mode = 0600;
-	xfs_fsop_geom_t	geometry = { 0 };
+	struct xfs_fsop_geom geometry = { 0 };
 	struct fs_path	fsp;
 
 	progname = basename(argv[0]);
diff --git a/io/io.h b/io/io.h
index 6469179e..0848ab98 100644
--- a/io/io.h
+++ b/io/io.h
@@ -38,7 +38,7 @@ typedef struct fileio {
 	int		fd;		/* open file descriptor */
 	int		flags;		/* flags describing file state */
 	char		*name;		/* file name at time of open */
-	xfs_fsop_geom_t	geom;		/* XFS filesystem geometry */
+	struct xfs_fsop_geom geom;	/* XFS filesystem geometry */
 	struct fs_path	fs_path;	/* XFS path information */
 } fileio_t;
 
@@ -70,9 +70,9 @@ extern void *check_mapping_range(mmap_region_t *, off64_t, size_t, int);
  */
 
 extern off64_t		filesize(void);
-extern int		openfile(char *, xfs_fsop_geom_t *, int, mode_t,
+extern int		openfile(char *, struct xfs_fsop_geom *, int, mode_t,
 				 struct fs_path *);
-extern int		addfile(char *, int , xfs_fsop_geom_t *, int,
+extern int		addfile(char *, int , struct xfs_fsop_geom *, int,
 				struct fs_path *);
 extern void		printxattr(uint, int, int, const char *, int, int);
 
diff --git a/io/open.c b/io/open.c
index 11805cd7..ce7a5362 100644
--- a/io/open.c
+++ b/io/open.c
@@ -51,7 +51,7 @@ static long extsize;
 int
 openfile(
 	char		*path,
-	xfs_fsop_geom_t	*geom,
+	struct xfs_fsop_geom *geom,
 	int		flags,
 	mode_t		mode,
 	struct fs_path	*fs_path)
@@ -156,7 +156,7 @@ int
 addfile(
 	char		*name,
 	int		fd,
-	xfs_fsop_geom_t	*geometry,
+	struct xfs_fsop_geom *geometry,
 	int		flags,
 	struct fs_path	*fs_path)
 {
@@ -229,7 +229,7 @@ open_f(
 	int		c, fd, flags = 0;
 	char		*sp;
 	mode_t		mode = 0600;
-	xfs_fsop_geom_t	geometry = { 0 };
+	struct xfs_fsop_geom geometry = { 0 };
 	struct fs_path	fsp;
 
 	if (argc == 1) {
diff --git a/man/man3/xfsctl.3 b/man/man3/xfsctl.3
index 6e5027c4..462ccbd8 100644
--- a/man/man3/xfsctl.3
+++ b/man/man3/xfsctl.3
@@ -640,7 +640,7 @@ operations on XFS filesystems.
 For
 .B XFS_IOC_FSGEOMETRY
 (get filesystem mkfs time information), the output structure is of type
-.BR xfs_fsop_geom_t .
+.BR struct xfs_fsop_geom .
 For
 .B XFS_FS_COUNTS
 (get filesystem dynamic global information), the output structure is of type
diff --git a/spaceman/file.c b/spaceman/file.c
index d2acf5db..a9b8461f 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -44,7 +44,7 @@ print_f(
 int
 openfile(
 	char		*path,
-	xfs_fsop_geom_t	*geom,
+	struct xfs_fsop_geom *geom,
 	struct fs_path	*fs_path)
 {
 	struct fs_path	*fsp;
@@ -84,7 +84,7 @@ int
 addfile(
 	char		*name,
 	int		fd,
-	xfs_fsop_geom_t	*geometry,
+	struct xfs_fsop_geom *geometry,
 	struct fs_path	*fs_path)
 {
 	char		*filename;
diff --git a/spaceman/init.c b/spaceman/init.c
index 181a3446..c845f920 100644
--- a/spaceman/init.c
+++ b/spaceman/init.c
@@ -60,7 +60,7 @@ init(
 	char		**argv)
 {
 	int		c;
-	xfs_fsop_geom_t	geometry = { 0 };
+	struct xfs_fsop_geom geometry = { 0 };
 	struct fs_path	fsp;
 
 	progname = basename(argv[0]);
diff --git a/spaceman/space.h b/spaceman/space.h
index bf9cc2bf..b246f602 100644
--- a/spaceman/space.h
+++ b/spaceman/space.h
@@ -7,7 +7,7 @@
 #define XFS_SPACEMAN_SPACE_H_
 
 typedef struct fileio {
-	xfs_fsop_geom_t	geom;		/* XFS filesystem geometry */
+	struct xfs_fsop_geom geom;		/* XFS filesystem geometry */
 	struct fs_path	fs_path;	/* XFS path information */
 	char		*name;		/* file name at time of open */
 	int		fd;		/* open file descriptor */
@@ -17,8 +17,8 @@ extern fileio_t		*filetable;	/* open file table */
 extern int		filecount;	/* number of open files */
 extern fileio_t		*file;		/* active file in file table */
 
-extern int	openfile(char *, xfs_fsop_geom_t *, struct fs_path *);
-extern int	addfile(char *, int , xfs_fsop_geom_t *, struct fs_path *);
+extern int	openfile(char *, struct xfs_fsop_geom *, struct fs_path *);
+extern int	addfile(char *, int , struct xfs_fsop_geom *, struct fs_path *);
 
 extern void	print_init(void);
 extern void	help_init(void);

[PATCH 07/12] libfrog: fix bitmap return values

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Fix the return types of non-predicate bitmap functions to return the
usual negative error codes instead of the "moveon" boolean.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/bitmap.h |    8 +++--
 libfrog/bitmap.c |   86 ++++++++++++++++++++++++++----------------------------
 repair/rmap.c    |   18 ++++++++---
 scrub/phase6.c   |   18 ++++-------
 4 files changed, 65 insertions(+), 65 deletions(-)


diff --git a/include/bitmap.h b/include/bitmap.h
index e29a4335..99a2fb23 100644
--- a/include/bitmap.h
+++ b/include/bitmap.h
@@ -11,11 +11,11 @@ struct bitmap {
 	struct avl64tree_desc	*bt_tree;
 };
 
-bool bitmap_init(struct bitmap **bmap);
+int bitmap_init(struct bitmap **bmap);
 void bitmap_free(struct bitmap **bmap);
-bool bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
-bool bitmap_iterate(struct bitmap *bmap,
-		bool (*fn)(uint64_t, uint64_t, void *), void *arg);
+int bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
+int bitmap_iterate(struct bitmap *bmap, int (*fn)(uint64_t, uint64_t, void *),
+		void *arg);
 bool bitmap_test(struct bitmap *bmap, uint64_t start,
 		uint64_t len);
 bool bitmap_empty(struct bitmap *bmap);
diff --git a/libfrog/bitmap.c b/libfrog/bitmap.c
index 450ebe0a..4dafc4c9 100644
--- a/libfrog/bitmap.c
+++ b/libfrog/bitmap.c
@@ -66,7 +66,7 @@ static struct avl64ops bitmap_ops = {
 };
 
 /* Initialize a bitmap. */
-bool
+int
 bitmap_init(
 	struct bitmap		**bmapp)
 {
@@ -74,18 +74,18 @@ bitmap_init(
 
 	bmap = calloc(1, sizeof(struct bitmap));
 	if (!bmap)
-		return false;
+		return -ENOMEM;
 	bmap->bt_tree = malloc(sizeof(struct avl64tree_desc));
 	if (!bmap->bt_tree) {
 		free(bmap);
-		return false;
+		return -ENOMEM;
 	}
 
 	pthread_mutex_init(&bmap->bt_lock, NULL);
 	avl64_init_tree(bmap->bt_tree, &bitmap_ops);
 	*bmapp = bmap;
 
-	return true;
+	return 0;
 }
 
 /* Free a bitmap. */
@@ -127,8 +127,31 @@ bitmap_node_init(
 	return ext;
 }
 
+/* Create a new bitmap node and insert it. */
+static inline int
+__bitmap_insert(
+	struct bitmap		*bmap,
+	uint64_t		start,
+	uint64_t		length)
+{
+	struct bitmap_node	*ext;
+	struct avl64node	*node;
+
+	ext = bitmap_node_init(start, length);
+	if (!ext)
+		return -ENOMEM;
+
+	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
+	if (node == NULL) {
+		free(ext);
+		return -EEXIST;
+	}
+
+	return 0;
+}
+
 /* Set a region of bits (locked). */
-static bool
+static int
 __bitmap_set(
 	struct bitmap		*bmap,
 	uint64_t		start,
@@ -142,28 +165,14 @@ __bitmap_set(
 	struct bitmap_node	*ext;
 	uint64_t		new_start;
 	uint64_t		new_length;
-	struct avl64node	*node;
-	bool			res = true;
 
 	/* Find any existing nodes adjacent or within that range. */
 	avl64_findranges(bmap->bt_tree, start - 1, start + length + 1,
 			&firstn, &lastn);
 
 	/* Nothing, just insert a new extent. */
-	if (firstn == NULL && lastn == NULL) {
-		ext = bitmap_node_init(start, length);
-		if (!ext)
-			return false;
-
-		node = avl64_insert(bmap->bt_tree, &ext->btn_node);
-		if (node == NULL) {
-			free(ext);
-			errno = EEXIST;
-			return false;
-		}
-
-		return true;
-	}
+	if (firstn == NULL && lastn == NULL)
+		return __bitmap_insert(bmap, start, length);
 
 	assert(firstn != NULL && lastn != NULL);
 	new_start = start;
@@ -175,7 +184,7 @@ __bitmap_set(
 		/* Bail if the new extent is contained within an old one. */
 		if (ext->btn_start <= start &&
 		    ext->btn_start + ext->btn_length >= start + length)
-			return res;
+			return 0;
 
 		/* Check for overlapping and adjacent extents. */
 		if (ext->btn_start + ext->btn_length >= start ||
@@ -195,28 +204,17 @@ __bitmap_set(
 		}
 	}
 
-	ext = bitmap_node_init(new_start, new_length);
-	if (!ext)
-		return false;
-
-	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
-	if (node == NULL) {
-		free(ext);
-		errno = EEXIST;
-		return false;
-	}
-
-	return res;
+	return __bitmap_insert(bmap, new_start, new_length);
 }
 
 /* Set a region of bits. */
-bool
+int
 bitmap_set(
 	struct bitmap		*bmap,
 	uint64_t		start,
 	uint64_t		length)
 {
-	bool			res;
+	int			res;
 
 	pthread_mutex_lock(&bmap->bt_lock);
 	res = __bitmap_set(bmap, start, length);
@@ -308,26 +306,26 @@ bitmap_clear(
 
 #ifdef DEBUG
 /* Iterate the set regions of this bitmap. */
-bool
+int
 bitmap_iterate(
 	struct bitmap		*bmap,
-	bool			(*fn)(uint64_t, uint64_t, void *),
+	int			(*fn)(uint64_t, uint64_t, void *),
 	void			*arg)
 {
 	struct avl64node	*node;
 	struct bitmap_node	*ext;
-	bool			moveon = true;
+	int			error = 0;
 
 	pthread_mutex_lock(&bmap->bt_lock);
 	avl_for_each(bmap->bt_tree, node) {
 		ext = container_of(node, struct bitmap_node, btn_node);
-		moveon = fn(ext->btn_start, ext->btn_length, arg);
-		if (!moveon)
+		error = fn(ext->btn_start, ext->btn_length, arg);
+		if (error)
 			break;
 	}
 	pthread_mutex_unlock(&bmap->bt_lock);
 
-	return moveon;
+	return error;
 }
 #endif
 
@@ -372,14 +370,14 @@ bitmap_empty(
 }
 
 #ifdef DEBUG
-static bool
+static int
 bitmap_dump_fn(
 	uint64_t		startblock,
 	uint64_t		blockcount,
 	void			*arg)
 {
 	printf("%"PRIu64":%"PRIu64"\n", startblock, blockcount);
-	return true;
+	return 0;
 }
 
 /* Dump bitmap. */
diff --git a/repair/rmap.c b/repair/rmap.c
index 19cceca3..47828a06 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -490,16 +490,22 @@ rmap_store_ag_btree_rec(
 	error = init_slab_cursor(ag_rmap->ar_raw_rmaps, rmap_compare, &rm_cur);
 	if (error)
 		goto err;
-	if (!bitmap_init(&own_ag_bitmap)) {
-		error = -ENOMEM;
+	error = -bitmap_init(&own_ag_bitmap);
+	if (error)
 		goto err_slab;
-	}
 	while ((rm_rec = pop_slab_cursor(rm_cur)) != NULL) {
 		if (rm_rec->rm_owner != XFS_RMAP_OWN_AG)
 			continue;
-		if (!bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
-					rm_rec->rm_blockcount)) {
-			error = EFSCORRUPTED;
+		error = -bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
+					rm_rec->rm_blockcount);
+		if (error) {
+			/*
+			 * If this range is already set, then the incore rmap
+			 * records for the AG free space btrees overlap and
+			 * we're toast because that is not allowed.
+			 */
+			if (error == EEXIST)
+				error = EFSCORRUPTED;
 			goto err_slab;
 		}
 	}
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 4b25f3bb..66e6451c 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -341,7 +341,6 @@ xfs_check_rmap_ioerr(
 	struct media_verify_state	*vs = arg;
 	struct bitmap			*tree;
 	dev_t				dev;
-	bool				moveon;
 
 	dev = xfs_disk_to_dev(ctx, disk);
 
@@ -356,8 +355,8 @@ xfs_check_rmap_ioerr(
 	else
 		tree = NULL;
 	if (tree) {
-		moveon = bitmap_set(tree, start, length);
-		if (!moveon)
+		errno = -bitmap_set(tree, start, length);
+		if (errno)
 			str_errno(ctx, ctx->mntpoint);
 	}
 
@@ -454,16 +453,16 @@ xfs_scan_blocks(
 	struct scrub_ctx		*ctx)
 {
 	struct media_verify_state	vs = { NULL };
-	bool				moveon;
+	bool				moveon = false;
 
-	moveon = bitmap_init(&vs.d_bad);
-	if (!moveon) {
+	errno = -bitmap_init(&vs.d_bad);
+	if (errno) {
 		str_errno(ctx, ctx->mntpoint);
 		goto out;
 	}
 
-	moveon = bitmap_init(&vs.r_bad);
-	if (!moveon) {
+	errno = -bitmap_init(&vs.r_bad);
+	if (errno) {
 		str_errno(ctx, ctx->mntpoint);
 		goto out_dbad;
 	}
@@ -472,7 +471,6 @@ xfs_scan_blocks(
 			ctx->geo.blocksize, xfs_check_rmap_ioerr,
 			scrub_nproc(ctx));
 	if (!vs.rvp_data) {
-		moveon = false;
 		str_info(ctx, ctx->mntpoint,
 _("Could not create data device media verifier."));
 		goto out_rbad;
@@ -482,7 +480,6 @@ _("Could not create data device media verifier."));
 				ctx->geo.blocksize, xfs_check_rmap_ioerr,
 				scrub_nproc(ctx));
 		if (!vs.rvp_log) {
-			moveon = false;
 			str_info(ctx, ctx->mntpoint,
 	_("Could not create log device media verifier."));
 			goto out_datapool;
@@ -493,7 +490,6 @@ _("Could not create data device media verifier."));
 				ctx->geo.blocksize, xfs_check_rmap_ioerr,
 				scrub_nproc(ctx));
 		if (!vs.rvp_realtime) {
-			moveon = false;
 			str_info(ctx, ctx->mntpoint,
 	_("Could not create realtime device media verifier."));
 			goto out_logpool;

[PATCH 08/12] xfs_repair: refactor namecheck functions

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Now that we have name check functions in libxfs, use them instead of our
custom versions.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/libxfs_api_defs.h |    2 ++
 repair/attr_repair.c     |   32 +++++++++++++-------------------
 repair/da_util.c         |   27 ---------------------------
 repair/da_util.h         |    6 ------
 repair/dir2.c            |   12 ++----------
 5 files changed, 17 insertions(+), 62 deletions(-)


diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 34bb552d..71a7ef53 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -143,6 +143,8 @@
 #define xfs_default_ifork_ops		libxfs_default_ifork_ops
 #define xfs_fs_geometry			libxfs_fs_geometry
 #define xfs_init_local_fork		libxfs_init_local_fork
+#define xfs_dir2_namecheck		libxfs_dir2_namecheck
+#define xfs_attr_namecheck		libxfs_attr_namecheck
 
 #define LIBXFS_ATTR_ROOT		ATTR_ROOT
 #define LIBXFS_ATTR_SECURE		ATTR_SECURE
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 5ad81c09..9a44f610 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -122,14 +122,6 @@ set_da_freemap(xfs_mount_t *mp, da_freemap_t *map, int start, int stop)
  * fork being emptied and put in shortform format.
  */
 
-static int
-attr_namecheck(
-	uint8_t	*name,
-	int	length)
-{
-	return namecheck((char *)name, length, false);
-}
-
 /*
  * This routine just checks what security needs are for attribute values
  * only called when root flag is set, otherwise these names could exist in
@@ -301,8 +293,8 @@ process_shortform_attr(
 		}
 
 		/* namecheck checks for null chars in attr names. */
-		if (attr_namecheck(currententry->nameval,
-						currententry->namelen)) {
+		if (!libxfs_attr_namecheck(currententry->nameval,
+					   currententry->namelen)) {
 			do_warn(
 	_("entry contains illegal character in shortform attribute name\n"));
 			junkit = 1;
@@ -464,8 +456,9 @@ process_leaf_attr_local(
 	xfs_attr_leaf_name_local_t *local;
 
 	local = xfs_attr3_leaf_name_local(leaf, i);
-	if (local->namelen == 0 || attr_namecheck(local->nameval,
-							local->namelen)) {
+	if (local->namelen == 0 ||
+	    !libxfs_attr_namecheck(local->nameval,
+				   local->namelen)) {
 		do_warn(
 	_("attribute entry %d in attr block %u, inode %" PRIu64 " has bad name (namelen = %d)\n"),
 			i, da_bno, ino, local->namelen);
@@ -519,13 +512,14 @@ process_leaf_attr_remote(
 
 	remotep = xfs_attr3_leaf_name_remote(leaf, i);
 
-	if (remotep->namelen == 0 || attr_namecheck(remotep->name,
-						remotep->namelen) ||
-			be32_to_cpu(entry->hashval) !=
-				libxfs_da_hashname((unsigned char *)&remotep->name[0],
-						remotep->namelen) ||
-			be32_to_cpu(entry->hashval) < last_hashval ||
-			be32_to_cpu(remotep->valueblk) == 0) {
+	if (remotep->namelen == 0 ||
+	    !libxfs_attr_namecheck(remotep->name,
+				   remotep->namelen) ||
+	    be32_to_cpu(entry->hashval) !=
+			libxfs_da_hashname((unsigned char *)&remotep->name[0],
+					   remotep->namelen) ||
+	    be32_to_cpu(entry->hashval) < last_hashval ||
+	    be32_to_cpu(remotep->valueblk) == 0) {
 		do_warn(
 	_("inconsistent remote attribute entry %d in attr block %u, ino %" PRIu64 "\n"), i, da_bno, ino);
 		return -1;
diff --git a/repair/da_util.c b/repair/da_util.c
index 4a258e58..8c818ea1 100644
--- a/repair/da_util.c
+++ b/repair/da_util.c
@@ -12,33 +12,6 @@
 #include "bmap.h"
 #include "da_util.h"
 
-/*
- * Takes a name and length (name need not be null-terminated) and whether
- * we are checking a dir (as opposed to an attr).
- * Returns 1 if the name contains a NUL or if a directory entry contains a '/'.
- * Returns 0 if the name checks out.
- */
-int
-namecheck(
-	char	*name,
-	int	length,
-	bool	isadir)
-{
-	char	*c;
-	int	i;
-
-	ASSERT(length < MAXNAMELEN);
-
-	for (c = name, i = 0; i < length; i++, c++) {
-		if (isadir && *c == '/')
-			return 1;
-		if (*c == '\0')
-			return 1;
-	}
-
-	return 0;
-}
-
 /*
  * the cursor gets passed up and down the da btree processing
  * routines.  The interior block processing routines use the
diff --git a/repair/da_util.h b/repair/da_util.h
index 041dff74..90fec00c 100644
--- a/repair/da_util.h
+++ b/repair/da_util.h
@@ -24,12 +24,6 @@ typedef struct da_bt_cursor {
 	struct blkmap		*blkmap;
 } da_bt_cursor_t;
 
-int
-namecheck(
-	char		*name,
-	int		length,
-	bool		isadir);
-
 struct xfs_buf *
 da_read_buf(
 	xfs_mount_t	*mp,
diff --git a/repair/dir2.c b/repair/dir2.c
index 094ecb3d..4ac0084e 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -44,14 +44,6 @@ _("malloc failed (%zu bytes) dir2_add_badlist:ino %" PRIu64 "\n"),
 	l->ino = ino;
 }
 
-static int
-dir_namecheck(
-	uint8_t	*name,
-	int	length)
-{
-	return namecheck((char *)name, length, true);
-}
-
 int
 dir2_is_badino(
 	xfs_ino_t	ino)
@@ -318,7 +310,7 @@ _("entry #%d %s in shortform dir %" PRIu64),
 		 * the length value is stored in a byte
 		 * so it can't be too big, it can only wrap
 		 */
-		if (dir_namecheck(sfep->name, namelen)) {
+		if (!libxfs_dir2_namecheck(sfep->name, namelen)) {
 			/*
 			 * junk entry
 			 */
@@ -789,7 +781,7 @@ _("\twould clear inode number in entry at offset %" PRIdPTR "...\n"),
 		 * during phase 4.
 		 */
 		junkit = dep->name[0] == '/';
-		nm_illegal = dir_namecheck(dep->name, dep->namelen);
+		nm_illegal = !libxfs_dir2_namecheck(dep->name, dep->namelen);
 		if (ino_discovery && nm_illegal) {
 			do_warn(
 _("entry at block %u offset %" PRIdPTR " in directory inode %" PRIu64 " has illegal name \"%*.*s\": "),

[PATCH 09/12] xfs_scrub: fix background-mode sleep throttling

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The comment preceding background_sleep() is wrong -- the function sleeps
100us, not 100ms, for every '-b' passed in after the first one.  This is
really not obvious from the magic numbers, so fix the comment and use
symbolic constants for easier reading.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/common.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)


diff --git a/scrub/common.c b/scrub/common.c
index c877c7c8..1cd2b7ba 100644
--- a/scrub/common.c
+++ b/scrub/common.c
@@ -253,21 +253,23 @@ scrub_nproc_workqueue(
 }
 
 /*
- * Sleep for 100ms * however many -b we got past the initial one.
+ * Sleep for 100us * however many -b we got past the initial one.
  * This is an (albeit clumsy) way to throttle scrub activity.
  */
+#define NSEC_PER_SEC	1000000000ULL
+#define NSEC_PER_USEC	1000ULL
 void
 background_sleep(void)
 {
-	unsigned long long	time;
+	unsigned long long	time_ns;
 	struct timespec		tv;
 
 	if (bg_mode < 2)
 		return;
 
-	time = 100000ULL * (bg_mode - 1);
-	tv.tv_sec = time / 1000000;
-	tv.tv_nsec = time % 1000000;
+	time_ns =  100 * NSEC_PER_USEC * (bg_mode - 1);
+	tv.tv_sec = time_ns / NSEC_PER_SEC;
+	tv.tv_nsec = time_ns % NSEC_PER_SEC;
 	nanosleep(&tv, NULL);
 }
 

[PATCH 10/12] mkfs: allow setting dax flag on root directory

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Teach mkfs to set the DAX flag on the root directory so that all new
files can be created in dax mode.  This is a complement to removing the
mount option.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 man/man8/mkfs.xfs.8 |   11 +++++++++++
 mkfs/xfs_mkfs.c     |   11 +++++++++++
 2 files changed, 22 insertions(+)


diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
index 4b8c78c3..0137f164 100644
--- a/man/man8/mkfs.xfs.8
+++ b/man/man8/mkfs.xfs.8
@@ -391,6 +391,17 @@ All inodes created by
 will have this extent size hint applied.
 The value must be provided in units of filesystem blocks.
 Directories will pass on this hint to newly created children.
+.TP
+.BI dax= value
+All inodes created by
+.B mkfs.xfs
+will have the DAX flag set.
+This means that directories will pass the flag on to newly created files
+and files will use the DAX IO paths when possible.
+This value is either 1 to enable the use or 0 to disable.
+By default,
+.B mkfs.xfs
+will not enable DAX mode.
 .RE
 .TP
 .B \-f
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 09106648..5b66074d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -59,6 +59,7 @@ enum {
 	D_PROJINHERIT,
 	D_EXTSZINHERIT,
 	D_COWEXTSIZE,
+	D_DAX,
 	D_MAX_OPTS,
 };
 
@@ -253,6 +254,7 @@ static struct opt_params dopts = {
 		[D_PROJINHERIT] = "projinherit",
 		[D_EXTSZINHERIT] = "extszinherit",
 		[D_COWEXTSIZE] = "cowextsize",
+		[D_DAX] = "dax",
 	},
 	.subopt_params = {
 		{ .index = D_AGCOUNT,
@@ -368,6 +370,12 @@ static struct opt_params dopts = {
 		  .maxval = UINT_MAX,
 		  .defaultval = SUBOPT_NEEDS_VAL,
 		},
+		{ .index = D_DAX,
+		  .conflicts = { { NULL, LAST_CONFLICT } },
+		  .minval = 0,
+		  .maxval = 1,
+		  .defaultval = 1,
+		},
 	},
 };
 
@@ -1465,6 +1473,9 @@ data_opts_parser(
 		cli->fsx.fsx_cowextsize = getnum(value, opts, subopt);
 		cli->fsx.fsx_xflags |= FS_XFLAG_COWEXTSIZE;
 		break;
+	case D_DAX:
+		cli->fsx.fsx_xflags |= FS_XFLAG_DAX;
+		break;
 	default:
 		return -EINVAL;
 	}

[PATCH 11/12] mkfs: validate start and end of aligned logs

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Validate that the start and end of the log stay within a single AG if
we adjust either end to align to stripe units.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)


diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 5b66074d..8f84536e 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3044,15 +3044,28 @@ align_internal_log(
 	struct xfs_mount	*mp,
 	int			sunit)
 {
+	uint64_t		logend;
+
 	/* round up log start if necessary */
 	if ((cfg->logstart % sunit) != 0)
 		cfg->logstart = ((cfg->logstart + (sunit - 1)) / sunit) * sunit;
 
+	/* if our log start rounds into the next AG we're done */
+	if (!xfs_verify_fsbno(mp, cfg->logstart)) {
+			fprintf(stderr,
+_("Due to stripe alignment, the internal log start (%lld) cannot be aligned\n"
+  "within an allocation group.\n"),
+			(long long) cfg->logstart);
+		usage();
+	}
+
 	/* round up/down the log size now */
 	align_log_size(cfg, sunit);
 
 	/* check the aligned log still fits in an AG. */
-	if (cfg->logblocks > cfg->agsize - XFS_FSB_TO_AGBNO(mp, cfg->logstart)) {
+	logend = cfg->logstart + cfg->logblocks - 1;
+	if (XFS_FSB_TO_AGNO(mp, cfg->logstart) != XFS_FSB_TO_AGNO(mp, logend) ||
+	    !xfs_verify_fsbno(mp, logend)) {
 		fprintf(stderr,
 _("Due to stripe alignment, the internal log size (%lld) is too large.\n"
   "Must fit within an allocation group.\n"),

[PATCH 12/12] mkfs: enable reflink by default

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Data block sharing (a.k.a. reflink) has been stable for a while, so turn
it on by default.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 8f84536e..afed46d0 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2015,7 +2015,7 @@ _("rmapbt not supported without CRC support\n"));
 		}
 		cli->sb_feat.rmapbt = false;
 
-		if (cli->sb_feat.reflink) {
+		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
 			fprintf(stderr,
 _("reflink not supported without CRC support\n"));
 			usage();
@@ -3900,7 +3900,7 @@ main(
 			.finobt = true,
 			.spinodes = true,
 			.rmapbt = false,
-			.reflink = false,
+			.reflink = true,
 			.parent_pointers = false,
 			.nodalign = false,
 			.nortalign = false,

Re: [PATCH 01/12] libxfs: fix attr include mess

[Eric Sandeen]

On 5/20/19 6:16 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Remove all the userspace xfs_attr shim cruft so that we have one
> definition of ATTR_* macros so that we can actually use xfs_attr.c
> routines and include xfs_attr.h without problems.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

<handwaves about namespace for flag #defines ok sure>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  include/libxfs.h         |   10 +---------
>  libxfs/libxfs_api_defs.h |    5 +++++
>  libxfs/libxfs_priv.h     |    8 --------
>  libxfs/xfs_attr.c        |    1 +
>  libxfs/xfs_attr_leaf.c   |    1 +
>  5 files changed, 8 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/include/libxfs.h b/include/libxfs.h
> index 230bc3e8..dd5fe542 100644
> --- a/include/libxfs.h
> +++ b/include/libxfs.h
> @@ -211,14 +211,6 @@ libxfs_bmbt_disk_get_all(
>  int libxfs_rtfree_extent(struct xfs_trans *, xfs_rtblock_t, xfs_extlen_t);
>  bool libxfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
>  
> -/* XXX: need parts of xfs_attr.h in userspace */
> -#define LIBXFS_ATTR_ROOT	0x0002	/* use attrs in root namespace */
> -#define LIBXFS_ATTR_SECURE	0x0008	/* use attrs in security namespace */
> -#define LIBXFS_ATTR_CREATE	0x0010	/* create, but fail if attr exists */
> -#define LIBXFS_ATTR_REPLACE	0x0020	/* set, but fail if attr not exists */
> -
> -int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
> -int xfs_attr_set(struct xfs_inode *dp, const unsigned char *name,
> -		 unsigned char *value, int valuelen, int flags);
> +#include "xfs_attr.h"
>  
>  #endif	/* __LIBXFS_H__ */
> diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
> index 1150ec93..34bb552d 100644
> --- a/libxfs/libxfs_api_defs.h
> +++ b/libxfs/libxfs_api_defs.h
> @@ -144,4 +144,9 @@
>  #define xfs_fs_geometry			libxfs_fs_geometry
>  #define xfs_init_local_fork		libxfs_init_local_fork
>  
> +#define LIBXFS_ATTR_ROOT		ATTR_ROOT
> +#define LIBXFS_ATTR_SECURE		ATTR_SECURE
> +#define LIBXFS_ATTR_CREATE		ATTR_CREATE
> +#define LIBXFS_ATTR_REPLACE		ATTR_REPLACE
> +
>  #endif /* __LIBXFS_API_DEFS_H__ */
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index d668a157..f60bff06 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -104,14 +104,6 @@ extern char    *progname;
>   */
>  #define PTR_FMT "%p"
>  
> -/* XXX: need to push these out to make LIBXFS_ATTR defines */
> -#define ATTR_ROOT			0x0002
> -#define ATTR_SECURE			0x0008
> -#define ATTR_CREATE			0x0010
> -#define ATTR_REPLACE			0x0020
> -#define ATTR_KERNOTIME			0
> -#define ATTR_KERNOVAL			0
> -
>  #define XFS_IGET_CREATE			0x1
>  #define XFS_IGET_UNTRUSTED		0x2
>  
> diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
> index b8838302..170e64cf 100644
> --- a/libxfs/xfs_attr.c
> +++ b/libxfs/xfs_attr.c
> @@ -20,6 +20,7 @@
>  #include "xfs_trans.h"
>  #include "xfs_bmap.h"
>  #include "xfs_bmap_btree.h"
> +#include "xfs_attr.h"
>  #include "xfs_attr_leaf.h"
>  #include "xfs_attr_remote.h"
>  #include "xfs_trans_space.h"
> diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
> index 679c7d0d..1027ca01 100644
> --- a/libxfs/xfs_attr_leaf.c
> +++ b/libxfs/xfs_attr_leaf.c
> @@ -21,6 +21,7 @@
>  #include "xfs_bmap.h"
>  #include "xfs_attr_sf.h"
>  #include "xfs_attr_remote.h"
> +#include "xfs_attr.h"
>  #include "xfs_attr_leaf.h"
>  #include "xfs_trace.h"
>  #include "xfs_cksum.h"
> 

Re: [PATCH 02/12] libxfs: set m_finobt_nores when initializing library

[Eric Sandeen]

On 5/20/19 6:16 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> We don't generally set up per-ag reservations in userspace, so set this
> flag to true to force transactions to set up block reservations.  This
> isn't necessary for userspace (since we never touch the finobt) but we
> shouldn't leave a logic bomb.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  libxfs/init.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> 
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 2f6decc8..1baccb31 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -640,6 +640,7 @@ libxfs_mount(
>  
>  	libxfs_buftarg_init(mp, dev, logdev, rtdev);
>  
> +	mp->m_finobt_nores = true;
>  	mp->m_flags = (LIBXFS_MOUNT_32BITINODES|LIBXFS_MOUNT_32BITINOOPT);
>  	mp->m_sb = *sb;
>  	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_KERNEL);
> 

Re: [PATCH 03/12] libxfs: refactor online geometry queries

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> 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.

Ok, helper is nice, but... libhandle?  I don't see how a geometry ioctl
wrapper is related to libhandle.  Would this make more sense in libfrog/ ?

-Eric

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  Makefile            |    9 +++++----
>  fsr/xfs_fsr.c       |   25 +++----------------------
>  growfs/Makefile     |    5 +++--
>  growfs/xfs_growfs.c |   24 ++++++++----------------
>  include/linux.h     |    5 +++++
>  io/bmap.c           |    2 +-
>  io/fsmap.c          |    2 +-
>  io/open.c           |    2 +-
>  io/stat.c           |    4 ++--
>  libhandle/Makefile  |    2 +-
>  libhandle/ioctl.c   |   26 ++++++++++++++++++++++++++
>  quota/Makefile      |    4 ++--
>  quota/free.c        |    5 ++---
>  repair/Makefile     |    6 +++---
>  repair/xfs_repair.c |    4 ++--
>  rtcp/Makefile       |    3 +++
>  rtcp/xfs_rtcp.c     |    6 +++---
>  scrub/phase1.c      |    2 +-
>  spaceman/Makefile   |    4 ++--
>  spaceman/file.c     |    2 +-
>  spaceman/info.c     |   24 +++++++-----------------
>  21 files changed, 82 insertions(+), 84 deletions(-)
>  create mode 100644 libhandle/ioctl.c
> 
> 
> diff --git a/Makefile b/Makefile
> index 9204bed8..b72a9209 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -99,14 +99,15 @@ $(LIB_SUBDIRS) $(TOOL_SUBDIRS): include libfrog
>  $(DLIB_SUBDIRS) $(TOOL_SUBDIRS): libxfs
>  db logprint: libxlog
>  fsr: libhandle
> -growfs: libxcmd
> +growfs: libxcmd libhandle
>  io: libxcmd libhandle
> -quota: libxcmd
> -repair: libxlog libxcmd
> +quota: libxcmd libhandle
> +repair: libxlog libxcmd libhandle
>  copy: libxlog
>  mkfs: libxcmd
> -spaceman: libxcmd
> +spaceman: libxcmd libhandle
>  scrub: libhandle libxcmd
> +rtcp: libhandle
>  
>  ifeq ($(HAVE_BUILDDEFS), yes)
>  include $(BUILDRULES)

...

Re: [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Remove all the uses of the old xfs_fsop_geom_t typedef.

Ok.  Any complaint if I tab stuff out to line up again when I commit
it, assuming it doesn't cause 80char problems?

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  growfs/xfs_growfs.c |    4 ++--
>  io/init.c           |    2 +-
>  io/io.h             |    6 +++---
>  io/open.c           |    6 +++---
>  man/man3/xfsctl.3   |    2 +-
>  spaceman/file.c     |    4 ++--
>  spaceman/init.c     |    2 +-
>  spaceman/space.h    |    6 +++---
>  8 files changed, 16 insertions(+), 16 deletions(-)
> 
> 
> diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
> index 392e4a00..ffd82f95 100644
> --- a/growfs/xfs_growfs.c
> +++ b/growfs/xfs_growfs.c
> @@ -44,7 +44,7 @@ main(int argc, char **argv)
>  	int			error;	/* we have hit an error */
>  	long			esize;	/* new rt extent size */
>  	int			ffd;	/* mount point file descriptor */
> -	xfs_fsop_geom_t		geo;	/* current fs geometry */
> +	struct xfs_fsop_geom	geo;	/* current fs geometry */
>  	int			iflag;	/* -i flag */
>  	int			isint;	/* log is currently internal */
>  	int			lflag;	/* -l flag */
> @@ -52,7 +52,7 @@ main(int argc, char **argv)
>  	int			maxpct;	/* -m flag value */
>  	int			mflag;	/* -m flag */
>  	int			nflag;	/* -n flag */
> -	xfs_fsop_geom_t		ngeo;	/* new fs geometry */
> +	struct xfs_fsop_geom	ngeo;	/* new fs geometry */
>  	int			rflag;	/* -r flag */
>  	long long		rsize;	/* new rt size in fs blocks */
>  	int			xflag;	/* -x flag */
> diff --git a/io/init.c b/io/init.c
> index 83f08f2d..7025aea5 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -133,7 +133,7 @@ init(
>  	int		c, flags = 0;
>  	char		*sp;
>  	mode_t		mode = 0600;
> -	xfs_fsop_geom_t	geometry = { 0 };
> +	struct xfs_fsop_geom geometry = { 0 };
>  	struct fs_path	fsp;
>  
>  	progname = basename(argv[0]);
> diff --git a/io/io.h b/io/io.h
> index 6469179e..0848ab98 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -38,7 +38,7 @@ typedef struct fileio {
>  	int		fd;		/* open file descriptor */
>  	int		flags;		/* flags describing file state */
>  	char		*name;		/* file name at time of open */
> -	xfs_fsop_geom_t	geom;		/* XFS filesystem geometry */
> +	struct xfs_fsop_geom geom;	/* XFS filesystem geometry */
>  	struct fs_path	fs_path;	/* XFS path information */
>  } fileio_t;
>  
> @@ -70,9 +70,9 @@ extern void *check_mapping_range(mmap_region_t *, off64_t, size_t, int);
>   */
>  
>  extern off64_t		filesize(void);
> -extern int		openfile(char *, xfs_fsop_geom_t *, int, mode_t,
> +extern int		openfile(char *, struct xfs_fsop_geom *, int, mode_t,
>  				 struct fs_path *);
> -extern int		addfile(char *, int , xfs_fsop_geom_t *, int,
> +extern int		addfile(char *, int , struct xfs_fsop_geom *, int,
>  				struct fs_path *);
>  extern void		printxattr(uint, int, int, const char *, int, int);
>  
> diff --git a/io/open.c b/io/open.c
> index 11805cd7..ce7a5362 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -51,7 +51,7 @@ static long extsize;
>  int
>  openfile(
>  	char		*path,
> -	xfs_fsop_geom_t	*geom,
> +	struct xfs_fsop_geom *geom,
>  	int		flags,
>  	mode_t		mode,
>  	struct fs_path	*fs_path)
> @@ -156,7 +156,7 @@ int
>  addfile(
>  	char		*name,
>  	int		fd,
> -	xfs_fsop_geom_t	*geometry,
> +	struct xfs_fsop_geom *geometry,
>  	int		flags,
>  	struct fs_path	*fs_path)
>  {
> @@ -229,7 +229,7 @@ open_f(
>  	int		c, fd, flags = 0;
>  	char		*sp;
>  	mode_t		mode = 0600;
> -	xfs_fsop_geom_t	geometry = { 0 };
> +	struct xfs_fsop_geom geometry = { 0 };
>  	struct fs_path	fsp;
>  
>  	if (argc == 1) {
> diff --git a/man/man3/xfsctl.3 b/man/man3/xfsctl.3
> index 6e5027c4..462ccbd8 100644
> --- a/man/man3/xfsctl.3
> +++ b/man/man3/xfsctl.3
> @@ -640,7 +640,7 @@ operations on XFS filesystems.
>  For
>  .B XFS_IOC_FSGEOMETRY
>  (get filesystem mkfs time information), the output structure is of type
> -.BR xfs_fsop_geom_t .
> +.BR struct xfs_fsop_geom .
>  For
>  .B XFS_FS_COUNTS
>  (get filesystem dynamic global information), the output structure is of type
> diff --git a/spaceman/file.c b/spaceman/file.c
> index d2acf5db..a9b8461f 100644
> --- a/spaceman/file.c
> +++ b/spaceman/file.c
> @@ -44,7 +44,7 @@ print_f(
>  int
>  openfile(
>  	char		*path,
> -	xfs_fsop_geom_t	*geom,
> +	struct xfs_fsop_geom *geom,
>  	struct fs_path	*fs_path)
>  {
>  	struct fs_path	*fsp;
> @@ -84,7 +84,7 @@ int
>  addfile(
>  	char		*name,
>  	int		fd,
> -	xfs_fsop_geom_t	*geometry,
> +	struct xfs_fsop_geom *geometry,
>  	struct fs_path	*fs_path)
>  {
>  	char		*filename;
> diff --git a/spaceman/init.c b/spaceman/init.c
> index 181a3446..c845f920 100644
> --- a/spaceman/init.c
> +++ b/spaceman/init.c
> @@ -60,7 +60,7 @@ init(
>  	char		**argv)
>  {
>  	int		c;
> -	xfs_fsop_geom_t	geometry = { 0 };
> +	struct xfs_fsop_geom geometry = { 0 };
>  	struct fs_path	fsp;
>  
>  	progname = basename(argv[0]);
> diff --git a/spaceman/space.h b/spaceman/space.h
> index bf9cc2bf..b246f602 100644
> --- a/spaceman/space.h
> +++ b/spaceman/space.h
> @@ -7,7 +7,7 @@
>  #define XFS_SPACEMAN_SPACE_H_
>  
>  typedef struct fileio {
> -	xfs_fsop_geom_t	geom;		/* XFS filesystem geometry */
> +	struct xfs_fsop_geom geom;		/* XFS filesystem geometry */
>  	struct fs_path	fs_path;	/* XFS path information */
>  	char		*name;		/* file name at time of open */
>  	int		fd;		/* open file descriptor */
> @@ -17,8 +17,8 @@ extern fileio_t		*filetable;	/* open file table */
>  extern int		filecount;	/* number of open files */
>  extern fileio_t		*file;		/* active file in file table */
>  
> -extern int	openfile(char *, xfs_fsop_geom_t *, struct fs_path *);
> -extern int	addfile(char *, int , xfs_fsop_geom_t *, struct fs_path *);
> +extern int	openfile(char *, struct xfs_fsop_geom *, struct fs_path *);
> +extern int	addfile(char *, int , struct xfs_fsop_geom *, struct fs_path *);
>  
>  extern void	print_init(void);
>  extern void	help_init(void);
> 

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Fix the return types of non-predicate bitmap functions to return the
> usual negative error codes instead of the "moveon" boolean.

This seems much better, though how did you decide on negative
error codes?  They are usual for the kernel, but in userspace
we have kind of a mishmash, even in libfrog.

  File                 Function                 Line
0 libfrog/paths.c      fs_table_insert          176 error = ENOMEM;
1 libfrog/paths.c      fs_extract_mount_options 354 return ENOMEM;
2 libfrog/radix-tree.c radix_tree_extend        135 return -ENOMEM;
3 libfrog/radix-tree.c radix_tree_insert        188 return -ENOMEM;
4 libfrog/workqueue.c  workqueue_add            110 return ENOMEM;

3 libfrog/paths.c         fs_table_initialise_mounts         384 return ENOENT;
4 libfrog/paths.c         fs_table_initialise_projects       489 error = ENOENT;
5 libfrog/paths.c         fs_table_insert_project_path       560 error = ENOENT;



> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  include/bitmap.h |    8 +++--
>  libfrog/bitmap.c |   86 ++++++++++++++++++++++++++----------------------------
>  repair/rmap.c    |   18 ++++++++---
>  scrub/phase6.c   |   18 ++++-------
>  4 files changed, 65 insertions(+), 65 deletions(-)
> 
> 
> diff --git a/include/bitmap.h b/include/bitmap.h
> index e29a4335..99a2fb23 100644
> --- a/include/bitmap.h
> +++ b/include/bitmap.h
> @@ -11,11 +11,11 @@ struct bitmap {
>  	struct avl64tree_desc	*bt_tree;
>  };
>  
> -bool bitmap_init(struct bitmap **bmap);
> +int bitmap_init(struct bitmap **bmap);
>  void bitmap_free(struct bitmap **bmap);
> -bool bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> -bool bitmap_iterate(struct bitmap *bmap,
> -		bool (*fn)(uint64_t, uint64_t, void *), void *arg);
> +int bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> +int bitmap_iterate(struct bitmap *bmap, int (*fn)(uint64_t, uint64_t, void *),
> +		void *arg);
>  bool bitmap_test(struct bitmap *bmap, uint64_t start,
>  		uint64_t len);
>  bool bitmap_empty(struct bitmap *bmap);
> diff --git a/libfrog/bitmap.c b/libfrog/bitmap.c
> index 450ebe0a..4dafc4c9 100644
> --- a/libfrog/bitmap.c
> +++ b/libfrog/bitmap.c
> @@ -66,7 +66,7 @@ static struct avl64ops bitmap_ops = {
>  };
>  
>  /* Initialize a bitmap. */
> -bool
> +int
>  bitmap_init(
>  	struct bitmap		**bmapp)
>  {
> @@ -74,18 +74,18 @@ bitmap_init(
>  
>  	bmap = calloc(1, sizeof(struct bitmap));
>  	if (!bmap)
> -		return false;
> +		return -ENOMEM;
>  	bmap->bt_tree = malloc(sizeof(struct avl64tree_desc));
>  	if (!bmap->bt_tree) {
>  		free(bmap);
> -		return false;
> +		return -ENOMEM;
>  	}
>  
>  	pthread_mutex_init(&bmap->bt_lock, NULL);
>  	avl64_init_tree(bmap->bt_tree, &bitmap_ops);
>  	*bmapp = bmap;
>  
> -	return true;
> +	return 0;
>  }
>  
>  /* Free a bitmap. */
> @@ -127,8 +127,31 @@ bitmap_node_init(
>  	return ext;
>  }
>  
> +/* Create a new bitmap node and insert it. */
> +static inline int
> +__bitmap_insert(
> +	struct bitmap		*bmap,
> +	uint64_t		start,
> +	uint64_t		length)
> +{
> +	struct bitmap_node	*ext;
> +	struct avl64node	*node;
> +
> +	ext = bitmap_node_init(start, length);
> +	if (!ext)
> +		return -ENOMEM;
> +
> +	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> +	if (node == NULL) {
> +		free(ext);
> +		return -EEXIST;
> +	}
> +
> +	return 0;
> +}
> +
>  /* Set a region of bits (locked). */
> -static bool
> +static int
>  __bitmap_set(
>  	struct bitmap		*bmap,
>  	uint64_t		start,
> @@ -142,28 +165,14 @@ __bitmap_set(
>  	struct bitmap_node	*ext;
>  	uint64_t		new_start;
>  	uint64_t		new_length;
> -	struct avl64node	*node;
> -	bool			res = true;
>  
>  	/* Find any existing nodes adjacent or within that range. */
>  	avl64_findranges(bmap->bt_tree, start - 1, start + length + 1,
>  			&firstn, &lastn);
>  
>  	/* Nothing, just insert a new extent. */
> -	if (firstn == NULL && lastn == NULL) {
> -		ext = bitmap_node_init(start, length);
> -		if (!ext)
> -			return false;
> -
> -		node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> -		if (node == NULL) {
> -			free(ext);
> -			errno = EEXIST;
> -			return false;
> -		}
> -
> -		return true;
> -	}
> +	if (firstn == NULL && lastn == NULL)
> +		return __bitmap_insert(bmap, start, length);
>  
>  	assert(firstn != NULL && lastn != NULL);
>  	new_start = start;
> @@ -175,7 +184,7 @@ __bitmap_set(
>  		/* Bail if the new extent is contained within an old one. */
>  		if (ext->btn_start <= start &&
>  		    ext->btn_start + ext->btn_length >= start + length)
> -			return res;
> +			return 0;
>  
>  		/* Check for overlapping and adjacent extents. */
>  		if (ext->btn_start + ext->btn_length >= start ||
> @@ -195,28 +204,17 @@ __bitmap_set(
>  		}
>  	}
>  
> -	ext = bitmap_node_init(new_start, new_length);
> -	if (!ext)
> -		return false;
> -
> -	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> -	if (node == NULL) {
> -		free(ext);
> -		errno = EEXIST;
> -		return false;
> -	}
> -
> -	return res;
> +	return __bitmap_insert(bmap, new_start, new_length);
>  }
>  
>  /* Set a region of bits. */
> -bool
> +int
>  bitmap_set(
>  	struct bitmap		*bmap,
>  	uint64_t		start,
>  	uint64_t		length)
>  {
> -	bool			res;
> +	int			res;
>  
>  	pthread_mutex_lock(&bmap->bt_lock);
>  	res = __bitmap_set(bmap, start, length);
> @@ -308,26 +306,26 @@ bitmap_clear(
>  
>  #ifdef DEBUG
>  /* Iterate the set regions of this bitmap. */
> -bool
> +int
>  bitmap_iterate(
>  	struct bitmap		*bmap,
> -	bool			(*fn)(uint64_t, uint64_t, void *),
> +	int			(*fn)(uint64_t, uint64_t, void *),
>  	void			*arg)
>  {
>  	struct avl64node	*node;
>  	struct bitmap_node	*ext;
> -	bool			moveon = true;
> +	int			error = 0;
>  
>  	pthread_mutex_lock(&bmap->bt_lock);
>  	avl_for_each(bmap->bt_tree, node) {
>  		ext = container_of(node, struct bitmap_node, btn_node);
> -		moveon = fn(ext->btn_start, ext->btn_length, arg);
> -		if (!moveon)
> +		error = fn(ext->btn_start, ext->btn_length, arg);
> +		if (error)
>  			break;
>  	}
>  	pthread_mutex_unlock(&bmap->bt_lock);
>  
> -	return moveon;
> +	return error;
>  }
>  #endif
>  
> @@ -372,14 +370,14 @@ bitmap_empty(
>  }
>  
>  #ifdef DEBUG
> -static bool
> +static int
>  bitmap_dump_fn(
>  	uint64_t		startblock,
>  	uint64_t		blockcount,
>  	void			*arg)
>  {
>  	printf("%"PRIu64":%"PRIu64"\n", startblock, blockcount);
> -	return true;
> +	return 0;
>  }
>  
>  /* Dump bitmap. */
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 19cceca3..47828a06 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -490,16 +490,22 @@ rmap_store_ag_btree_rec(
>  	error = init_slab_cursor(ag_rmap->ar_raw_rmaps, rmap_compare, &rm_cur);
>  	if (error)
>  		goto err;
> -	if (!bitmap_init(&own_ag_bitmap)) {
> -		error = -ENOMEM;
> +	error = -bitmap_init(&own_ag_bitmap);
> +	if (error)
>  		goto err_slab;
> -	}
>  	while ((rm_rec = pop_slab_cursor(rm_cur)) != NULL) {
>  		if (rm_rec->rm_owner != XFS_RMAP_OWN_AG)
>  			continue;
> -		if (!bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> -					rm_rec->rm_blockcount)) {
> -			error = EFSCORRUPTED;
> +		error = -bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> +					rm_rec->rm_blockcount);
> +		if (error) {
> +			/*
> +			 * If this range is already set, then the incore rmap
> +			 * records for the AG free space btrees overlap and
> +			 * we're toast because that is not allowed.
> +			 */
> +			if (error == EEXIST)
> +				error = EFSCORRUPTED;
>  			goto err_slab;
>  		}
>  	}
> diff --git a/scrub/phase6.c b/scrub/phase6.c
> index 4b25f3bb..66e6451c 100644
> --- a/scrub/phase6.c
> +++ b/scrub/phase6.c
> @@ -341,7 +341,6 @@ xfs_check_rmap_ioerr(
>  	struct media_verify_state	*vs = arg;
>  	struct bitmap			*tree;
>  	dev_t				dev;
> -	bool				moveon;
>  
>  	dev = xfs_disk_to_dev(ctx, disk);
>  
> @@ -356,8 +355,8 @@ xfs_check_rmap_ioerr(
>  	else
>  		tree = NULL;
>  	if (tree) {
> -		moveon = bitmap_set(tree, start, length);
> -		if (!moveon)
> +		errno = -bitmap_set(tree, start, length);
> +		if (errno)
>  			str_errno(ctx, ctx->mntpoint);
>  	}
>  
> @@ -454,16 +453,16 @@ xfs_scan_blocks(
>  	struct scrub_ctx		*ctx)
>  {
>  	struct media_verify_state	vs = { NULL };
> -	bool				moveon;
> +	bool				moveon = false;
>  
> -	moveon = bitmap_init(&vs.d_bad);
> -	if (!moveon) {
> +	errno = -bitmap_init(&vs.d_bad);
> +	if (errno) {
>  		str_errno(ctx, ctx->mntpoint);
>  		goto out;
>  	}
>  
> -	moveon = bitmap_init(&vs.r_bad);
> -	if (!moveon) {
> +	errno = -bitmap_init(&vs.r_bad);
> +	if (errno) {
>  		str_errno(ctx, ctx->mntpoint);
>  		goto out_dbad;
>  	}
> @@ -472,7 +471,6 @@ xfs_scan_blocks(
>  			ctx->geo.blocksize, xfs_check_rmap_ioerr,
>  			scrub_nproc(ctx));
>  	if (!vs.rvp_data) {
> -		moveon = false;
>  		str_info(ctx, ctx->mntpoint,
>  _("Could not create data device media verifier."));
>  		goto out_rbad;
> @@ -482,7 +480,6 @@ _("Could not create data device media verifier."));
>  				ctx->geo.blocksize, xfs_check_rmap_ioerr,
>  				scrub_nproc(ctx));
>  		if (!vs.rvp_log) {
> -			moveon = false;
>  			str_info(ctx, ctx->mntpoint,
>  	_("Could not create log device media verifier."));
>  			goto out_datapool;
> @@ -493,7 +490,6 @@ _("Could not create data device media verifier."));
>  				ctx->geo.blocksize, xfs_check_rmap_ioerr,
>  				scrub_nproc(ctx));
>  		if (!vs.rvp_realtime) {
> -			moveon = false;
>  			str_info(ctx, ctx->mntpoint,
>  	_("Could not create realtime device media verifier."));
>  			goto out_logpool;
> 

Re: [PATCH 03/12] libxfs: refactor online geometry queries

[Darrick J. Wong]

On Tue, May 21, 2019 at 11:38:56AM -0500, Eric Sandeen wrote:
> On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > 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.
> 
> Ok, helper is nice, but... libhandle?  I don't see how a geometry ioctl
> wrapper is related to libhandle.  Would this make more sense in libfrog/ ?

Secret goal here : I'd also like to convert xfsdump and xfstests to use
these helpers instead of forcing everyone to write their own graceful
degradation gluecode if they want to keep up with the new FSGEOMETRY
ioctl we're introducing in 5.2.

At the same time, putting it in libhandle means we need a better prefix
than xfs_ since that's for libxfs stuff.  Uh, maybe I'll redo this patch
with xfrog_ instead of xfs_ ?

--D

> -Eric
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  Makefile            |    9 +++++----
> >  fsr/xfs_fsr.c       |   25 +++----------------------
> >  growfs/Makefile     |    5 +++--
> >  growfs/xfs_growfs.c |   24 ++++++++----------------
> >  include/linux.h     |    5 +++++
> >  io/bmap.c           |    2 +-
> >  io/fsmap.c          |    2 +-
> >  io/open.c           |    2 +-
> >  io/stat.c           |    4 ++--
> >  libhandle/Makefile  |    2 +-
> >  libhandle/ioctl.c   |   26 ++++++++++++++++++++++++++
> >  quota/Makefile      |    4 ++--
> >  quota/free.c        |    5 ++---
> >  repair/Makefile     |    6 +++---
> >  repair/xfs_repair.c |    4 ++--
> >  rtcp/Makefile       |    3 +++
> >  rtcp/xfs_rtcp.c     |    6 +++---
> >  scrub/phase1.c      |    2 +-
> >  spaceman/Makefile   |    4 ++--
> >  spaceman/file.c     |    2 +-
> >  spaceman/info.c     |   24 +++++++-----------------
> >  21 files changed, 82 insertions(+), 84 deletions(-)
> >  create mode 100644 libhandle/ioctl.c
> > 
> > 
> > diff --git a/Makefile b/Makefile
> > index 9204bed8..b72a9209 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -99,14 +99,15 @@ $(LIB_SUBDIRS) $(TOOL_SUBDIRS): include libfrog
> >  $(DLIB_SUBDIRS) $(TOOL_SUBDIRS): libxfs
> >  db logprint: libxlog
> >  fsr: libhandle
> > -growfs: libxcmd
> > +growfs: libxcmd libhandle
> >  io: libxcmd libhandle
> > -quota: libxcmd
> > -repair: libxlog libxcmd
> > +quota: libxcmd libhandle
> > +repair: libxlog libxcmd libhandle
> >  copy: libxlog
> >  mkfs: libxcmd
> > -spaceman: libxcmd
> > +spaceman: libxcmd libhandle
> >  scrub: libhandle libxcmd
> > +rtcp: libhandle
> >  
> >  ifeq ($(HAVE_BUILDDEFS), yes)
> >  include $(BUILDRULES)
> 
> ...

Re: [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t

[Darrick J. Wong]

On Tue, May 21, 2019 at 11:43:43AM -0500, Eric Sandeen wrote:
> On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Remove all the uses of the old xfs_fsop_geom_t typedef.
> 
> Ok.  Any complaint if I tab stuff out to line up again when I commit
> it, assuming it doesn't cause 80char problems?

None here.

--D

> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  growfs/xfs_growfs.c |    4 ++--
> >  io/init.c           |    2 +-
> >  io/io.h             |    6 +++---
> >  io/open.c           |    6 +++---
> >  man/man3/xfsctl.3   |    2 +-
> >  spaceman/file.c     |    4 ++--
> >  spaceman/init.c     |    2 +-
> >  spaceman/space.h    |    6 +++---
> >  8 files changed, 16 insertions(+), 16 deletions(-)
> > 
> > 
> > diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
> > index 392e4a00..ffd82f95 100644
> > --- a/growfs/xfs_growfs.c
> > +++ b/growfs/xfs_growfs.c
> > @@ -44,7 +44,7 @@ main(int argc, char **argv)
> >  	int			error;	/* we have hit an error */
> >  	long			esize;	/* new rt extent size */
> >  	int			ffd;	/* mount point file descriptor */
> > -	xfs_fsop_geom_t		geo;	/* current fs geometry */
> > +	struct xfs_fsop_geom	geo;	/* current fs geometry */
> >  	int			iflag;	/* -i flag */
> >  	int			isint;	/* log is currently internal */
> >  	int			lflag;	/* -l flag */
> > @@ -52,7 +52,7 @@ main(int argc, char **argv)
> >  	int			maxpct;	/* -m flag value */
> >  	int			mflag;	/* -m flag */
> >  	int			nflag;	/* -n flag */
> > -	xfs_fsop_geom_t		ngeo;	/* new fs geometry */
> > +	struct xfs_fsop_geom	ngeo;	/* new fs geometry */
> >  	int			rflag;	/* -r flag */
> >  	long long		rsize;	/* new rt size in fs blocks */
> >  	int			xflag;	/* -x flag */
> > diff --git a/io/init.c b/io/init.c
> > index 83f08f2d..7025aea5 100644
> > --- a/io/init.c
> > +++ b/io/init.c
> > @@ -133,7 +133,7 @@ init(
> >  	int		c, flags = 0;
> >  	char		*sp;
> >  	mode_t		mode = 0600;
> > -	xfs_fsop_geom_t	geometry = { 0 };
> > +	struct xfs_fsop_geom geometry = { 0 };
> >  	struct fs_path	fsp;
> >  
> >  	progname = basename(argv[0]);
> > diff --git a/io/io.h b/io/io.h
> > index 6469179e..0848ab98 100644
> > --- a/io/io.h
> > +++ b/io/io.h
> > @@ -38,7 +38,7 @@ typedef struct fileio {
> >  	int		fd;		/* open file descriptor */
> >  	int		flags;		/* flags describing file state */
> >  	char		*name;		/* file name at time of open */
> > -	xfs_fsop_geom_t	geom;		/* XFS filesystem geometry */
> > +	struct xfs_fsop_geom geom;	/* XFS filesystem geometry */
> >  	struct fs_path	fs_path;	/* XFS path information */
> >  } fileio_t;
> >  
> > @@ -70,9 +70,9 @@ extern void *check_mapping_range(mmap_region_t *, off64_t, size_t, int);
> >   */
> >  
> >  extern off64_t		filesize(void);
> > -extern int		openfile(char *, xfs_fsop_geom_t *, int, mode_t,
> > +extern int		openfile(char *, struct xfs_fsop_geom *, int, mode_t,
> >  				 struct fs_path *);
> > -extern int		addfile(char *, int , xfs_fsop_geom_t *, int,
> > +extern int		addfile(char *, int , struct xfs_fsop_geom *, int,
> >  				struct fs_path *);
> >  extern void		printxattr(uint, int, int, const char *, int, int);
> >  
> > diff --git a/io/open.c b/io/open.c
> > index 11805cd7..ce7a5362 100644
> > --- a/io/open.c
> > +++ b/io/open.c
> > @@ -51,7 +51,7 @@ static long extsize;
> >  int
> >  openfile(
> >  	char		*path,
> > -	xfs_fsop_geom_t	*geom,
> > +	struct xfs_fsop_geom *geom,
> >  	int		flags,
> >  	mode_t		mode,
> >  	struct fs_path	*fs_path)
> > @@ -156,7 +156,7 @@ int
> >  addfile(
> >  	char		*name,
> >  	int		fd,
> > -	xfs_fsop_geom_t	*geometry,
> > +	struct xfs_fsop_geom *geometry,
> >  	int		flags,
> >  	struct fs_path	*fs_path)
> >  {
> > @@ -229,7 +229,7 @@ open_f(
> >  	int		c, fd, flags = 0;
> >  	char		*sp;
> >  	mode_t		mode = 0600;
> > -	xfs_fsop_geom_t	geometry = { 0 };
> > +	struct xfs_fsop_geom geometry = { 0 };
> >  	struct fs_path	fsp;
> >  
> >  	if (argc == 1) {
> > diff --git a/man/man3/xfsctl.3 b/man/man3/xfsctl.3
> > index 6e5027c4..462ccbd8 100644
> > --- a/man/man3/xfsctl.3
> > +++ b/man/man3/xfsctl.3
> > @@ -640,7 +640,7 @@ operations on XFS filesystems.
> >  For
> >  .B XFS_IOC_FSGEOMETRY
> >  (get filesystem mkfs time information), the output structure is of type
> > -.BR xfs_fsop_geom_t .
> > +.BR struct xfs_fsop_geom .
> >  For
> >  .B XFS_FS_COUNTS
> >  (get filesystem dynamic global information), the output structure is of type
> > diff --git a/spaceman/file.c b/spaceman/file.c
> > index d2acf5db..a9b8461f 100644
> > --- a/spaceman/file.c
> > +++ b/spaceman/file.c
> > @@ -44,7 +44,7 @@ print_f(
> >  int
> >  openfile(
> >  	char		*path,
> > -	xfs_fsop_geom_t	*geom,
> > +	struct xfs_fsop_geom *geom,
> >  	struct fs_path	*fs_path)
> >  {
> >  	struct fs_path	*fsp;
> > @@ -84,7 +84,7 @@ int
> >  addfile(
> >  	char		*name,
> >  	int		fd,
> > -	xfs_fsop_geom_t	*geometry,
> > +	struct xfs_fsop_geom *geometry,
> >  	struct fs_path	*fs_path)
> >  {
> >  	char		*filename;
> > diff --git a/spaceman/init.c b/spaceman/init.c
> > index 181a3446..c845f920 100644
> > --- a/spaceman/init.c
> > +++ b/spaceman/init.c
> > @@ -60,7 +60,7 @@ init(
> >  	char		**argv)
> >  {
> >  	int		c;
> > -	xfs_fsop_geom_t	geometry = { 0 };
> > +	struct xfs_fsop_geom geometry = { 0 };
> >  	struct fs_path	fsp;
> >  
> >  	progname = basename(argv[0]);
> > diff --git a/spaceman/space.h b/spaceman/space.h
> > index bf9cc2bf..b246f602 100644
> > --- a/spaceman/space.h
> > +++ b/spaceman/space.h
> > @@ -7,7 +7,7 @@
> >  #define XFS_SPACEMAN_SPACE_H_
> >  
> >  typedef struct fileio {
> > -	xfs_fsop_geom_t	geom;		/* XFS filesystem geometry */
> > +	struct xfs_fsop_geom geom;		/* XFS filesystem geometry */
> >  	struct fs_path	fs_path;	/* XFS path information */
> >  	char		*name;		/* file name at time of open */
> >  	int		fd;		/* open file descriptor */
> > @@ -17,8 +17,8 @@ extern fileio_t		*filetable;	/* open file table */
> >  extern int		filecount;	/* number of open files */
> >  extern fileio_t		*file;		/* active file in file table */
> >  
> > -extern int	openfile(char *, xfs_fsop_geom_t *, struct fs_path *);
> > -extern int	addfile(char *, int , xfs_fsop_geom_t *, struct fs_path *);
> > +extern int	openfile(char *, struct xfs_fsop_geom *, struct fs_path *);
> > +extern int	addfile(char *, int , struct xfs_fsop_geom *, struct fs_path *);
> >  
> >  extern void	print_init(void);
> >  extern void	help_init(void);
> > 

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Darrick J. Wong]

On Tue, May 21, 2019 at 11:54:18AM -0500, Eric Sandeen wrote:
> On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Fix the return types of non-predicate bitmap functions to return the
> > usual negative error codes instead of the "moveon" boolean.
> 
> This seems much better, though how did you decide on negative
> error codes?  They are usual for the kernel, but in userspace
> we have kind of a mishmash, even in libfrog.
> 
>   File                 Function                 Line
> 0 libfrog/paths.c      fs_table_insert          176 error = ENOMEM;
> 1 libfrog/paths.c      fs_extract_mount_options 354 return ENOMEM;
> 2 libfrog/radix-tree.c radix_tree_extend        135 return -ENOMEM;
> 3 libfrog/radix-tree.c radix_tree_insert        188 return -ENOMEM;
> 4 libfrog/workqueue.c  workqueue_add            110 return ENOMEM;
> 
> 3 libfrog/paths.c         fs_table_initialise_mounts         384 return ENOENT;
> 4 libfrog/paths.c         fs_table_initialise_projects       489 error = ENOENT;
> 5 libfrog/paths.c         fs_table_insert_project_path       560 error = ENOENT;

Blindly copying libxfs style. :)

I see your point about being consistent within libfrog but OTOH it's
messy that we're not consistent across the various xfsprogs libraries.

Uhm.... I'll change it if you want.

--D

> 
> 
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  include/bitmap.h |    8 +++--
> >  libfrog/bitmap.c |   86 ++++++++++++++++++++++++++----------------------------
> >  repair/rmap.c    |   18 ++++++++---
> >  scrub/phase6.c   |   18 ++++-------
> >  4 files changed, 65 insertions(+), 65 deletions(-)
> > 
> > 
> > diff --git a/include/bitmap.h b/include/bitmap.h
> > index e29a4335..99a2fb23 100644
> > --- a/include/bitmap.h
> > +++ b/include/bitmap.h
> > @@ -11,11 +11,11 @@ struct bitmap {
> >  	struct avl64tree_desc	*bt_tree;
> >  };
> >  
> > -bool bitmap_init(struct bitmap **bmap);
> > +int bitmap_init(struct bitmap **bmap);
> >  void bitmap_free(struct bitmap **bmap);
> > -bool bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> > -bool bitmap_iterate(struct bitmap *bmap,
> > -		bool (*fn)(uint64_t, uint64_t, void *), void *arg);
> > +int bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> > +int bitmap_iterate(struct bitmap *bmap, int (*fn)(uint64_t, uint64_t, void *),
> > +		void *arg);
> >  bool bitmap_test(struct bitmap *bmap, uint64_t start,
> >  		uint64_t len);
> >  bool bitmap_empty(struct bitmap *bmap);
> > diff --git a/libfrog/bitmap.c b/libfrog/bitmap.c
> > index 450ebe0a..4dafc4c9 100644
> > --- a/libfrog/bitmap.c
> > +++ b/libfrog/bitmap.c
> > @@ -66,7 +66,7 @@ static struct avl64ops bitmap_ops = {
> >  };
> >  
> >  /* Initialize a bitmap. */
> > -bool
> > +int
> >  bitmap_init(
> >  	struct bitmap		**bmapp)
> >  {
> > @@ -74,18 +74,18 @@ bitmap_init(
> >  
> >  	bmap = calloc(1, sizeof(struct bitmap));
> >  	if (!bmap)
> > -		return false;
> > +		return -ENOMEM;
> >  	bmap->bt_tree = malloc(sizeof(struct avl64tree_desc));
> >  	if (!bmap->bt_tree) {
> >  		free(bmap);
> > -		return false;
> > +		return -ENOMEM;
> >  	}
> >  
> >  	pthread_mutex_init(&bmap->bt_lock, NULL);
> >  	avl64_init_tree(bmap->bt_tree, &bitmap_ops);
> >  	*bmapp = bmap;
> >  
> > -	return true;
> > +	return 0;
> >  }
> >  
> >  /* Free a bitmap. */
> > @@ -127,8 +127,31 @@ bitmap_node_init(
> >  	return ext;
> >  }
> >  
> > +/* Create a new bitmap node and insert it. */
> > +static inline int
> > +__bitmap_insert(
> > +	struct bitmap		*bmap,
> > +	uint64_t		start,
> > +	uint64_t		length)
> > +{
> > +	struct bitmap_node	*ext;
> > +	struct avl64node	*node;
> > +
> > +	ext = bitmap_node_init(start, length);
> > +	if (!ext)
> > +		return -ENOMEM;
> > +
> > +	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> > +	if (node == NULL) {
> > +		free(ext);
> > +		return -EEXIST;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /* Set a region of bits (locked). */
> > -static bool
> > +static int
> >  __bitmap_set(
> >  	struct bitmap		*bmap,
> >  	uint64_t		start,
> > @@ -142,28 +165,14 @@ __bitmap_set(
> >  	struct bitmap_node	*ext;
> >  	uint64_t		new_start;
> >  	uint64_t		new_length;
> > -	struct avl64node	*node;
> > -	bool			res = true;
> >  
> >  	/* Find any existing nodes adjacent or within that range. */
> >  	avl64_findranges(bmap->bt_tree, start - 1, start + length + 1,
> >  			&firstn, &lastn);
> >  
> >  	/* Nothing, just insert a new extent. */
> > -	if (firstn == NULL && lastn == NULL) {
> > -		ext = bitmap_node_init(start, length);
> > -		if (!ext)
> > -			return false;
> > -
> > -		node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> > -		if (node == NULL) {
> > -			free(ext);
> > -			errno = EEXIST;
> > -			return false;
> > -		}
> > -
> > -		return true;
> > -	}
> > +	if (firstn == NULL && lastn == NULL)
> > +		return __bitmap_insert(bmap, start, length);
> >  
> >  	assert(firstn != NULL && lastn != NULL);
> >  	new_start = start;
> > @@ -175,7 +184,7 @@ __bitmap_set(
> >  		/* Bail if the new extent is contained within an old one. */
> >  		if (ext->btn_start <= start &&
> >  		    ext->btn_start + ext->btn_length >= start + length)
> > -			return res;
> > +			return 0;
> >  
> >  		/* Check for overlapping and adjacent extents. */
> >  		if (ext->btn_start + ext->btn_length >= start ||
> > @@ -195,28 +204,17 @@ __bitmap_set(
> >  		}
> >  	}
> >  
> > -	ext = bitmap_node_init(new_start, new_length);
> > -	if (!ext)
> > -		return false;
> > -
> > -	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> > -	if (node == NULL) {
> > -		free(ext);
> > -		errno = EEXIST;
> > -		return false;
> > -	}
> > -
> > -	return res;
> > +	return __bitmap_insert(bmap, new_start, new_length);
> >  }
> >  
> >  /* Set a region of bits. */
> > -bool
> > +int
> >  bitmap_set(
> >  	struct bitmap		*bmap,
> >  	uint64_t		start,
> >  	uint64_t		length)
> >  {
> > -	bool			res;
> > +	int			res;
> >  
> >  	pthread_mutex_lock(&bmap->bt_lock);
> >  	res = __bitmap_set(bmap, start, length);
> > @@ -308,26 +306,26 @@ bitmap_clear(
> >  
> >  #ifdef DEBUG
> >  /* Iterate the set regions of this bitmap. */
> > -bool
> > +int
> >  bitmap_iterate(
> >  	struct bitmap		*bmap,
> > -	bool			(*fn)(uint64_t, uint64_t, void *),
> > +	int			(*fn)(uint64_t, uint64_t, void *),
> >  	void			*arg)
> >  {
> >  	struct avl64node	*node;
> >  	struct bitmap_node	*ext;
> > -	bool			moveon = true;
> > +	int			error = 0;
> >  
> >  	pthread_mutex_lock(&bmap->bt_lock);
> >  	avl_for_each(bmap->bt_tree, node) {
> >  		ext = container_of(node, struct bitmap_node, btn_node);
> > -		moveon = fn(ext->btn_start, ext->btn_length, arg);
> > -		if (!moveon)
> > +		error = fn(ext->btn_start, ext->btn_length, arg);
> > +		if (error)
> >  			break;
> >  	}
> >  	pthread_mutex_unlock(&bmap->bt_lock);
> >  
> > -	return moveon;
> > +	return error;
> >  }
> >  #endif
> >  
> > @@ -372,14 +370,14 @@ bitmap_empty(
> >  }
> >  
> >  #ifdef DEBUG
> > -static bool
> > +static int
> >  bitmap_dump_fn(
> >  	uint64_t		startblock,
> >  	uint64_t		blockcount,
> >  	void			*arg)
> >  {
> >  	printf("%"PRIu64":%"PRIu64"\n", startblock, blockcount);
> > -	return true;
> > +	return 0;
> >  }
> >  
> >  /* Dump bitmap. */
> > diff --git a/repair/rmap.c b/repair/rmap.c
> > index 19cceca3..47828a06 100644
> > --- a/repair/rmap.c
> > +++ b/repair/rmap.c
> > @@ -490,16 +490,22 @@ rmap_store_ag_btree_rec(
> >  	error = init_slab_cursor(ag_rmap->ar_raw_rmaps, rmap_compare, &rm_cur);
> >  	if (error)
> >  		goto err;
> > -	if (!bitmap_init(&own_ag_bitmap)) {
> > -		error = -ENOMEM;
> > +	error = -bitmap_init(&own_ag_bitmap);
> > +	if (error)
> >  		goto err_slab;
> > -	}
> >  	while ((rm_rec = pop_slab_cursor(rm_cur)) != NULL) {
> >  		if (rm_rec->rm_owner != XFS_RMAP_OWN_AG)
> >  			continue;
> > -		if (!bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> > -					rm_rec->rm_blockcount)) {
> > -			error = EFSCORRUPTED;
> > +		error = -bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> > +					rm_rec->rm_blockcount);
> > +		if (error) {
> > +			/*
> > +			 * If this range is already set, then the incore rmap
> > +			 * records for the AG free space btrees overlap and
> > +			 * we're toast because that is not allowed.
> > +			 */
> > +			if (error == EEXIST)
> > +				error = EFSCORRUPTED;
> >  			goto err_slab;
> >  		}
> >  	}
> > diff --git a/scrub/phase6.c b/scrub/phase6.c
> > index 4b25f3bb..66e6451c 100644
> > --- a/scrub/phase6.c
> > +++ b/scrub/phase6.c
> > @@ -341,7 +341,6 @@ xfs_check_rmap_ioerr(
> >  	struct media_verify_state	*vs = arg;
> >  	struct bitmap			*tree;
> >  	dev_t				dev;
> > -	bool				moveon;
> >  
> >  	dev = xfs_disk_to_dev(ctx, disk);
> >  
> > @@ -356,8 +355,8 @@ xfs_check_rmap_ioerr(
> >  	else
> >  		tree = NULL;
> >  	if (tree) {
> > -		moveon = bitmap_set(tree, start, length);
> > -		if (!moveon)
> > +		errno = -bitmap_set(tree, start, length);
> > +		if (errno)
> >  			str_errno(ctx, ctx->mntpoint);
> >  	}
> >  
> > @@ -454,16 +453,16 @@ xfs_scan_blocks(
> >  	struct scrub_ctx		*ctx)
> >  {
> >  	struct media_verify_state	vs = { NULL };
> > -	bool				moveon;
> > +	bool				moveon = false;
> >  
> > -	moveon = bitmap_init(&vs.d_bad);
> > -	if (!moveon) {
> > +	errno = -bitmap_init(&vs.d_bad);
> > +	if (errno) {
> >  		str_errno(ctx, ctx->mntpoint);
> >  		goto out;
> >  	}
> >  
> > -	moveon = bitmap_init(&vs.r_bad);
> > -	if (!moveon) {
> > +	errno = -bitmap_init(&vs.r_bad);
> > +	if (errno) {
> >  		str_errno(ctx, ctx->mntpoint);
> >  		goto out_dbad;
> >  	}
> > @@ -472,7 +471,6 @@ xfs_scan_blocks(
> >  			ctx->geo.blocksize, xfs_check_rmap_ioerr,
> >  			scrub_nproc(ctx));
> >  	if (!vs.rvp_data) {
> > -		moveon = false;
> >  		str_info(ctx, ctx->mntpoint,
> >  _("Could not create data device media verifier."));
> >  		goto out_rbad;
> > @@ -482,7 +480,6 @@ _("Could not create data device media verifier."));
> >  				ctx->geo.blocksize, xfs_check_rmap_ioerr,
> >  				scrub_nproc(ctx));
> >  		if (!vs.rvp_log) {
> > -			moveon = false;
> >  			str_info(ctx, ctx->mntpoint,
> >  	_("Could not create log device media verifier."));
> >  			goto out_datapool;
> > @@ -493,7 +490,6 @@ _("Could not create data device media verifier."));
> >  				ctx->geo.blocksize, xfs_check_rmap_ioerr,
> >  				scrub_nproc(ctx));
> >  		if (!vs.rvp_realtime) {
> > -			moveon = false;
> >  			str_info(ctx, ctx->mntpoint,
> >  	_("Could not create realtime device media verifier."));
> >  			goto out_logpool;
> > 

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Eric Sandeen]

On 5/21/19 12:01 PM, Darrick J. Wong wrote:
> On Tue, May 21, 2019 at 11:54:18AM -0500, Eric Sandeen wrote:
>> On 5/20/19 6:17 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Fix the return types of non-predicate bitmap functions to return the
>>> usual negative error codes instead of the "moveon" boolean.
>>
>> This seems much better, though how did you decide on negative
>> error codes?  They are usual for the kernel, but in userspace
>> we have kind of a mishmash, even in libfrog.
>>
>>   File                 Function                 Line
>> 0 libfrog/paths.c      fs_table_insert          176 error = ENOMEM;
>> 1 libfrog/paths.c      fs_extract_mount_options 354 return ENOMEM;
>> 2 libfrog/radix-tree.c radix_tree_extend        135 return -ENOMEM;
>> 3 libfrog/radix-tree.c radix_tree_insert        188 return -ENOMEM;
>> 4 libfrog/workqueue.c  workqueue_add            110 return ENOMEM;
>>
>> 3 libfrog/paths.c         fs_table_initialise_mounts         384 return ENOENT;
>> 4 libfrog/paths.c         fs_table_initialise_projects       489 error = ENOENT;
>> 5 libfrog/paths.c         fs_table_insert_project_path       560 error = ENOENT;
> 
> Blindly copying libxfs style. :)
> 
> I see your point about being consistent within libfrog but OTOH it's
> messy that we're not consistent across the various xfsprogs libraries.
> 
> Uhm.... I'll change it if you want.

If I were king (am I king?) I'd say that kernel code does negative values,
userspace does positive, and we handle differences at the interface.

Mostly I just want it to be predictable and consistent...

AFAICT everything in libfrog is positive except for the radix-tree stuff,
which is nominally kernel-ish ...?

Userspace clearly needs some cleanups; mkfs parsers return negative, everything
else seems positive; a couple metadump functions return negative, the rest is
positive; most userspace callers of libxfs negate the negative error return...

So yeah I'm of the opinion that unless it's kernel(-ish?) code it should be
positive, and I can send a patch to clean up stuff that's not.

I can be swayed by counterarguments if you have them.  :)

-Eric

Re: [PATCH 08/12] xfs_repair: refactor namecheck functions

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that we have name check functions in libxfs, use them instead of our
> custom versions.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I dig it.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  libxfs/libxfs_api_defs.h |    2 ++
>  repair/attr_repair.c     |   32 +++++++++++++-------------------
>  repair/da_util.c         |   27 ---------------------------
>  repair/da_util.h         |    6 ------
>  repair/dir2.c            |   12 ++----------
>  5 files changed, 17 insertions(+), 62 deletions(-)
> 
> 
> diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
> index 34bb552d..71a7ef53 100644
> --- a/libxfs/libxfs_api_defs.h
> +++ b/libxfs/libxfs_api_defs.h
> @@ -143,6 +143,8 @@
>  #define xfs_default_ifork_ops		libxfs_default_ifork_ops
>  #define xfs_fs_geometry			libxfs_fs_geometry
>  #define xfs_init_local_fork		libxfs_init_local_fork
> +#define xfs_dir2_namecheck		libxfs_dir2_namecheck
> +#define xfs_attr_namecheck		libxfs_attr_namecheck
>  
>  #define LIBXFS_ATTR_ROOT		ATTR_ROOT
>  #define LIBXFS_ATTR_SECURE		ATTR_SECURE
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index 5ad81c09..9a44f610 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -122,14 +122,6 @@ set_da_freemap(xfs_mount_t *mp, da_freemap_t *map, int start, int stop)
>   * fork being emptied and put in shortform format.
>   */
>  
> -static int
> -attr_namecheck(
> -	uint8_t	*name,
> -	int	length)
> -{
> -	return namecheck((char *)name, length, false);
> -}
> -
>  /*
>   * This routine just checks what security needs are for attribute values
>   * only called when root flag is set, otherwise these names could exist in
> @@ -301,8 +293,8 @@ process_shortform_attr(
>  		}
>  
>  		/* namecheck checks for null chars in attr names. */
> -		if (attr_namecheck(currententry->nameval,
> -						currententry->namelen)) {
> +		if (!libxfs_attr_namecheck(currententry->nameval,
> +					   currententry->namelen)) {
>  			do_warn(
>  	_("entry contains illegal character in shortform attribute name\n"));
>  			junkit = 1;
> @@ -464,8 +456,9 @@ process_leaf_attr_local(
>  	xfs_attr_leaf_name_local_t *local;
>  
>  	local = xfs_attr3_leaf_name_local(leaf, i);
> -	if (local->namelen == 0 || attr_namecheck(local->nameval,
> -							local->namelen)) {
> +	if (local->namelen == 0 ||
> +	    !libxfs_attr_namecheck(local->nameval,
> +				   local->namelen)) {
>  		do_warn(
>  	_("attribute entry %d in attr block %u, inode %" PRIu64 " has bad name (namelen = %d)\n"),
>  			i, da_bno, ino, local->namelen);
> @@ -519,13 +512,14 @@ process_leaf_attr_remote(
>  
>  	remotep = xfs_attr3_leaf_name_remote(leaf, i);
>  
> -	if (remotep->namelen == 0 || attr_namecheck(remotep->name,
> -						remotep->namelen) ||
> -			be32_to_cpu(entry->hashval) !=
> -				libxfs_da_hashname((unsigned char *)&remotep->name[0],
> -						remotep->namelen) ||
> -			be32_to_cpu(entry->hashval) < last_hashval ||
> -			be32_to_cpu(remotep->valueblk) == 0) {
> +	if (remotep->namelen == 0 ||
> +	    !libxfs_attr_namecheck(remotep->name,
> +				   remotep->namelen) ||
> +	    be32_to_cpu(entry->hashval) !=
> +			libxfs_da_hashname((unsigned char *)&remotep->name[0],
> +					   remotep->namelen) ||
> +	    be32_to_cpu(entry->hashval) < last_hashval ||
> +	    be32_to_cpu(remotep->valueblk) == 0) {
>  		do_warn(
>  	_("inconsistent remote attribute entry %d in attr block %u, ino %" PRIu64 "\n"), i, da_bno, ino);
>  		return -1;
> diff --git a/repair/da_util.c b/repair/da_util.c
> index 4a258e58..8c818ea1 100644
> --- a/repair/da_util.c
> +++ b/repair/da_util.c
> @@ -12,33 +12,6 @@
>  #include "bmap.h"
>  #include "da_util.h"
>  
> -/*
> - * Takes a name and length (name need not be null-terminated) and whether
> - * we are checking a dir (as opposed to an attr).
> - * Returns 1 if the name contains a NUL or if a directory entry contains a '/'.
> - * Returns 0 if the name checks out.
> - */
> -int
> -namecheck(
> -	char	*name,
> -	int	length,
> -	bool	isadir)
> -{
> -	char	*c;
> -	int	i;
> -
> -	ASSERT(length < MAXNAMELEN);
> -
> -	for (c = name, i = 0; i < length; i++, c++) {
> -		if (isadir && *c == '/')
> -			return 1;
> -		if (*c == '\0')
> -			return 1;
> -	}
> -
> -	return 0;
> -}
> -
>  /*
>   * the cursor gets passed up and down the da btree processing
>   * routines.  The interior block processing routines use the
> diff --git a/repair/da_util.h b/repair/da_util.h
> index 041dff74..90fec00c 100644
> --- a/repair/da_util.h
> +++ b/repair/da_util.h
> @@ -24,12 +24,6 @@ typedef struct da_bt_cursor {
>  	struct blkmap		*blkmap;
>  } da_bt_cursor_t;
>  
> -int
> -namecheck(
> -	char		*name,
> -	int		length,
> -	bool		isadir);
> -
>  struct xfs_buf *
>  da_read_buf(
>  	xfs_mount_t	*mp,
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 094ecb3d..4ac0084e 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -44,14 +44,6 @@ _("malloc failed (%zu bytes) dir2_add_badlist:ino %" PRIu64 "\n"),
>  	l->ino = ino;
>  }
>  
> -static int
> -dir_namecheck(
> -	uint8_t	*name,
> -	int	length)
> -{
> -	return namecheck((char *)name, length, true);
> -}
> -
>  int
>  dir2_is_badino(
>  	xfs_ino_t	ino)
> @@ -318,7 +310,7 @@ _("entry #%d %s in shortform dir %" PRIu64),
>  		 * the length value is stored in a byte
>  		 * so it can't be too big, it can only wrap
>  		 */
> -		if (dir_namecheck(sfep->name, namelen)) {
> +		if (!libxfs_dir2_namecheck(sfep->name, namelen)) {
>  			/*
>  			 * junk entry
>  			 */
> @@ -789,7 +781,7 @@ _("\twould clear inode number in entry at offset %" PRIdPTR "...\n"),
>  		 * during phase 4.
>  		 */
>  		junkit = dep->name[0] == '/';
> -		nm_illegal = dir_namecheck(dep->name, dep->namelen);
> +		nm_illegal = !libxfs_dir2_namecheck(dep->name, dep->namelen);
>  		if (ino_discovery && nm_illegal) {
>  			do_warn(
>  _("entry at block %u offset %" PRIdPTR " in directory inode %" PRIu64 " has illegal name \"%*.*s\": "),
> 

Re: [PATCH 09/12] xfs_scrub: fix background-mode sleep throttling

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The comment preceding background_sleep() is wrong -- the function sleeps
> 100us, not 100ms, for every '-b' passed in after the first one.  This is
> really not obvious from the magic numbers, so fix the comment and use
> symbolic constants for easier reading.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Ok, looks like man page was already correct.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  scrub/common.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> 
> diff --git a/scrub/common.c b/scrub/common.c
> index c877c7c8..1cd2b7ba 100644
> --- a/scrub/common.c
> +++ b/scrub/common.c
> @@ -253,21 +253,23 @@ scrub_nproc_workqueue(
>  }
>  
>  /*
> - * Sleep for 100ms * however many -b we got past the initial one.
> + * Sleep for 100us * however many -b we got past the initial one.
>   * This is an (albeit clumsy) way to throttle scrub activity.
>   */
> +#define NSEC_PER_SEC	1000000000ULL
> +#define NSEC_PER_USEC	1000ULL
>  void
>  background_sleep(void)
>  {
> -	unsigned long long	time;
> +	unsigned long long	time_ns;
>  	struct timespec		tv;
>  
>  	if (bg_mode < 2)
>  		return;
>  
> -	time = 100000ULL * (bg_mode - 1);
> -	tv.tv_sec = time / 1000000;
> -	tv.tv_nsec = time % 1000000;
> +	time_ns =  100 * NSEC_PER_USEC * (bg_mode - 1);
> +	tv.tv_sec = time_ns / NSEC_PER_SEC;
> +	tv.tv_nsec = time_ns % NSEC_PER_SEC;
>  	nanosleep(&tv, NULL);
>  }
>  
> 

Re: [PATCH 10/12] mkfs: allow setting dax flag on root directory

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Teach mkfs to set the DAX flag on the root directory so that all new
> files can be created in dax mode.  This is a complement to removing the
> mount option.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  man/man8/mkfs.xfs.8 |   11 +++++++++++
>  mkfs/xfs_mkfs.c     |   11 +++++++++++
>  2 files changed, 22 insertions(+)

I'll tuck this away for 2024 when we decide what to do about dax.

-Eric

> 
> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
> index 4b8c78c3..0137f164 100644
> --- a/man/man8/mkfs.xfs.8
> +++ b/man/man8/mkfs.xfs.8
> @@ -391,6 +391,17 @@ All inodes created by
>  will have this extent size hint applied.
>  The value must be provided in units of filesystem blocks.
>  Directories will pass on this hint to newly created children.
> +.TP
> +.BI dax= value
> +All inodes created by
> +.B mkfs.xfs
> +will have the DAX flag set.
> +This means that directories will pass the flag on to newly created files
> +and files will use the DAX IO paths when possible.
> +This value is either 1 to enable the use or 0 to disable.
> +By default,
> +.B mkfs.xfs
> +will not enable DAX mode.
>  .RE
>  .TP
>  .B \-f
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 09106648..5b66074d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -59,6 +59,7 @@ enum {
>  	D_PROJINHERIT,
>  	D_EXTSZINHERIT,
>  	D_COWEXTSIZE,
> +	D_DAX,
>  	D_MAX_OPTS,
>  };
>  
> @@ -253,6 +254,7 @@ static struct opt_params dopts = {
>  		[D_PROJINHERIT] = "projinherit",
>  		[D_EXTSZINHERIT] = "extszinherit",
>  		[D_COWEXTSIZE] = "cowextsize",
> +		[D_DAX] = "dax",
>  	},
>  	.subopt_params = {
>  		{ .index = D_AGCOUNT,
> @@ -368,6 +370,12 @@ static struct opt_params dopts = {
>  		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
> +		{ .index = D_DAX,
> +		  .conflicts = { { NULL, LAST_CONFLICT } },
> +		  .minval = 0,
> +		  .maxval = 1,
> +		  .defaultval = 1,
> +		},
>  	},
>  };
>  
> @@ -1465,6 +1473,9 @@ data_opts_parser(
>  		cli->fsx.fsx_cowextsize = getnum(value, opts, subopt);
>  		cli->fsx.fsx_xflags |= FS_XFLAG_COWEXTSIZE;
>  		break;
> +	case D_DAX:
> +		cli->fsx.fsx_xflags |= FS_XFLAG_DAX;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> 

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Christoph Hellwig]

On Tue, May 21, 2019 at 01:59:58PM -0500, Eric Sandeen wrote:
> So yeah I'm of the opinion that unless it's kernel(-ish?) code it should be
> positive, and I can send a patch to clean up stuff that's not.
> 
> I can be swayed by counterarguments if you have them.  :)

What speaks against everything is negative?  It isn't like returning
positive errors really is a traditional userspace convention, as that
is return -1 (negative!) and look at errno..

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Eric Sandeen]

On 5/21/19 2:19 PM, Christoph Hellwig wrote:
> On Tue, May 21, 2019 at 01:59:58PM -0500, Eric Sandeen wrote:
>> So yeah I'm of the opinion that unless it's kernel(-ish?) code it should be
>> positive, and I can send a patch to clean up stuff that's not.
>>
>> I can be swayed by counterarguments if you have them.  :)
> 
> What speaks against everything is negative?  It isn't like returning
> positive errors really is a traditional userspace convention, as that
> is return -1 (negative!) and look at errno..

Sorry, I wasn't clear - I meant returning negative errnos.  That's
the part that's not consistent.

-Eric

Re: [PATCH 11/12] mkfs: validate start and end of aligned logs

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Validate that the start and end of the log stay within a single AG if
> we adjust either end to align to stripe units.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  mkfs/xfs_mkfs.c |   15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 5b66074d..8f84536e 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3044,15 +3044,28 @@ align_internal_log(
>  	struct xfs_mount	*mp,
>  	int			sunit)
>  {
> +	uint64_t		logend;
> +
>  	/* round up log start if necessary */
>  	if ((cfg->logstart % sunit) != 0)
>  		cfg->logstart = ((cfg->logstart + (sunit - 1)) / sunit) * sunit;
>  
> +	/* if our log start rounds into the next AG we're done */

/* If our log start overlaps the next AG's metadata, fail */

> +	if (!xfs_verify_fsbno(mp, cfg->logstart)) {
> +			fprintf(stderr,
> +_("Due to stripe alignment, the internal log start (%lld) cannot be aligned\n"
> +  "within an allocation group.\n"),
> +			(long long) cfg->logstart);

Hm, should it suggest what should be modified to try again ...?

> +		usage();
> +	}
> +
>  	/* round up/down the log size now */
>  	align_log_size(cfg, sunit);
>  
>  	/* check the aligned log still fits in an AG. */
> -	if (cfg->logblocks > cfg->agsize - XFS_FSB_TO_AGBNO(mp, cfg->logstart)) {
> +	logend = cfg->logstart + cfg->logblocks - 1;
> +	if (XFS_FSB_TO_AGNO(mp, cfg->logstart) != XFS_FSB_TO_AGNO(mp, logend) ||
> +	    !xfs_verify_fsbno(mp, logend)) {

this xfs_verify_fsbno is probably redundant but can't hurt?

-Eric

>  		fprintf(stderr,
>  _("Due to stripe alignment, the internal log size (%lld) is too large.\n"
>    "Must fit within an allocation group.\n"),
> 

Re: [PATCH 12/12] mkfs: enable reflink by default

[Eric Sandeen]

On 5/20/19 6:18 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Data block sharing (a.k.a. reflink) has been stable for a while, so turn
> it on by default.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I'm cool with this.  :)  But I think we need a manpage update too.
I also had some comment updates - I'll send a reply as a patch.

-Eric

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Christoph Hellwig]

On Tue, May 21, 2019 at 02:20:51PM -0500, Eric Sandeen wrote:
> > On Tue, May 21, 2019 at 01:59:58PM -0500, Eric Sandeen wrote:
> >> So yeah I'm of the opinion that unless it's kernel(-ish?) code it should be
> >> positive, and I can send a patch to clean up stuff that's not.
> >>
> >> I can be swayed by counterarguments if you have them.  :)
> > 
> > What speaks against everything is negative?  It isn't like returning
> > positive errors really is a traditional userspace convention, as that
> > is return -1 (negative!) and look at errno..
> 
> Sorry, I wasn't clear - I meant returning negative errnos.  That's
> the part that's not consistent.

Yes.  And for libxfs/libfrog/etc stuff I think sticking to that and
always returning negative error values sounds sanest to me.  Note that
some userspace libraries have adopted that calling convention, for
example libaio.

[PATCH 12/12 V2] mkfs: enable reflink by default

[Eric Sandeen]

From: Darrick J. Wong <darrick.wong@oracle.com>

Data block sharing (a.k.a. reflink) has been stable for a while, so turn
it on by default.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
[sandeen: update comments & man page]
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
index 4b8c78c..78b1501 100644
--- a/man/man8/mkfs.xfs.8
+++ b/man/man8/mkfs.xfs.8
@@ -231,7 +231,7 @@ available for the data forks of regular files.
 .IP
 By default,
 .B mkfs.xfs
-will not create reference count btrees and therefore will not enable the
+will create reference count btrees and therefore will enable the
 reflink feature.  This feature is only available for filesystems created with
 the (default)
 .B \-m crc=1
@@ -239,6 +239,13 @@ option set. When the option
 .B \-m crc=0
 is used, the reference count btree feature is not supported and reflink is
 disabled.
+.IP
+Note: the filesystem DAX mount option (
+.B \-o dax
+) is incompatible with
+reflink-enabled XFS filesystems.  To use filesystem DAX with XFS, specify the
+.B \-m reflink=0
+option to mkfs.xfs to disable the reflink feature.
 .RE
 .TP
 .BI \-d " data_section_options"
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 0910664..ddb25ec 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1973,15 +1973,15 @@ _("Directory ftype field always enabled on CRC enabled filesystems\n"));
 			usage();
 		}
 
-	} else {
+	} else {	/* !crcs_enabled */
 		/*
-		 * The kernel doesn't currently support crc=0,finobt=1
-		 * filesystems. If crcs are not enabled and the user has not
-		 * explicitly turned finobt on, then silently turn it off to
-		 * avoid an unnecessary warning.
+		 * The kernel doesn't support crc=0,finobt=1 filesystems.
+		 * If crcs are not enabled and the user has not explicitly
+		 * turned finobt on, then silently turn it off to avoid an
+		 * unnecessary warning.
 		 * If the user explicitly tried to use crc=0,finobt=1,
 		 * then issue an error.
-		 * The same is also for sparse inodes.
+		 * The same is also true for sparse inodes and reflink.
 		 */
 		if (cli->sb_feat.finobt && cli_opt_set(&mopts, M_FINOBT)) {
 			fprintf(stderr,
@@ -2004,7 +2004,7 @@ _("rmapbt not supported without CRC support\n"));
 		}
 		cli->sb_feat.rmapbt = false;
 
-		if (cli->sb_feat.reflink) {
+		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
 			fprintf(stderr,
 _("reflink not supported without CRC support\n"));
 			usage();
@@ -3876,7 +3876,7 @@ main(
 			.finobt = true,
 			.spinodes = true,
 			.rmapbt = false,
-			.reflink = false,
+			.reflink = true,
 			.parent_pointers = false,
 			.nodalign = false,
 			.nortalign = false,

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Eric Sandeen]

On 5/21/19 2:28 PM, Christoph Hellwig wrote:
> On Tue, May 21, 2019 at 02:20:51PM -0500, Eric Sandeen wrote:
>>> On Tue, May 21, 2019 at 01:59:58PM -0500, Eric Sandeen wrote:
>>>> So yeah I'm of the opinion that unless it's kernel(-ish?) code it should be
>>>> positive, and I can send a patch to clean up stuff that's not.
>>>>
>>>> I can be swayed by counterarguments if you have them.  :)
>>>
>>> What speaks against everything is negative?  It isn't like returning
>>> positive errors really is a traditional userspace convention, as that
>>> is return -1 (negative!) and look at errno..
>>
>> Sorry, I wasn't clear - I meant returning negative errnos.  That's
>> the part that's not consistent.
> 
> Yes.  And for libxfs/libfrog/etc stuff I think sticking to that and
> always returning negative error values sounds sanest to me.  Note that
> some userspace libraries have adopted that calling convention, for
> example libaio.
> 

Oh, I see what you mean.  *shrug* sure, that makes sense.

Sorry, it's my weird old SGI/Irix/ancient-xfs history confusing me.  ;)

-Eric

Re: [PATCH 07/12] libfrog: fix bitmap return values

[Eric Sandeen]

On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Fix the return types of non-predicate bitmap functions to return the
> usual negative error codes instead of the "moveon" boolean.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  include/bitmap.h |    8 +++--
>  libfrog/bitmap.c |   86 ++++++++++++++++++++++++++----------------------------
>  repair/rmap.c    |   18 ++++++++---
>  scrub/phase6.c   |   18 ++++-------
>  4 files changed, 65 insertions(+), 65 deletions(-)

Sorry for the drama on signs.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> 
> diff --git a/include/bitmap.h b/include/bitmap.h
> index e29a4335..99a2fb23 100644
> --- a/include/bitmap.h
> +++ b/include/bitmap.h
> @@ -11,11 +11,11 @@ struct bitmap {
>  	struct avl64tree_desc	*bt_tree;
>  };
>  
> -bool bitmap_init(struct bitmap **bmap);
> +int bitmap_init(struct bitmap **bmap);
>  void bitmap_free(struct bitmap **bmap);
> -bool bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> -bool bitmap_iterate(struct bitmap *bmap,
> -		bool (*fn)(uint64_t, uint64_t, void *), void *arg);
> +int bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> +int bitmap_iterate(struct bitmap *bmap, int (*fn)(uint64_t, uint64_t, void *),
> +		void *arg);
>  bool bitmap_test(struct bitmap *bmap, uint64_t start,
>  		uint64_t len);
>  bool bitmap_empty(struct bitmap *bmap);
> diff --git a/libfrog/bitmap.c b/libfrog/bitmap.c
> index 450ebe0a..4dafc4c9 100644
> --- a/libfrog/bitmap.c
> +++ b/libfrog/bitmap.c
> @@ -66,7 +66,7 @@ static struct avl64ops bitmap_ops = {
>  };
>  
>  /* Initialize a bitmap. */
> -bool
> +int
>  bitmap_init(
>  	struct bitmap		**bmapp)
>  {
> @@ -74,18 +74,18 @@ bitmap_init(
>  
>  	bmap = calloc(1, sizeof(struct bitmap));
>  	if (!bmap)
> -		return false;
> +		return -ENOMEM;
>  	bmap->bt_tree = malloc(sizeof(struct avl64tree_desc));
>  	if (!bmap->bt_tree) {
>  		free(bmap);
> -		return false;
> +		return -ENOMEM;
>  	}
>  
>  	pthread_mutex_init(&bmap->bt_lock, NULL);
>  	avl64_init_tree(bmap->bt_tree, &bitmap_ops);
>  	*bmapp = bmap;
>  
> -	return true;
> +	return 0;
>  }
>  
>  /* Free a bitmap. */
> @@ -127,8 +127,31 @@ bitmap_node_init(
>  	return ext;
>  }
>  
> +/* Create a new bitmap node and insert it. */
> +static inline int
> +__bitmap_insert(
> +	struct bitmap		*bmap,
> +	uint64_t		start,
> +	uint64_t		length)
> +{
> +	struct bitmap_node	*ext;
> +	struct avl64node	*node;
> +
> +	ext = bitmap_node_init(start, length);
> +	if (!ext)
> +		return -ENOMEM;
> +
> +	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> +	if (node == NULL) {
> +		free(ext);
> +		return -EEXIST;
> +	}
> +
> +	return 0;
> +}
> +
>  /* Set a region of bits (locked). */
> -static bool
> +static int
>  __bitmap_set(
>  	struct bitmap		*bmap,
>  	uint64_t		start,
> @@ -142,28 +165,14 @@ __bitmap_set(
>  	struct bitmap_node	*ext;
>  	uint64_t		new_start;
>  	uint64_t		new_length;
> -	struct avl64node	*node;
> -	bool			res = true;
>  
>  	/* Find any existing nodes adjacent or within that range. */
>  	avl64_findranges(bmap->bt_tree, start - 1, start + length + 1,
>  			&firstn, &lastn);
>  
>  	/* Nothing, just insert a new extent. */
> -	if (firstn == NULL && lastn == NULL) {
> -		ext = bitmap_node_init(start, length);
> -		if (!ext)
> -			return false;
> -
> -		node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> -		if (node == NULL) {
> -			free(ext);
> -			errno = EEXIST;
> -			return false;
> -		}
> -
> -		return true;
> -	}
> +	if (firstn == NULL && lastn == NULL)
> +		return __bitmap_insert(bmap, start, length);
>  
>  	assert(firstn != NULL && lastn != NULL);
>  	new_start = start;
> @@ -175,7 +184,7 @@ __bitmap_set(
>  		/* Bail if the new extent is contained within an old one. */
>  		if (ext->btn_start <= start &&
>  		    ext->btn_start + ext->btn_length >= start + length)
> -			return res;
> +			return 0;
>  
>  		/* Check for overlapping and adjacent extents. */
>  		if (ext->btn_start + ext->btn_length >= start ||
> @@ -195,28 +204,17 @@ __bitmap_set(
>  		}
>  	}
>  
> -	ext = bitmap_node_init(new_start, new_length);
> -	if (!ext)
> -		return false;
> -
> -	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> -	if (node == NULL) {
> -		free(ext);
> -		errno = EEXIST;
> -		return false;
> -	}
> -
> -	return res;
> +	return __bitmap_insert(bmap, new_start, new_length);
>  }
>  
>  /* Set a region of bits. */
> -bool
> +int
>  bitmap_set(
>  	struct bitmap		*bmap,
>  	uint64_t		start,
>  	uint64_t		length)
>  {
> -	bool			res;
> +	int			res;
>  
>  	pthread_mutex_lock(&bmap->bt_lock);
>  	res = __bitmap_set(bmap, start, length);
> @@ -308,26 +306,26 @@ bitmap_clear(
>  
>  #ifdef DEBUG
>  /* Iterate the set regions of this bitmap. */
> -bool
> +int
>  bitmap_iterate(
>  	struct bitmap		*bmap,
> -	bool			(*fn)(uint64_t, uint64_t, void *),
> +	int			(*fn)(uint64_t, uint64_t, void *),
>  	void			*arg)
>  {
>  	struct avl64node	*node;
>  	struct bitmap_node	*ext;
> -	bool			moveon = true;
> +	int			error = 0;
>  
>  	pthread_mutex_lock(&bmap->bt_lock);
>  	avl_for_each(bmap->bt_tree, node) {
>  		ext = container_of(node, struct bitmap_node, btn_node);
> -		moveon = fn(ext->btn_start, ext->btn_length, arg);
> -		if (!moveon)
> +		error = fn(ext->btn_start, ext->btn_length, arg);
> +		if (error)
>  			break;
>  	}
>  	pthread_mutex_unlock(&bmap->bt_lock);
>  
> -	return moveon;
> +	return error;
>  }
>  #endif
>  
> @@ -372,14 +370,14 @@ bitmap_empty(
>  }
>  
>  #ifdef DEBUG
> -static bool
> +static int
>  bitmap_dump_fn(
>  	uint64_t		startblock,
>  	uint64_t		blockcount,
>  	void			*arg)
>  {
>  	printf("%"PRIu64":%"PRIu64"\n", startblock, blockcount);
> -	return true;
> +	return 0;
>  }
>  
>  /* Dump bitmap. */
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 19cceca3..47828a06 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -490,16 +490,22 @@ rmap_store_ag_btree_rec(
>  	error = init_slab_cursor(ag_rmap->ar_raw_rmaps, rmap_compare, &rm_cur);
>  	if (error)
>  		goto err;
> -	if (!bitmap_init(&own_ag_bitmap)) {
> -		error = -ENOMEM;
> +	error = -bitmap_init(&own_ag_bitmap);
> +	if (error)
>  		goto err_slab;
> -	}
>  	while ((rm_rec = pop_slab_cursor(rm_cur)) != NULL) {
>  		if (rm_rec->rm_owner != XFS_RMAP_OWN_AG)
>  			continue;
> -		if (!bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> -					rm_rec->rm_blockcount)) {
> -			error = EFSCORRUPTED;
> +		error = -bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> +					rm_rec->rm_blockcount);
> +		if (error) {
> +			/*
> +			 * If this range is already set, then the incore rmap
> +			 * records for the AG free space btrees overlap and
> +			 * we're toast because that is not allowed.
> +			 */
> +			if (error == EEXIST)
> +				error = EFSCORRUPTED;
>  			goto err_slab;
>  		}
>  	}
> diff --git a/scrub/phase6.c b/scrub/phase6.c
> index 4b25f3bb..66e6451c 100644
> --- a/scrub/phase6.c
> +++ b/scrub/phase6.c
> @@ -341,7 +341,6 @@ xfs_check_rmap_ioerr(
>  	struct media_verify_state	*vs = arg;
>  	struct bitmap			*tree;
>  	dev_t				dev;
> -	bool				moveon;
>  
>  	dev = xfs_disk_to_dev(ctx, disk);
>  
> @@ -356,8 +355,8 @@ xfs_check_rmap_ioerr(
>  	else
>  		tree = NULL;
>  	if (tree) {
> -		moveon = bitmap_set(tree, start, length);
> -		if (!moveon)
> +		errno = -bitmap_set(tree, start, length);
> +		if (errno)
>  			str_errno(ctx, ctx->mntpoint);
>  	}
>  
> @@ -454,16 +453,16 @@ xfs_scan_blocks(
>  	struct scrub_ctx		*ctx)
>  {
>  	struct media_verify_state	vs = { NULL };
> -	bool				moveon;
> +	bool				moveon = false;
>  
> -	moveon = bitmap_init(&vs.d_bad);
> -	if (!moveon) {
> +	errno = -bitmap_init(&vs.d_bad);
> +	if (errno) {
>  		str_errno(ctx, ctx->mntpoint);
>  		goto out;
>  	}
>  
> -	moveon = bitmap_init(&vs.r_bad);
> -	if (!moveon) {
> +	errno = -bitmap_init(&vs.r_bad);
> +	if (errno) {
>  		str_errno(ctx, ctx->mntpoint);
>  		goto out_dbad;
>  	}
> @@ -472,7 +471,6 @@ xfs_scan_blocks(
>  			ctx->geo.blocksize, xfs_check_rmap_ioerr,
>  			scrub_nproc(ctx));
>  	if (!vs.rvp_data) {
> -		moveon = false;
>  		str_info(ctx, ctx->mntpoint,
>  _("Could not create data device media verifier."));
>  		goto out_rbad;
> @@ -482,7 +480,6 @@ _("Could not create data device media verifier."));
>  				ctx->geo.blocksize, xfs_check_rmap_ioerr,
>  				scrub_nproc(ctx));
>  		if (!vs.rvp_log) {
> -			moveon = false;
>  			str_info(ctx, ctx->mntpoint,
>  	_("Could not create log device media verifier."));
>  			goto out_datapool;
> @@ -493,7 +490,6 @@ _("Could not create data device media verifier."));
>  				ctx->geo.blocksize, xfs_check_rmap_ioerr,
>  				scrub_nproc(ctx));
>  		if (!vs.rvp_realtime) {
> -			moveon = false;
>  			str_info(ctx, ctx->mntpoint,
>  	_("Could not create realtime device media verifier."));
>  			goto out_logpool;
> 

Re: [PATCH 11/12] mkfs: validate start and end of aligned logs

[Darrick J. Wong]

On Tue, May 21, 2019 at 02:24:40PM -0500, Eric Sandeen wrote:
> On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Validate that the start and end of the log stay within a single AG if
> > we adjust either end to align to stripe units.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  mkfs/xfs_mkfs.c |   15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 5b66074d..8f84536e 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -3044,15 +3044,28 @@ align_internal_log(
> >  	struct xfs_mount	*mp,
> >  	int			sunit)
> >  {
> > +	uint64_t		logend;
> > +
> >  	/* round up log start if necessary */
> >  	if ((cfg->logstart % sunit) != 0)
> >  		cfg->logstart = ((cfg->logstart + (sunit - 1)) / sunit) * sunit;
> >  
> > +	/* if our log start rounds into the next AG we're done */
> 
> /* If our log start overlaps the next AG's metadata, fail */

Ok.

> > +	if (!xfs_verify_fsbno(mp, cfg->logstart)) {
> > +			fprintf(stderr,
> > +_("Due to stripe alignment, the internal log start (%lld) cannot be aligned\n"
> > +  "within an allocation group.\n"),
> > +			(long long) cfg->logstart);
> 
> Hm, should it suggest what should be modified to try again ...?

But what should be modified, exactly?  -d su=0,sw=0?

> > +		usage();
> > +	}
> > +
> >  	/* round up/down the log size now */
> >  	align_log_size(cfg, sunit);
> >  
> >  	/* check the aligned log still fits in an AG. */
> > -	if (cfg->logblocks > cfg->agsize - XFS_FSB_TO_AGBNO(mp, cfg->logstart)) {
> > +	logend = cfg->logstart + cfg->logblocks - 1;
> > +	if (XFS_FSB_TO_AGNO(mp, cfg->logstart) != XFS_FSB_TO_AGNO(mp, logend) ||
> > +	    !xfs_verify_fsbno(mp, logend)) {
> 
> this xfs_verify_fsbno is probably redundant but can't hurt?

<nod>

Will respin patch.

--D

> -Eric
> 
> >  		fprintf(stderr,
> >  _("Due to stripe alignment, the internal log size (%lld) is too large.\n"
> >    "Must fit within an allocation group.\n"),
> > 

Re: [PATCH 12/12 V2] mkfs: enable reflink by default

[Darrick J. Wong]

On Tue, May 21, 2019 at 02:30:57PM -0500, Eric Sandeen wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Data block sharing (a.k.a. reflink) has been stable for a while, so turn
> it on by default.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> [sandeen: update comments & man page]
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good to me.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Hmm I'm not allowed to RVB my own patch, right?

--D

> ---
> 
> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
> index 4b8c78c..78b1501 100644
> --- a/man/man8/mkfs.xfs.8
> +++ b/man/man8/mkfs.xfs.8
> @@ -231,7 +231,7 @@ available for the data forks of regular files.
>  .IP
>  By default,
>  .B mkfs.xfs
> -will not create reference count btrees and therefore will not enable the
> +will create reference count btrees and therefore will enable the
>  reflink feature.  This feature is only available for filesystems created with
>  the (default)
>  .B \-m crc=1
> @@ -239,6 +239,13 @@ option set. When the option
>  .B \-m crc=0
>  is used, the reference count btree feature is not supported and reflink is
>  disabled.
> +.IP
> +Note: the filesystem DAX mount option (
> +.B \-o dax
> +) is incompatible with
> +reflink-enabled XFS filesystems.  To use filesystem DAX with XFS, specify the
> +.B \-m reflink=0
> +option to mkfs.xfs to disable the reflink feature.
>  .RE
>  .TP
>  .BI \-d " data_section_options"
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 0910664..ddb25ec 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1973,15 +1973,15 @@ _("Directory ftype field always enabled on CRC enabled filesystems\n"));
>  			usage();
>  		}
>  
> -	} else {
> +	} else {	/* !crcs_enabled */
>  		/*
> -		 * The kernel doesn't currently support crc=0,finobt=1
> -		 * filesystems. If crcs are not enabled and the user has not
> -		 * explicitly turned finobt on, then silently turn it off to
> -		 * avoid an unnecessary warning.
> +		 * The kernel doesn't support crc=0,finobt=1 filesystems.
> +		 * If crcs are not enabled and the user has not explicitly
> +		 * turned finobt on, then silently turn it off to avoid an
> +		 * unnecessary warning.
>  		 * If the user explicitly tried to use crc=0,finobt=1,
>  		 * then issue an error.
> -		 * The same is also for sparse inodes.
> +		 * The same is also true for sparse inodes and reflink.
>  		 */
>  		if (cli->sb_feat.finobt && cli_opt_set(&mopts, M_FINOBT)) {
>  			fprintf(stderr,
> @@ -2004,7 +2004,7 @@ _("rmapbt not supported without CRC support\n"));
>  		}
>  		cli->sb_feat.rmapbt = false;
>  
> -		if (cli->sb_feat.reflink) {
> +		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
>  			fprintf(stderr,
>  _("reflink not supported without CRC support\n"));
>  			usage();
> @@ -3876,7 +3876,7 @@ main(
>  			.finobt = true,
>  			.spinodes = true,
>  			.rmapbt = false,
> -			.reflink = false,
> +			.reflink = true,
>  			.parent_pointers = false,
>  			.nodalign = false,
>  			.nortalign = false,
> 
> 

Re: [PATCH 12/12 V2] mkfs: enable reflink by default

[Eric Sandeen]

On 5/22/19 11:44 AM, Darrick J. Wong wrote:
> On Tue, May 21, 2019 at 02:30:57PM -0500, Eric Sandeen wrote:
>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> Data block sharing (a.k.a. reflink) has been stable for a while, so turn
>> it on by default.
>>
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>> [sandeen: update comments & man page]
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> Looks good to me.
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Hmm I'm not allowed to RVB my own patch, right?

Nor I my part?  ;)  I'll maintain your authorship, add my
[sandeen: stuff]
part, and add my RVB.

-Eric

> 
> --D
> 
>> ---
>>
>> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
>> index 4b8c78c..78b1501 100644
>> --- a/man/man8/mkfs.xfs.8
>> +++ b/man/man8/mkfs.xfs.8
>> @@ -231,7 +231,7 @@ available for the data forks of regular files.
>>  .IP
>>  By default,
>>  .B mkfs.xfs
>> -will not create reference count btrees and therefore will not enable the
>> +will create reference count btrees and therefore will enable the
>>  reflink feature.  This feature is only available for filesystems created with
>>  the (default)
>>  .B \-m crc=1
>> @@ -239,6 +239,13 @@ option set. When the option
>>  .B \-m crc=0
>>  is used, the reference count btree feature is not supported and reflink is
>>  disabled.
>> +.IP
>> +Note: the filesystem DAX mount option (
>> +.B \-o dax
>> +) is incompatible with
>> +reflink-enabled XFS filesystems.  To use filesystem DAX with XFS, specify the
>> +.B \-m reflink=0
>> +option to mkfs.xfs to disable the reflink feature.
>>  .RE
>>  .TP
>>  .BI \-d " data_section_options"
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index 0910664..ddb25ec 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -1973,15 +1973,15 @@ _("Directory ftype field always enabled on CRC enabled filesystems\n"));
>>  			usage();
>>  		}
>>  
>> -	} else {
>> +	} else {	/* !crcs_enabled */
>>  		/*
>> -		 * The kernel doesn't currently support crc=0,finobt=1
>> -		 * filesystems. If crcs are not enabled and the user has not
>> -		 * explicitly turned finobt on, then silently turn it off to
>> -		 * avoid an unnecessary warning.
>> +		 * The kernel doesn't support crc=0,finobt=1 filesystems.
>> +		 * If crcs are not enabled and the user has not explicitly
>> +		 * turned finobt on, then silently turn it off to avoid an
>> +		 * unnecessary warning.
>>  		 * If the user explicitly tried to use crc=0,finobt=1,
>>  		 * then issue an error.
>> -		 * The same is also for sparse inodes.
>> +		 * The same is also true for sparse inodes and reflink.
>>  		 */
>>  		if (cli->sb_feat.finobt && cli_opt_set(&mopts, M_FINOBT)) {
>>  			fprintf(stderr,
>> @@ -2004,7 +2004,7 @@ _("rmapbt not supported without CRC support\n"));
>>  		}
>>  		cli->sb_feat.rmapbt = false;
>>  
>> -		if (cli->sb_feat.reflink) {
>> +		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
>>  			fprintf(stderr,
>>  _("reflink not supported without CRC support\n"));
>>  			usage();
>> @@ -3876,7 +3876,7 @@ main(
>>  			.finobt = true,
>>  			.spinodes = true,
>>  			.rmapbt = false,
>> -			.reflink = false,
>> +			.reflink = true,
>>  			.parent_pointers = false,
>>  			.nodalign = false,
>>  			.nortalign = false,
>>
>>
>