[PATCH 0/7] libfrog: switch to negative error codes

[PATCH 0/7] libfrog: switch to negative error codes

[Darrick J. Wong]

Hi all,

Start converting libfrog to return negative error codes to match libxfs.

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=libfrog-negative-error-codes

[PATCH 1/7] libfrog: print library errors

[Darrick J. Wong]

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

Add a libfrog library function that will print tagged error messages.
This will eliminate the need for a lot of open-coded:

errno = ret;
perror("...");

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/bulkstat.c     |   19 +++++++------------
 io/imap.c         |    4 ++--
 io/open.c         |   13 +++++--------
 io/stat.c         |    4 ++--
 io/swapext.c      |    7 +++----
 libfrog/Makefile  |    2 ++
 libfrog/logging.c |   18 ++++++++++++++++++
 libfrog/logging.h |   11 +++++++++++
 quota/free.c      |    4 ++--
 quota/quot.c      |   10 ++++------
 spaceman/file.c   |    7 +++----
 spaceman/health.c |   13 +++++--------
 12 files changed, 64 insertions(+), 48 deletions(-)
 create mode 100644 libfrog/logging.c
 create mode 100644 libfrog/logging.h


diff --git a/io/bulkstat.c b/io/bulkstat.c
index 625f0abe..0ea21584 100644
--- a/io/bulkstat.c
+++ b/io/bulkstat.c
@@ -7,6 +7,7 @@
 #include "platform_defs.h"
 #include "command.h"
 #include "init.h"
+#include "libfrog/logging.h"
 #include "libfrog/fsgeom.h"
 #include "libfrog/bulkstat.h"
 #include "libfrog/paths.h"
@@ -165,8 +166,7 @@ bulkstat_f(
 
 	ret = xfd_prepare_geometry(&xfd);
 	if (ret) {
-		errno = ret;
-		perror("xfd_prepare_geometry");
+		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
 		return 0;
 	}
@@ -203,8 +203,7 @@ _("bulkstat: startino=%lld flags=0x%x agno=%u ret=%d icount=%u ocount=%u\n"),
 		}
 	}
 	if (ret) {
-		errno = ret;
-		perror("xfrog_bulkstat");
+		xfrog_perror(ret, "xfrog_bulkstat");
 		exitcode = 1;
 		return 0;
 	}
@@ -269,8 +268,7 @@ bulkstat_single_f(
 
 	ret = xfd_prepare_geometry(&xfd);
 	if (ret) {
-		errno = ret;
-		perror("xfd_prepare_geometry");
+		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
 		return 0;
 	}
@@ -313,8 +311,7 @@ bulkstat_single_f(
 
 		ret = xfrog_bulkstat_single(&xfd, ino, flags, &bulkstat);
 		if (ret) {
-			errno = ret;
-			perror("xfrog_bulkstat_single");
+			xfrog_perror(ret, "xfrog_bulkstat_single");
 			continue;
 		}
 
@@ -427,8 +424,7 @@ inumbers_f(
 
 	ret = xfd_prepare_geometry(&xfd);
 	if (ret) {
-		errno = ret;
-		perror("xfd_prepare_geometry");
+		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
 		return 0;
 	}
@@ -465,8 +461,7 @@ _("bulkstat: startino=%"PRIu64" flags=0x%"PRIx32" agno=%"PRIu32" ret=%d icount=%
 		}
 	}
 	if (ret) {
-		errno = ret;
-		perror("xfrog_inumbers");
+		xfrog_perror(ret, "xfrog_inumbers");
 		exitcode = 1;
 		return 0;
 	}
diff --git a/io/imap.c b/io/imap.c
index fa69676e..6b338640 100644
--- a/io/imap.c
+++ b/io/imap.c
@@ -8,6 +8,7 @@
 #include "input.h"
 #include "init.h"
 #include "io.h"
+#include "libfrog/logging.h"
 #include "libfrog/fsgeom.h"
 #include "libfrog/bulkstat.h"
 
@@ -44,8 +45,7 @@ imap_f(int argc, char **argv)
 	}
 
 	if (error) {
-		errno = error;
-		perror("xfsctl(XFS_IOC_FSINUMBERS)");
+		xfrog_perror(error, "xfsctl(XFS_IOC_FSINUMBERS)");
 		exitcode = 1;
 	}
 	free(ireq);
diff --git a/io/open.c b/io/open.c
index 3c6113a1..c07ecb04 100644
--- a/io/open.c
+++ b/io/open.c
@@ -9,6 +9,7 @@
 #include "init.h"
 #include "io.h"
 #include "libxfs.h"
+#include "libfrog/logging.h"
 #include "libfrog/fsgeom.h"
 #include "libfrog/bulkstat.h"
 
@@ -125,8 +126,7 @@ openfile(
 
 		ret = xfrog_geometry(fd, geom);
 		if (ret) {
-			errno = ret;
-			perror("XFS_IOC_FSGEOMETRY");
+			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 			close(fd);
 			return -1;
 		}
@@ -696,8 +696,7 @@ get_last_inode(void)
 
 		ret = xfrog_inumbers(&xfd, ireq);
 		if (ret) {
-			errno = ret;
-			perror("XFS_IOC_FSINUMBERS");
+			xfrog_perror(ret, "XFS_IOC_FSINUMBERS");
 			free(ireq);
 			goto out;
 		}
@@ -798,8 +797,7 @@ inode_f(
 		/* get next inode */
 		ret = xfrog_bulkstat(&xfd, breq);
 		if (ret) {
-			errno = ret;
-			perror("bulkstat");
+			xfrog_perror(ret, "bulkstat");
 			free(breq);
 			exitcode = 1;
 			return 0;
@@ -820,8 +818,7 @@ inode_f(
 			/* Not in use */
 			result_ino = 0;
 		} else if (ret) {
-			errno = ret;
-			perror("bulkstat_single");
+			xfrog_perror(ret, "bulkstat_single");
 			exitcode = 1;
 			return 0;
 		} else {
diff --git a/io/stat.c b/io/stat.c
index 6c666146..db335780 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -12,6 +12,7 @@
 #include "io.h"
 #include "statx.h"
 #include "libxfs.h"
+#include "libfrog/logging.h"
 #include "libfrog/fsgeom.h"
 
 #include <fcntl.h>
@@ -198,8 +199,7 @@ statfs_f(
 		return 0;
 	ret = xfrog_geometry(file->fd, &fsgeo);
 	if (ret) {
-		errno = ret;
-		perror("XFS_IOC_FSGEOMETRY");
+		xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 	} else {
 		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
 		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
diff --git a/io/swapext.c b/io/swapext.c
index 1139cf21..dc4e418f 100644
--- a/io/swapext.c
+++ b/io/swapext.c
@@ -8,6 +8,7 @@
 #include "input.h"
 #include "init.h"
 #include "io.h"
+#include "libfrog/logging.h"
 #include "libfrog/fsgeom.h"
 #include "libfrog/bulkstat.h"
 
@@ -51,14 +52,12 @@ swapext_f(
 
 	error = xfrog_bulkstat_single(&fxfd, stat.st_ino, 0, &bulkstat);
 	if (error) {
-		errno = error;
-		perror("bulkstat");
+		xfrog_perror(error, "bulkstat");
 		goto out;
 	}
 	error = xfrog_bulkstat_v5_to_v1(&fxfd, &sx.sx_stat, &bulkstat);
 	if (error) {
-		errno = error;
-		perror("bulkstat conversion");
+		xfrog_perror(error, "bulkstat conversion");
 		goto out;
 	}
 	sx.sx_version = XFS_SX_VERSION;
diff --git a/libfrog/Makefile b/libfrog/Makefile
index de67bf00..780600cd 100644
--- a/libfrog/Makefile
+++ b/libfrog/Makefile
@@ -19,6 +19,7 @@ crc32.c \
 fsgeom.c \
 list_sort.c \
 linux.c \
+logging.c \
 paths.c \
 projects.c \
 ptvar.c \
@@ -38,6 +39,7 @@ crc32cselftest.h \
 crc32defs.h \
 crc32table.h \
 fsgeom.h \
+logging.h \
 paths.h \
 projects.h \
 ptvar.h \
diff --git a/libfrog/logging.c b/libfrog/logging.c
new file mode 100644
index 00000000..91ea4537
--- /dev/null
+++ b/libfrog/logging.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2019 Oracle.  All Rights Reserved.
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ */
+#include <errno.h>
+#include <stdio.h>
+#include "logging.h"
+
+/* Print an error. */
+void
+xfrog_perror(
+	int		error,
+	const char	*s)
+{
+	errno = error < 0 ? -error : error;
+	perror(s);
+}
diff --git a/libfrog/logging.h b/libfrog/logging.h
new file mode 100644
index 00000000..8b125bfd
--- /dev/null
+++ b/libfrog/logging.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2019 Oracle, Inc.
+ * All Rights Reserved.
+ */
+#ifndef __LIBFROG_LOGGING_H__
+#define __LIBFROG_LOGGING_H__
+
+void xfrog_perror(int error, const char *s);
+
+#endif	/* __LIBFROG_LOGGING_H__ */
diff --git a/quota/free.c b/quota/free.c
index 73aeb459..45ce2ceb 100644
--- a/quota/free.c
+++ b/quota/free.c
@@ -8,6 +8,7 @@
 #include "command.h"
 #include "init.h"
 #include "quota.h"
+#include "libfrog/logging.h"
 #include "libfrog/fsgeom.h"
 
 static cmdinfo_t free_cmd;
@@ -70,8 +71,7 @@ mount_free_space_data(
 	if (!(mount->fs_flags & FS_FOREIGN)) {
 		ret = xfrog_geometry(fd, &fsgeo);
 		if (ret) {
-			errno = ret;
-			perror("XFS_IOC_FSGEOMETRY");
+			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 			close(fd);
 			return 0;
 		}
diff --git a/quota/quot.c b/quota/quot.c
index 7edfad16..0f69fabd 100644
--- a/quota/quot.c
+++ b/quota/quot.c
@@ -11,6 +11,7 @@
 #include <grp.h>
 #include "init.h"
 #include "quota.h"
+#include "libfrog/logging.h"
 #include "libfrog/fsgeom.h"
 #include "libfrog/bulkstat.h"
 
@@ -147,8 +148,7 @@ quot_bulkstat_mount(
 
 	ret = xfd_open(&fsxfd, fsdir, O_RDONLY);
 	if (ret) {
-		errno = ret;
-		perror(fsdir);
+		xfrog_perror(ret, fsdir);
 		return;
 	}
 
@@ -165,10 +165,8 @@ quot_bulkstat_mount(
 		for (i = 0; i < breq->hdr.ocount; i++)
 			quot_bulkstat_add(&breq->bulkstat[i], flags);
 	}
-	if (sts < 0) {
-		errno = sts;
-		perror("XFS_IOC_FSBULKSTAT");
-	}
+	if (sts < 0)
+		xfrog_perror(sts, "XFS_IOC_FSBULKSTAT");
 	free(breq);
 	xfd_close(&fsxfd);
 }
diff --git a/spaceman/file.c b/spaceman/file.c
index 43b87e14..b7794328 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -10,6 +10,7 @@
 #include "command.h"
 #include "input.h"
 #include "init.h"
+#include "libfrog/logging.h"
 #include "libfrog/paths.h"
 #include "libfrog/fsgeom.h"
 #include "space.h"
@@ -57,10 +58,8 @@ openfile(
 			fprintf(stderr,
 _("%s: Not on a mounted XFS filesystem.\n"),
 					path);
-		else {
-			errno = ret;
-			perror(path);
-		}
+		else
+			xfrog_perror(ret, path);
 		return -1;
 	}
 
diff --git a/spaceman/health.c b/spaceman/health.c
index 8fd985a2..c3b89e8f 100644
--- a/spaceman/health.c
+++ b/spaceman/health.c
@@ -8,6 +8,7 @@
 #include "command.h"
 #include "init.h"
 #include "input.h"
+#include "libfrog/logging.h"
 #include "libfrog/paths.h"
 #include "libfrog/fsgeom.h"
 #include "libfrog/bulkstat.h"
@@ -193,8 +194,7 @@ report_ag_sick(
 
 	ret = xfrog_ag_geometry(file->xfd.fd, agno, &ageo);
 	if (ret) {
-		errno = ret;
-		perror("ag_geometry");
+		xfrog_perror(ret, "ag_geometry");
 		return 1;
 	}
 	snprintf(descr, sizeof(descr) - 1, _("AG %u"), agno);
@@ -219,8 +219,7 @@ report_inode_health(
 
 	ret = xfrog_bulkstat_single(&file->xfd, ino, 0, &bs);
 	if (ret) {
-		errno = ret;
-		perror(descr);
+		xfrog_perror(ret, descr);
 		return 1;
 	}
 
@@ -294,10 +293,8 @@ report_bulkstat_health(
 		}
 	} while (breq->hdr.ocount > 0);
 
-	if (error) {
-		errno = error;
-		perror("bulkstat");
-	}
+	if (error)
+		xfrog_perror(error, "bulkstat");
 
 	free(breq);
 	return error;

[PATCH 2/7] libfrog: convert bitmap.c to negative error codes

[Darrick J. Wong]

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

Convert libfrog functions to return negative error codes like libxfs
does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libfrog/bitmap.c |   29 ++++++++++++++++-------------
 repair/rmap.c    |    4 ++--
 scrub/phase6.c   |   12 ++++++------
 3 files changed, 24 insertions(+), 21 deletions(-)


diff --git a/libfrog/bitmap.c b/libfrog/bitmap.c
index 5daa1081..3b4c603c 100644
--- a/libfrog/bitmap.c
+++ b/libfrog/bitmap.c
@@ -76,14 +76,14 @@ bitmap_alloc(
 
 	bmap = calloc(1, sizeof(struct bitmap));
 	if (!bmap)
-		return errno;
+		return -errno;
 	bmap->bt_tree = malloc(sizeof(struct avl64tree_desc));
 	if (!bmap->bt_tree) {
-		ret = errno;
+		ret = -errno;
 		goto out;
 	}
 
-	ret = pthread_mutex_init(&bmap->bt_lock, NULL);
+	ret = -pthread_mutex_init(&bmap->bt_lock, NULL);
 	if (ret)
 		goto out_tree;
 
@@ -149,12 +149,12 @@ __bitmap_insert(
 
 	ext = bitmap_node_init(start, length);
 	if (!ext)
-		return errno;
+		return -errno;
 
 	node = avl64_insert(bmap->bt_tree, &ext->btn_node);
 	if (node == NULL) {
 		free(ext);
-		return EEXIST;
+		return -EEXIST;
 	}
 
 	return 0;
@@ -235,7 +235,7 @@ bitmap_set(
 
 #if 0	/* Unused, provided for completeness. */
 /* Clear a region of bits. */
-bool
+int
 bitmap_clear(
 	struct bitmap		*bmap,
 	uint64_t		start,
@@ -259,7 +259,7 @@ bitmap_clear(
 	/* Nothing, we're done. */
 	if (firstn == NULL && lastn == NULL) {
 		pthread_mutex_unlock(&bmap->bt_lock);
-		return true;
+		return 0;
 	}
 
 	assert(firstn != NULL && lastn != NULL);
@@ -297,20 +297,23 @@ bitmap_clear(
 					new_start;
 
 			ext = bitmap_node_init(new_start, new_length);
-			if (!ext)
-				return false;
+			if (!ext) {
+				ret = -errno;
+				goto out;
+			}
 
 			node = avl64_insert(bmap->bt_tree, &ext->btn_node);
 			if (node == NULL) {
-				errno = EEXIST;
-				return false;
+				ret = -EEXIST;
+				goto out;
 			}
 			break;
 		}
 	}
 
+out:
 	pthread_mutex_unlock(&bmap->bt_lock);
-	return true;
+	return ret;
 }
 #endif
 
@@ -323,7 +326,7 @@ bitmap_iterate(
 {
 	struct avl64node	*node;
 	struct bitmap_node	*ext;
-	int			ret;
+	int			ret = 0;
 
 	pthread_mutex_lock(&bmap->bt_lock);
 	avl_for_each(bmap->bt_tree, node) {
diff --git a/repair/rmap.c b/repair/rmap.c
index c6ed25a9..c4c99131 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -490,13 +490,13 @@ rmap_store_ag_btree_rec(
 	error = init_slab_cursor(ag_rmap->ar_raw_rmaps, rmap_compare, &rm_cur);
 	if (error)
 		goto err;
-	error = bitmap_alloc(&own_ag_bitmap);
+	error = -bitmap_alloc(&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;
-		error = bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
+		error = -bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
 					rm_rec->rm_blockcount);
 		if (error) {
 			/*
diff --git a/scrub/phase6.c b/scrub/phase6.c
index f0977d6a..bbe0d7d4 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -174,7 +174,7 @@ report_data_loss(
 	else
 		bmp = vs->d_bad;
 
-	return bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
+	return -bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
 			report_badfile, br);
 }
 
@@ -444,7 +444,7 @@ remember_ioerr(
 	if (!tree)
 		return;
 
-	ret = bitmap_set(tree, start, length);
+	ret = -bitmap_set(tree, start, length);
 	if (ret)
 		str_liberror(ctx, ret, _("setting bad block bitmap"));
 }
@@ -476,7 +476,7 @@ walk_ioerr(
 	(keys + 1)->fmr_owner = ULLONG_MAX;
 	(keys + 1)->fmr_offset = ULLONG_MAX;
 	(keys + 1)->fmr_flags = UINT_MAX;
-	return scrub_iterate_fsmap(wioerr->ctx, keys, ioerr_fsmap_report,
+	return -scrub_iterate_fsmap(wioerr->ctx, keys, ioerr_fsmap_report,
 			&start);
 }
 
@@ -498,7 +498,7 @@ walk_ioerrs(
 	tree = bitmap_for_disk(ctx, disk, vs);
 	if (!tree)
 		return 0;
-	return bitmap_iterate(tree, walk_ioerr, &wioerr);
+	return -bitmap_iterate(tree, walk_ioerr, &wioerr);
 }
 
 /* Given bad extent lists for the data & rtdev, find bad files. */
@@ -666,13 +666,13 @@ phase6_func(
 	struct media_verify_state	vs = { NULL };
 	int				ret, ret2, ret3;
 
-	ret = bitmap_alloc(&vs.d_bad);
+	ret = -bitmap_alloc(&vs.d_bad);
 	if (ret) {
 		str_liberror(ctx, ret, _("creating datadev badblock bitmap"));
 		return ret;
 	}
 
-	ret = bitmap_alloc(&vs.r_bad);
+	ret = -bitmap_alloc(&vs.r_bad);
 	if (ret) {
 		str_liberror(ctx, ret, _("creating realtime badblock bitmap"));
 		goto out_dbad;

[PATCH 3/7] libfrog: convert fsgeom.c functions to negative error codes

[Darrick J. Wong]

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

Convert libfrog functions to return negative error codes like libxfs
does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fsr/xfs_fsr.c       |    4 ++--
 growfs/xfs_growfs.c |    4 ++--
 io/bmap.c           |    2 +-
 io/bulkstat.c       |    6 +++---
 io/fsmap.c          |    4 ++--
 io/open.c           |    2 +-
 io/stat.c           |    2 +-
 libfrog/bulkstat.c  |    2 +-
 libfrog/fsgeom.c    |   18 +++++++++---------
 quota/free.c        |    2 +-
 quota/quot.c        |    2 +-
 repair/xfs_repair.c |    2 +-
 rtcp/xfs_rtcp.c     |    2 +-
 scrub/phase1.c      |    4 ++--
 spaceman/file.c     |    2 +-
 spaceman/health.c   |    2 +-
 16 files changed, 30 insertions(+), 30 deletions(-)


diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index af5d6169..3e9ba27c 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -602,7 +602,7 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
 		return -1;
 	}
 
-	ret = xfd_open(&fsxfd, mntdir, O_RDONLY);
+	ret = -xfd_open(&fsxfd, mntdir, O_RDONLY);
 	if (ret) {
 		fsrprintf(_("unable to open XFS file: %s: %s\n"),
 		          mntdir, strerror(ret));
@@ -748,7 +748,7 @@ fsrfile(
 	 * Need to open something on the same filesystem as the
 	 * file.  Open the parent.
 	 */
-	error = xfd_open(&fsxfd, getparent(fname), O_RDONLY);
+	error = -xfd_open(&fsxfd, getparent(fname), O_RDONLY);
 	if (error) {
 		fsrprintf(_("unable to open sys handle for XFS file %s: %s\n"),
 			fname, strerror(error));
diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index eab15984..d1908046 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -166,7 +166,7 @@ main(int argc, char **argv)
 	}
 
 	/* get the current filesystem size & geometry */
-	ret = xfrog_geometry(ffd, &geo);
+	ret = -xfrog_geometry(ffd, &geo);
 	if (ret) {
 		fprintf(stderr,
 	_("%s: cannot determine geometry of filesystem mounted at %s: %s\n"),
@@ -352,7 +352,7 @@ main(int argc, char **argv)
 		}
 	}
 
-	ret = xfrog_geometry(ffd, &ngeo);
+	ret = -xfrog_geometry(ffd, &ngeo);
 	if (ret) {
 		fprintf(stderr, _("%s: XFS_IOC_FSGEOMETRY xfsctl failed: %s\n"),
 			progname, strerror(ret));
diff --git a/io/bmap.c b/io/bmap.c
index cf4ea12b..f838840e 100644
--- a/io/bmap.c
+++ b/io/bmap.c
@@ -106,7 +106,7 @@ bmap_f(
 		bmv_iflags &= ~(BMV_IF_PREALLOC|BMV_IF_NO_DMAPI_READ);
 
 	if (vflag) {
-		c = xfrog_geometry(file->fd, &fsgeo);
+		c = -xfrog_geometry(file->fd, &fsgeo);
 		if (c) {
 			fprintf(stderr,
 				_("%s: can't get geometry [\"%s\"]: %s\n"),
diff --git a/io/bulkstat.c b/io/bulkstat.c
index 0ea21584..e5ee4296 100644
--- a/io/bulkstat.c
+++ b/io/bulkstat.c
@@ -164,7 +164,7 @@ bulkstat_f(
 		return 0;
 	}
 
-	ret = xfd_prepare_geometry(&xfd);
+	ret = -xfd_prepare_geometry(&xfd);
 	if (ret) {
 		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
@@ -266,7 +266,7 @@ bulkstat_single_f(
 		}
 	}
 
-	ret = xfd_prepare_geometry(&xfd);
+	ret = -xfd_prepare_geometry(&xfd);
 	if (ret) {
 		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
@@ -422,7 +422,7 @@ inumbers_f(
 		return 0;
 	}
 
-	ret = xfd_prepare_geometry(&xfd);
+	ret = -xfd_prepare_geometry(&xfd);
 	if (ret) {
 		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
diff --git a/io/fsmap.c b/io/fsmap.c
index 12ec1e44..feacb264 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -448,11 +448,11 @@ fsmap_f(
 	}
 
 	if (vflag) {
-		c = xfrog_geometry(file->fd, &fsgeo);
+		c = -xfrog_geometry(file->fd, &fsgeo);
 		if (c) {
 			fprintf(stderr,
 				_("%s: can't get geometry [\"%s\"]: %s\n"),
-				progname, file->name, strerror(errno));
+				progname, file->name, strerror(c));
 			exitcode = 1;
 			return 0;
 		}
diff --git a/io/open.c b/io/open.c
index c07ecb04..3bbc5d0a 100644
--- a/io/open.c
+++ b/io/open.c
@@ -124,7 +124,7 @@ openfile(
 	} else {
 		int	ret;
 
-		ret = xfrog_geometry(fd, geom);
+		ret = -xfrog_geometry(fd, geom);
 		if (ret) {
 			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 			close(fd);
diff --git a/io/stat.c b/io/stat.c
index db335780..d125a0f7 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -197,7 +197,7 @@ statfs_f(
 	}
 	if (file->flags & IO_FOREIGN)
 		return 0;
-	ret = xfrog_geometry(file->fd, &fsgeo);
+	ret = -xfrog_geometry(file->fd, &fsgeo);
 	if (ret) {
 		xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 	} else {
diff --git a/libfrog/bulkstat.c b/libfrog/bulkstat.c
index 538b5197..38d634f7 100644
--- a/libfrog/bulkstat.c
+++ b/libfrog/bulkstat.c
@@ -39,7 +39,7 @@ xfrog_bulkstat_prep_v1_emulation(
 	if (xfd->fsgeom.blocksize > 0)
 		return 0;
 
-	return xfd_prepare_geometry(xfd);
+	return -xfd_prepare_geometry(xfd);
 }
 
 /* Bulkstat a single inode using v5 ioctl. */
diff --git a/libfrog/fsgeom.c b/libfrog/fsgeom.c
index 3ea91e3f..19a4911f 100644
--- a/libfrog/fsgeom.c
+++ b/libfrog/fsgeom.c
@@ -69,7 +69,7 @@ xfs_report_geom(
 			(unsigned long long)geo->rtextents);
 }
 
-/* Try to obtain the xfs geometry.  On error returns a positive error code. */
+/* Try to obtain the xfs geometry.  On error returns a negative error code. */
 int
 xfrog_geometry(
 	int			fd,
@@ -91,12 +91,12 @@ xfrog_geometry(
 	if (!ret)
 		return 0;
 
-	return errno;
+	return -errno;
 }
 
 /*
  * Prepare xfs_fd structure for future ioctl operations by computing the xfs
- * geometry for @xfd->fd.  Returns zero or a positive error code.
+ * geometry for @xfd->fd.  Returns zero or a negative error code.
  */
 int
 xfd_prepare_geometry(
@@ -117,7 +117,7 @@ xfd_prepare_geometry(
 	return 0;
 }
 
-/* Open a file on an XFS filesystem.  Returns zero or a positive error code. */
+/* Open a file on an XFS filesystem.  Returns zero or a negative error code. */
 int
 xfd_open(
 	struct xfs_fd		*xfd,
@@ -128,7 +128,7 @@ xfd_open(
 
 	xfd->fd = open(pathname, flags);
 	if (xfd->fd < 0)
-		return errno;
+		return -errno;
 
 	ret = xfd_prepare_geometry(xfd);
 	if (ret) {
@@ -141,7 +141,7 @@ xfd_open(
 
 /*
  * Release any resources associated with this xfs_fd structure.  Returns zero
- * or a positive error code.
+ * or a negative error code.
  */
 int
 xfd_close(
@@ -155,12 +155,12 @@ xfd_close(
 	ret = close(xfd->fd);
 	xfd->fd = -1;
 	if (ret < 0)
-		return errno;
+		return -errno;
 
 	return 0;
 }
 
-/* Try to obtain an AG's geometry.  Returns zero or a positive error code. */
+/* Try to obtain an AG's geometry.  Returns zero or a negative error code. */
 int
 xfrog_ag_geometry(
 	int			fd,
@@ -172,6 +172,6 @@ xfrog_ag_geometry(
 	ageo->ag_number = agno;
 	ret = ioctl(fd, XFS_IOC_AG_GEOMETRY, ageo);
 	if (ret)
-		return errno;
+		return -errno;
 	return 0;
 }
diff --git a/quota/free.c b/quota/free.c
index 45ce2ceb..ea9c112f 100644
--- a/quota/free.c
+++ b/quota/free.c
@@ -69,7 +69,7 @@ mount_free_space_data(
 	}
 
 	if (!(mount->fs_flags & FS_FOREIGN)) {
-		ret = xfrog_geometry(fd, &fsgeo);
+		ret = -xfrog_geometry(fd, &fsgeo);
 		if (ret) {
 			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 			close(fd);
diff --git a/quota/quot.c b/quota/quot.c
index 0f69fabd..df3825f2 100644
--- a/quota/quot.c
+++ b/quota/quot.c
@@ -146,7 +146,7 @@ quot_bulkstat_mount(
 			*dp = NULL;
 	ndu[0] = ndu[1] = ndu[2] = 0;
 
-	ret = xfd_open(&fsxfd, fsdir, O_RDONLY);
+	ret = -xfd_open(&fsxfd, fsdir, O_RDONLY);
 	if (ret) {
 		xfrog_perror(ret, fsdir);
 		return;
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index 7e810ef4..61ea3b11 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -641,7 +641,7 @@ check_fs_vs_host_sectsize(
 
 	fd = libxfs_device_to_fd(x.ddev);
 
-	ret = xfrog_geometry(fd, &geom);
+	ret = -xfrog_geometry(fd, &geom);
 	if (ret) {
 		do_log(_("Cannot get host filesystem geometry.\n"
 	"Repair may fail if there is a sector size mismatch between\n"
diff --git a/rtcp/xfs_rtcp.c b/rtcp/xfs_rtcp.c
index a5737699..7c4197b1 100644
--- a/rtcp/xfs_rtcp.c
+++ b/rtcp/xfs_rtcp.c
@@ -378,7 +378,7 @@ xfsrtextsize( char *path)
 			progname, path, strerror(errno));
 		return -1;
 	}
-	rval = xfrog_geometry(fd, &geo);
+	rval = -xfrog_geometry(fd, &geo);
 	close(fd);
 	if (rval)
 		return -1;
diff --git a/scrub/phase1.c b/scrub/phase1.c
index 0d343b03..eabcaecf 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -60,7 +60,7 @@ scrub_cleanup(
 	if (ctx->datadev)
 		disk_close(ctx->datadev);
 	fshandle_destroy();
-	error = xfd_close(&ctx->mnt);
+	error = -xfd_close(&ctx->mnt);
 	if (error)
 		str_liberror(ctx, error, _("closing mountpoint fd"));
 	fs_table_destroy();
@@ -84,7 +84,7 @@ phase1_func(
 	 * CAP_SYS_ADMIN, which we probably need to do anything fancy
 	 * with the (XFS driver) kernel.
 	 */
-	error = xfd_open(&ctx->mnt, ctx->mntpoint,
+	error = -xfd_open(&ctx->mnt, ctx->mntpoint,
 			O_RDONLY | O_NOATIME | O_DIRECTORY);
 	if (error) {
 		if (error == EPERM)
diff --git a/spaceman/file.c b/spaceman/file.c
index b7794328..eec7ee9f 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -52,7 +52,7 @@ openfile(
 	struct fs_path	*fsp;
 	int		ret;
 
-	ret = xfd_open(xfd, path, O_RDONLY);
+	ret = -xfd_open(xfd, path, O_RDONLY);
 	if (ret) {
 		if (ret == ENOTTY)
 			fprintf(stderr,
diff --git a/spaceman/health.c b/spaceman/health.c
index c3b89e8f..a10d2d4a 100644
--- a/spaceman/health.c
+++ b/spaceman/health.c
@@ -192,7 +192,7 @@ report_ag_sick(
 	char			descr[256];
 	int			ret;
 
-	ret = xfrog_ag_geometry(file->xfd.fd, agno, &ageo);
+	ret = -xfrog_ag_geometry(file->xfd.fd, agno, &ageo);
 	if (ret) {
 		xfrog_perror(ret, "ag_geometry");
 		return 1;

[PATCH 4/7] libfrog: convert bulkstat.c functions to negative error codes

[Darrick J. Wong]

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

Convert libfrog functions to return negative error codes like libxfs
does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fsr/xfs_fsr.c      |   18 ++++++-----
 io/bulkstat.c      |   18 ++++++-----
 io/imap.c          |    8 +++--
 io/open.c          |   21 ++++++-------
 io/swapext.c       |    4 +--
 libfrog/bulkstat.c |   82 +++++++++++++++++++++++++++-------------------------
 libfrog/bulkstat.h |    8 +++--
 quota/quot.c       |    8 +++--
 scrub/fscounters.c |    8 +++--
 scrub/inodes.c     |   20 ++++++-------
 spaceman/health.c  |   10 +++---
 11 files changed, 104 insertions(+), 101 deletions(-)


diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 3e9ba27c..77a10a1d 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -613,16 +613,15 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
 
 	tmp_init(mntdir);
 
-	breq = xfrog_bulkstat_alloc_req(GRABSZ, startino);
-	if (!breq) {
-		fsrprintf(_("Skipping %s: not enough memory\n"),
-			  mntdir);
+	ret = -xfrog_bulkstat_alloc_req(GRABSZ, startino, &breq);
+	if (ret) {
+		fsrprintf(_("Skipping %s: %s\n"), mntdir, strerror(ret));
 		xfd_close(&fsxfd);
 		free(fshandlep);
 		return -1;
 	}
 
-	while ((ret = xfrog_bulkstat(&fsxfd, breq) == 0)) {
+	while ((ret = -xfrog_bulkstat(&fsxfd, breq) == 0)) {
 		struct xfs_bstat	bs1;
 		struct xfs_bulkstat	*buf = breq->bulkstat;
 		struct xfs_bulkstat	*p;
@@ -643,7 +642,7 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
 			     (p->bs_extents < 2))
 				continue;
 
-			ret = xfrog_bulkstat_v5_to_v1(&fsxfd, &bs1, p);
+			ret = -xfrog_bulkstat_v5_to_v1(&fsxfd, &bs1, p);
 			if (ret) {
 				fsrprintf(_("bstat conversion error: %s\n"),
 						strerror(ret));
@@ -755,13 +754,13 @@ fsrfile(
 		goto out;
 	}
 
-	error = xfrog_bulkstat_single(&fsxfd, ino, 0, &bulkstat);
+	error = -xfrog_bulkstat_single(&fsxfd, ino, 0, &bulkstat);
 	if (error) {
 		fsrprintf(_("unable to get bstat on %s: %s\n"),
 			fname, strerror(error));
 		goto out;
 	}
-	error = xfrog_bulkstat_v5_to_v1(&fsxfd, &statbuf, &bulkstat);
+	error = -xfrog_bulkstat_v5_to_v1(&fsxfd, &statbuf, &bulkstat);
 	if (error) {
 		fsrprintf(_("bstat conversion error on %s: %s\n"),
 			fname, strerror(error));
@@ -996,7 +995,8 @@ fsr_setup_attr_fork(
 		 * this to compare against the target and determine what we
 		 * need to do.
 		 */
-		ret = xfrog_bulkstat_single(&txfd, tstatbuf.st_ino, 0, &tbstat);
+		ret = -xfrog_bulkstat_single(&txfd, tstatbuf.st_ino, 0,
+				&tbstat);
 		if (ret) {
 			fsrprintf(_("unable to get bstat on temp file: %s\n"),
 						strerror(ret));
diff --git a/io/bulkstat.c b/io/bulkstat.c
index e5ee4296..58afaf76 100644
--- a/io/bulkstat.c
+++ b/io/bulkstat.c
@@ -171,9 +171,9 @@ bulkstat_f(
 		return 0;
 	}
 
-	breq = xfrog_bulkstat_alloc_req(batch_size, startino);
-	if (!breq) {
-		perror("alloc bulkreq");
+	ret = -xfrog_bulkstat_alloc_req(batch_size, startino, &breq);
+	if (ret) {
+		xfrog_perror(ret, "alloc bulkreq");
 		exitcode = 1;
 		return 0;
 	}
@@ -183,7 +183,7 @@ bulkstat_f(
 
 	set_xfd_flags(&xfd, ver);
 
-	while ((ret = xfrog_bulkstat(&xfd, breq)) == 0) {
+	while ((ret = -xfrog_bulkstat(&xfd, breq)) == 0) {
 		if (debug)
 			printf(
 _("bulkstat: startino=%lld flags=0x%x agno=%u ret=%d icount=%u ocount=%u\n"),
@@ -309,7 +309,7 @@ bulkstat_single_f(
 			}
 		}
 
-		ret = xfrog_bulkstat_single(&xfd, ino, flags, &bulkstat);
+		ret = -xfrog_bulkstat_single(&xfd, ino, flags, &bulkstat);
 		if (ret) {
 			xfrog_perror(ret, "xfrog_bulkstat_single");
 			continue;
@@ -429,9 +429,9 @@ inumbers_f(
 		return 0;
 	}
 
-	ireq = xfrog_inumbers_alloc_req(batch_size, startino);
-	if (!ireq) {
-		perror("alloc inumbersreq");
+	ret = -xfrog_inumbers_alloc_req(batch_size, startino, &ireq);
+	if (ret) {
+		xfrog_perror(ret, "alloc inumbersreq");
 		exitcode = 1;
 		return 0;
 	}
@@ -441,7 +441,7 @@ inumbers_f(
 
 	set_xfd_flags(&xfd, ver);
 
-	while ((ret = xfrog_inumbers(&xfd, ireq)) == 0) {
+	while ((ret = -xfrog_inumbers(&xfd, ireq)) == 0) {
 		if (debug)
 			printf(
 _("bulkstat: startino=%"PRIu64" flags=0x%"PRIx32" agno=%"PRIu32" ret=%d icount=%"PRIu32" ocount=%"PRIu32"\n"),
diff --git a/io/imap.c b/io/imap.c
index 6b338640..e75dad1a 100644
--- a/io/imap.c
+++ b/io/imap.c
@@ -28,13 +28,13 @@ imap_f(int argc, char **argv)
 	else
 		nent = atoi(argv[1]);
 
-	ireq = xfrog_inumbers_alloc_req(nent, 0);
-	if (!ireq) {
-		perror("alloc req");
+	error = -xfrog_inumbers_alloc_req(nent, 0, &ireq);
+	if (error) {
+		xfrog_perror(error, "alloc req");
 		return 0;
 	}
 
-	while ((error = xfrog_inumbers(&xfd, ireq)) == 0 &&
+	while ((error = -xfrog_inumbers(&xfd, ireq)) == 0 &&
 	       ireq->hdr.ocount > 0) {
 		for (i = 0; i < ireq->hdr.ocount; i++) {
 			printf(_("ino %10"PRIu64" count %2d mask %016"PRIx64"\n"),
diff --git a/io/open.c b/io/open.c
index 3bbc5d0a..eb5d653c 100644
--- a/io/open.c
+++ b/io/open.c
@@ -684,17 +684,16 @@ get_last_inode(void)
 	struct xfs_inumbers_req	*ireq;
 	uint32_t		lastgrp = 0;
 	__u64			last_ino = 0;
+	int			ret;
 
-	ireq = xfrog_inumbers_alloc_req(IGROUP_NR, 0);
-	if (!ireq) {
-		perror("alloc req");
+	ret = -xfrog_inumbers_alloc_req(IGROUP_NR, 0, &ireq);
+	if (ret) {
+		xfrog_perror(ret, "alloc req");
 		return 0;
 	}
 
 	for (;;) {
-		int		ret;
-
-		ret = xfrog_inumbers(&xfd, ireq);
+		ret = -xfrog_inumbers(&xfd, ireq);
 		if (ret) {
 			xfrog_perror(ret, "XFS_IOC_FSINUMBERS");
 			free(ireq);
@@ -787,15 +786,15 @@ inode_f(
 		 * The -n option means that the caller wants to know the number
 		 * of the next allocated inode, so we need to increment here.
 		 */
-		breq = xfrog_bulkstat_alloc_req(1, userino + 1);
-		if (!breq) {
-			perror("alloc bulkstat");
+		ret = -xfrog_bulkstat_alloc_req(1, userino + 1, &breq);
+		if (ret) {
+			xfrog_perror(ret, "alloc bulkstat");
 			exitcode = 1;
 			return 0;
 		}
 
 		/* get next inode */
-		ret = xfrog_bulkstat(&xfd, breq);
+		ret = -xfrog_bulkstat(&xfd, breq);
 		if (ret) {
 			xfrog_perror(ret, "bulkstat");
 			free(breq);
@@ -813,7 +812,7 @@ inode_f(
 		struct xfs_fd	xfd = XFS_FD_INIT(file->fd);
 
 		/* get this inode */
-		ret = xfrog_bulkstat_single(&xfd, userino, 0, &bulkstat);
+		ret = -xfrog_bulkstat_single(&xfd, userino, 0, &bulkstat);
 		if (ret == EINVAL) {
 			/* Not in use */
 			result_ino = 0;
diff --git a/io/swapext.c b/io/swapext.c
index dc4e418f..a4153bb7 100644
--- a/io/swapext.c
+++ b/io/swapext.c
@@ -50,12 +50,12 @@ swapext_f(
 		goto out;
 	}
 
-	error = xfrog_bulkstat_single(&fxfd, stat.st_ino, 0, &bulkstat);
+	error = -xfrog_bulkstat_single(&fxfd, stat.st_ino, 0, &bulkstat);
 	if (error) {
 		xfrog_perror(error, "bulkstat");
 		goto out;
 	}
-	error = xfrog_bulkstat_v5_to_v1(&fxfd, &sx.sx_stat, &bulkstat);
+	error = -xfrog_bulkstat_v5_to_v1(&fxfd, &sx.sx_stat, &bulkstat);
 	if (error) {
 		xfrog_perror(error, "bulkstat conversion");
 		goto out;
diff --git a/libfrog/bulkstat.c b/libfrog/bulkstat.c
index 38d634f7..c3e5c5f8 100644
--- a/libfrog/bulkstat.c
+++ b/libfrog/bulkstat.c
@@ -39,7 +39,7 @@ xfrog_bulkstat_prep_v1_emulation(
 	if (xfd->fsgeom.blocksize > 0)
 		return 0;
 
-	return -xfd_prepare_geometry(xfd);
+	return xfd_prepare_geometry(xfd);
 }
 
 /* Bulkstat a single inode using v5 ioctl. */
@@ -54,21 +54,21 @@ xfrog_bulkstat_single5(
 	int				ret;
 
 	if (flags & ~(XFS_BULK_IREQ_SPECIAL))
-		return EINVAL;
+		return -EINVAL;
 
-	req = xfrog_bulkstat_alloc_req(1, ino);
-	if (!req)
-		return ENOMEM;
+	ret = xfrog_bulkstat_alloc_req(1, ino, &req);
+	if (ret)
+		return ret;
 
 	req->hdr.flags = flags;
 	ret = ioctl(xfd->fd, XFS_IOC_BULKSTAT, req);
 	if (ret) {
-		ret = errno;
+		ret = -errno;
 		goto free;
 	}
 
 	if (req->hdr.ocount == 0) {
-		ret = ENOENT;
+		ret = -ENOENT;
 		goto free;
 	}
 
@@ -91,7 +91,7 @@ xfrog_bulkstat_single1(
 	int				error;
 
 	if (flags)
-		return EINVAL;
+		return -EINVAL;
 
 	error = xfrog_bulkstat_prep_v1_emulation(xfd);
 	if (error)
@@ -102,13 +102,13 @@ xfrog_bulkstat_single1(
 	bulkreq.ubuffer = &bstat;
 	error = ioctl(xfd->fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
 	if (error)
-		return errno;
+		return -errno;
 
 	xfrog_bulkstat_v1_to_v5(xfd, bulkstat, &bstat);
 	return 0;
 }
 
-/* Bulkstat a single inode.  Returns zero or a positive error code. */
+/* Bulkstat a single inode.  Returns zero or a negative error code. */
 int
 xfrog_bulkstat_single(
 	struct xfs_fd			*xfd,
@@ -127,8 +127,8 @@ xfrog_bulkstat_single(
 
 	/* If the v5 ioctl wasn't found, we punt to v1. */
 	switch (error) {
-	case EOPNOTSUPP:
-	case ENOTTY:
+	case -EOPNOTSUPP:
+	case -ENOTTY:
 		xfd->flags |= XFROG_FLAG_BULKSTAT_FORCE_V1;
 		break;
 	}
@@ -143,7 +143,7 @@ xfrog_bulkstat_single(
  * kernels.
  *
  * Returns 0 if the emulation should proceed; ECANCELED if there are no
- * records; or a positive error code.
+ * records; or a negative error code.
  */
 static int
 xfrog_bulk_req_v1_setup(
@@ -160,7 +160,7 @@ xfrog_bulk_req_v1_setup(
 		if (hdr->ino == 0)
 			hdr->ino = cvt_agino_to_ino(xfd, hdr->agno, 0);
 		else if (agno < hdr->agno)
-			return EINVAL;
+			return -EINVAL;
 		else if (agno > hdr->agno)
 			goto no_results;
 	}
@@ -170,7 +170,7 @@ xfrog_bulk_req_v1_setup(
 
 	buf = malloc(hdr->icount * rec_size);
 	if (!buf)
-		return errno;
+		return -errno;
 
 	if (hdr->ino)
 		hdr->ino--;
@@ -182,7 +182,7 @@ xfrog_bulk_req_v1_setup(
 
 no_results:
 	hdr->ocount = 0;
-	return ECANCELED;
+	return -ECANCELED;
 }
 
 /*
@@ -210,7 +210,7 @@ xfrog_bulk_req_v1_cleanup(
 	void			*v5_rec = v5_records;
 	unsigned int		i;
 
-	if (error == ECANCELED) {
+	if (error == -ECANCELED) {
 		error = 0;
 		goto free;
 	}
@@ -262,7 +262,7 @@ xfrog_bulkstat5(
 
 	ret = ioctl(xfd->fd, XFS_IOC_BULKSTAT, req);
 	if (ret)
-		return errno;
+		return -errno;
 	return 0;
 }
 
@@ -281,14 +281,14 @@ xfrog_bulkstat1(
 
 	error = xfrog_bulk_req_v1_setup(xfd, &req->hdr, &bulkreq,
 			sizeof(struct xfs_bstat));
-	if (error == ECANCELED)
+	if (error == -ECANCELED)
 		goto out_teardown;
 	if (error)
 		return error;
 
 	error = ioctl(xfd->fd, XFS_IOC_FSBULKSTAT, &bulkreq);
 	if (error)
-		error = errno;
+		error = -errno;
 
 out_teardown:
 	return xfrog_bulk_req_v1_cleanup(xfd, &req->hdr, &bulkreq,
@@ -314,8 +314,8 @@ xfrog_bulkstat(
 
 	/* If the v5 ioctl wasn't found, we punt to v1. */
 	switch (error) {
-	case EOPNOTSUPP:
-	case ENOTTY:
+	case -EOPNOTSUPP:
+	case -ENOTTY:
 		xfd->flags |= XFROG_FLAG_BULKSTAT_FORCE_V1;
 		break;
 	}
@@ -347,7 +347,7 @@ xfrog_bulkstat_v5_to_v1(
 	    time_too_big(bs5->bs_atime) ||
 	    time_too_big(bs5->bs_ctime) ||
 	    time_too_big(bs5->bs_mtime))
-		return ERANGE;
+		return -ERANGE;
 
 	bs1->bs_ino = bs5->bs_ino;
 	bs1->bs_mode = bs5->bs_mode;
@@ -417,22 +417,24 @@ xfrog_bulkstat_v1_to_v5(
 	bs5->bs_aextents = bs1->bs_aextents;
 }
 
-/* Allocate a bulkstat request.  On error returns NULL and sets errno. */
-struct xfs_bulkstat_req *
+/* Allocate a bulkstat request.  Returns zero or a negative error code. */
+int
 xfrog_bulkstat_alloc_req(
 	uint32_t		nr,
-	uint64_t		startino)
+	uint64_t		startino,
+	struct xfs_bulkstat_req **preq)
 {
 	struct xfs_bulkstat_req	*breq;
 
 	breq = calloc(1, XFS_BULKSTAT_REQ_SIZE(nr));
 	if (!breq)
-		return NULL;
+		return -errno;
 
 	breq->hdr.icount = nr;
 	breq->hdr.ino = startino;
 
-	return breq;
+	*preq = breq;
+	return 0;
 }
 
 /* Set a bulkstat cursor to iterate only a particular AG. */
@@ -490,7 +492,7 @@ xfrog_inumbers5(
 
 	ret = ioctl(xfd->fd, XFS_IOC_INUMBERS, req);
 	if (ret)
-		return errno;
+		return -errno;
 	return 0;
 }
 
@@ -509,14 +511,14 @@ xfrog_inumbers1(
 
 	error = xfrog_bulk_req_v1_setup(xfd, &req->hdr, &bulkreq,
 			sizeof(struct xfs_inogrp));
-	if (error == ECANCELED)
+	if (error == -ECANCELED)
 		goto out_teardown;
 	if (error)
 		return error;
 
 	error = ioctl(xfd->fd, XFS_IOC_FSINUMBERS, &bulkreq);
 	if (error)
-		error = errno;
+		error = -errno;
 
 out_teardown:
 	return xfrog_bulk_req_v1_cleanup(xfd, &req->hdr, &bulkreq,
@@ -526,7 +528,7 @@ xfrog_inumbers1(
 }
 
 /*
- * Query inode allocation bitmask information.  Returns zero or a positive
+ * Query inode allocation bitmask information.  Returns zero or a negative
  * error code.
  */
 int
@@ -545,8 +547,8 @@ xfrog_inumbers(
 
 	/* If the v5 ioctl wasn't found, we punt to v1. */
 	switch (error) {
-	case EOPNOTSUPP:
-	case ENOTTY:
+	case -EOPNOTSUPP:
+	case -ENOTTY:
 		xfd->flags |= XFROG_FLAG_BULKSTAT_FORCE_V1;
 		break;
 	}
@@ -555,22 +557,24 @@ xfrog_inumbers(
 	return xfrog_inumbers1(xfd, req);
 }
 
-/* Allocate a inumbers request.  On error returns NULL and sets errno. */
-struct xfs_inumbers_req *
+/* Allocate a inumbers request.  Returns zero or a negative error code. */
+int
 xfrog_inumbers_alloc_req(
 	uint32_t		nr,
-	uint64_t		startino)
+	uint64_t		startino,
+	struct xfs_inumbers_req **preq)
 {
 	struct xfs_inumbers_req	*ireq;
 
 	ireq = calloc(1, XFS_INUMBERS_REQ_SIZE(nr));
 	if (!ireq)
-		return NULL;
+		return -errno;
 
 	ireq->hdr.icount = nr;
 	ireq->hdr.ino = startino;
 
-	return ireq;
+	*preq = ireq;
+	return 0;
 }
 
 /* Set an inumbers cursor to iterate only a particular AG. */
diff --git a/libfrog/bulkstat.h b/libfrog/bulkstat.h
index 133a99b8..56ef7f9a 100644
--- a/libfrog/bulkstat.h
+++ b/libfrog/bulkstat.h
@@ -12,8 +12,8 @@ int xfrog_bulkstat_single(struct xfs_fd *xfd, uint64_t ino, unsigned int flags,
 		struct xfs_bulkstat *bulkstat);
 int xfrog_bulkstat(struct xfs_fd *xfd, struct xfs_bulkstat_req *req);
 
-struct xfs_bulkstat_req *xfrog_bulkstat_alloc_req(uint32_t nr,
-		uint64_t startino);
+int xfrog_bulkstat_alloc_req(uint32_t nr, uint64_t startino,
+		struct xfs_bulkstat_req **preq);
 int xfrog_bulkstat_v5_to_v1(struct xfs_fd *xfd, struct xfs_bstat *bs1,
 		const struct xfs_bulkstat *bstat);
 void xfrog_bulkstat_v1_to_v5(struct xfs_fd *xfd, struct xfs_bulkstat *bstat,
@@ -24,8 +24,8 @@ void xfrog_bulkstat_set_ag(struct xfs_bulkstat_req *req, uint32_t agno);
 struct xfs_inogrp;
 int xfrog_inumbers(struct xfs_fd *xfd, struct xfs_inumbers_req *req);
 
-struct xfs_inumbers_req *xfrog_inumbers_alloc_req(uint32_t nr,
-		uint64_t startino);
+int xfrog_inumbers_alloc_req(uint32_t nr, uint64_t startino,
+		struct xfs_inumbers_req **preq);
 void xfrog_inumbers_set_ag(struct xfs_inumbers_req *req, uint32_t agno);
 void xfrog_inumbers_v5_to_v1(struct xfs_inogrp *ig1,
 		const struct xfs_inumbers *ig);
diff --git a/quota/quot.c b/quota/quot.c
index df3825f2..8544aef6 100644
--- a/quota/quot.c
+++ b/quota/quot.c
@@ -152,14 +152,14 @@ quot_bulkstat_mount(
 		return;
 	}
 
-	breq = xfrog_bulkstat_alloc_req(NBSTAT, 0);
-	if (!breq) {
-		perror("calloc");
+	ret = -xfrog_bulkstat_alloc_req(NBSTAT, 0, &breq);
+	if (ret) {
+		xfrog_perror(ret, "calloc");
 		xfd_close(&fsxfd);
 		return;
 	}
 
-	while ((sts = xfrog_bulkstat(&fsxfd, breq)) == 0) {
+	while ((sts = -xfrog_bulkstat(&fsxfd, breq)) == 0) {
 		if (breq->hdr.ocount == 0)
 			break;
 		for (i = 0; i < breq->hdr.ocount; i++)
diff --git a/scrub/fscounters.c b/scrub/fscounters.c
index 2581947f..a6b62f34 100644
--- a/scrub/fscounters.c
+++ b/scrub/fscounters.c
@@ -47,14 +47,14 @@ count_ag_inodes(
 	unsigned int		i;
 	int			error;
 
-	ireq = xfrog_inumbers_alloc_req(64, 0);
-	if (!ireq) {
-		ci->error = errno;
+	error = -xfrog_inumbers_alloc_req(64, 0, &ireq);
+	if (error) {
+		ci->error = error;
 		return;
 	}
 	xfrog_inumbers_set_ag(ireq, agno);
 
-	while (!ci->error && (error = xfrog_inumbers(&ctx->mnt, ireq)) == 0) {
+	while (!ci->error && (error = -xfrog_inumbers(&ctx->mnt, ireq)) == 0) {
 		if (ireq->hdr.ocount == 0)
 			break;
 		for (i = 0; i < ireq->hdr.ocount; i++)
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 142582eb..90d66c45 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -65,7 +65,7 @@ fill_in_bulkstat_holes(
 		}
 
 		/* Load the one inode. */
-		error = xfrog_bulkstat_single(&ctx->mnt,
+		error = -xfrog_bulkstat_single(&ctx->mnt,
 				inumbers->xi_startino + i, 0, bs);
 		if (error || bs->bs_ino != inumbers->xi_startino + i) {
 			memset(bs, 0, sizeof(struct xfs_bulkstat));
@@ -116,16 +116,16 @@ scan_ag_inodes(
 			sizeof(handle.ha_fid.fid_len);
 	handle.ha_fid.fid_pad = 0;
 
-	breq = xfrog_bulkstat_alloc_req(XFS_INODES_PER_CHUNK, 0);
-	if (!breq) {
-		str_errno(ctx, descr);
+	error = -xfrog_bulkstat_alloc_req(XFS_INODES_PER_CHUNK, 0, &breq);
+	if (error) {
+		str_liberror(ctx, error, descr);
 		si->aborted = true;
 		return;
 	}
 
-	ireq = xfrog_inumbers_alloc_req(1, 0);
-	if (!ireq) {
-		str_errno(ctx, descr);
+	error = -xfrog_inumbers_alloc_req(1, 0, &ireq);
+	if (error) {
+		str_liberror(ctx, error, descr);
 		free(breq);
 		si->aborted = true;
 		return;
@@ -134,7 +134,7 @@ scan_ag_inodes(
 	xfrog_inumbers_set_ag(ireq, agno);
 
 	/* Find the inode chunk & alloc mask */
-	error = xfrog_inumbers(&ctx->mnt, ireq);
+	error = -xfrog_inumbers(&ctx->mnt, ireq);
 	while (!error && !si->aborted && ireq->hdr.ocount > 0) {
 		/*
 		 * We can have totally empty inode chunks on filesystems where
@@ -145,7 +145,7 @@ scan_ag_inodes(
 
 		breq->hdr.ino = inumbers->xi_startino;
 		breq->hdr.icount = inumbers->xi_alloccount;
-		error = xfrog_bulkstat(&ctx->mnt, breq);
+		error = -xfrog_bulkstat(&ctx->mnt, breq);
 		if (error) {
 			char	errbuf[DESCR_BUFSZ];
 
@@ -193,7 +193,7 @@ _("Changed too many times during scan; giving up."));
 
 		stale_count = 0;
 igrp_retry:
-		error = xfrog_inumbers(&ctx->mnt, ireq);
+		error = -xfrog_inumbers(&ctx->mnt, ireq);
 	}
 
 err:
diff --git a/spaceman/health.c b/spaceman/health.c
index a10d2d4a..14538f55 100644
--- a/spaceman/health.c
+++ b/spaceman/health.c
@@ -217,7 +217,7 @@ report_inode_health(
 		descr = d;
 	}
 
-	ret = xfrog_bulkstat_single(&file->xfd, ino, 0, &bs);
+	ret = -xfrog_bulkstat_single(&file->xfd, ino, 0, &bs);
 	if (ret) {
 		xfrog_perror(ret, descr);
 		return 1;
@@ -270,9 +270,9 @@ report_bulkstat_health(
 	uint32_t		i;
 	int			error;
 
-	breq = xfrog_bulkstat_alloc_req(BULKSTAT_NR, 0);
-	if (!breq) {
-		perror("bulk alloc req");
+	error = -xfrog_bulkstat_alloc_req(BULKSTAT_NR, 0, &breq);
+	if (error) {
+		xfrog_perror(error, "bulk alloc req");
 		exitcode = 1;
 		return 1;
 	}
@@ -281,7 +281,7 @@ report_bulkstat_health(
 		xfrog_bulkstat_set_ag(breq, agno);
 
 	do {
-		error = xfrog_bulkstat(&file->xfd, breq);
+		error = -xfrog_bulkstat(&file->xfd, breq);
 		if (error)
 			break;
 		for (i = 0; i < breq->hdr.ocount; i++) {

[PATCH 5/7] libfrog: convert ptvar.c functions to negative error codes

[Darrick J. Wong]

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

Convert libfrog functions to return negative error codes like libxfs
does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libfrog/ptvar.c     |    8 ++++----
 scrub/counter.c     |    6 +++---
 scrub/descr.c       |    2 +-
 scrub/phase7.c      |    8 ++++----
 scrub/read_verify.c |    8 ++++----
 5 files changed, 16 insertions(+), 16 deletions(-)


diff --git a/libfrog/ptvar.c b/libfrog/ptvar.c
index 55324b71..9c4b1736 100644
--- a/libfrog/ptvar.c
+++ b/libfrog/ptvar.c
@@ -54,15 +54,15 @@ ptvar_alloc(
 
 	ptv = malloc(PTVAR_SIZE(nr, size));
 	if (!ptv)
-		return errno;
+		return -errno;
 	ptv->data_size = size;
 	ptv->nr_counters = nr;
 	ptv->nr_used = 0;
 	memset(ptv->data, 0, nr * size);
-	ret = pthread_mutex_init(&ptv->lock, NULL);
+	ret = -pthread_mutex_init(&ptv->lock, NULL);
 	if (ret)
 		goto out;
-	ret = pthread_key_create(&ptv->key, NULL);
+	ret = -pthread_key_create(&ptv->key, NULL);
 	if (ret)
 		goto out_mutex;
 
@@ -99,7 +99,7 @@ ptvar_get(
 		pthread_mutex_lock(&ptv->lock);
 		assert(ptv->nr_used < ptv->nr_counters);
 		p = &ptv->data[(ptv->nr_used++) * ptv->data_size];
-		ret = pthread_setspecific(ptv->key, p);
+		ret = -pthread_setspecific(ptv->key, p);
 		if (ret)
 			goto out_unlock;
 		pthread_mutex_unlock(&ptv->lock);
diff --git a/scrub/counter.c b/scrub/counter.c
index 1bb726dc..6d91eb6e 100644
--- a/scrub/counter.c
+++ b/scrub/counter.c
@@ -38,7 +38,7 @@ ptcounter_alloc(
 	p = malloc(sizeof(struct ptcounter));
 	if (!p)
 		return errno;
-	ret = ptvar_alloc(nr, sizeof(uint64_t), &p->var);
+	ret = -ptvar_alloc(nr, sizeof(uint64_t), &p->var);
 	if (ret) {
 		free(p);
 		return ret;
@@ -67,7 +67,7 @@ ptcounter_add(
 
 	p = ptvar_get(ptc->var, &ret);
 	if (ret)
-		return ret;
+		return -ret;
 	*p += nr;
 	return 0;
 }
@@ -92,5 +92,5 @@ ptcounter_value(
 	uint64_t		*sum)
 {
 	*sum = 0;
-	return ptvar_foreach(ptc->var, ptcounter_val_helper, sum);
+	return -ptvar_foreach(ptc->var, ptcounter_val_helper, sum);
 }
diff --git a/scrub/descr.c b/scrub/descr.c
index 7f65a4e0..a863c065 100644
--- a/scrub/descr.c
+++ b/scrub/descr.c
@@ -89,7 +89,7 @@ descr_init_phase(
 	int			ret;
 
 	assert(descr_ptvar == NULL);
-	ret = ptvar_alloc(nr_threads, DESCR_BUFSZ, &descr_ptvar);
+	ret = -ptvar_alloc(nr_threads, DESCR_BUFSZ, &descr_ptvar);
 	if (ret)
 		str_liberror(ctx, ret, _("creating description buffer"));
 
diff --git a/scrub/phase7.c b/scrub/phase7.c
index f25a8765..f8410439 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -39,8 +39,8 @@ count_block_summary(
 
 	counts = ptvar_get((struct ptvar *)arg, &ret);
 	if (ret) {
-		str_liberror(ctx, ret, _("retrieving summary counts"));
-		return ret;
+		str_liberror(ctx, -ret, _("retrieving summary counts"));
+		return -ret;
 	}
 	if (fsmap->fmr_device == ctx->fsinfo.fs_logdev)
 		return 0;
@@ -135,7 +135,7 @@ phase7_func(
 		return error;
 	}
 
-	error = ptvar_alloc(scrub_nproc(ctx), sizeof(struct summary_counts),
+	error = -ptvar_alloc(scrub_nproc(ctx), sizeof(struct summary_counts),
 			&ptvar);
 	if (error) {
 		str_liberror(ctx, error, _("setting up block counter"));
@@ -146,7 +146,7 @@ phase7_func(
 	error = scrub_scan_all_spacemaps(ctx, count_block_summary, ptvar);
 	if (error)
 		goto out_free;
-	error = ptvar_foreach(ptvar, add_summaries, &totalcount);
+	error = -ptvar_foreach(ptvar, add_summaries, &totalcount);
 	if (error) {
 		str_liberror(ctx, error, _("counting blocks"));
 		goto out_free;
diff --git a/scrub/read_verify.c b/scrub/read_verify.c
index 414d25a6..b7e9eb91 100644
--- a/scrub/read_verify.c
+++ b/scrub/read_verify.c
@@ -115,7 +115,7 @@ read_verify_pool_alloc(
 	rvp->disk = disk;
 	rvp->ioerr_fn = ioerr_fn;
 	rvp->errors_seen = 0;
-	ret = ptvar_alloc(submitter_threads, sizeof(struct read_verify),
+	ret = -ptvar_alloc(submitter_threads, sizeof(struct read_verify),
 			&rvp->rvstate);
 	if (ret)
 		goto out_counter;
@@ -334,7 +334,7 @@ read_verify_schedule_io(
 
 	rv = ptvar_get(rvp->rvstate, &ret);
 	if (ret)
-		return ret;
+		return -ret;
 	req_end = start + length;
 	rv_end = rv->io_start + rv->io_length;
 
@@ -382,7 +382,7 @@ force_one_io(
 	if (rv->io_length == 0)
 		return 0;
 
-	return read_verify_queue(rvp, rv);
+	return -read_verify_queue(rvp, rv);
 }
 
 /* Force any stashed IOs into the verifier. */
@@ -392,7 +392,7 @@ read_verify_force_io(
 {
 	assert(rvp->readbuf);
 
-	return ptvar_foreach(rvp->rvstate, force_one_io, rvp);
+	return -ptvar_foreach(rvp->rvstate, force_one_io, rvp);
 }
 
 /* How many bytes has this process verified? */

[PATCH 6/7] libfrog: convert scrub.c functions to negative error codes

[Darrick J. Wong]

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

Convert libfrog functions to return negative error codes like libxfs
does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libfrog/scrub.c |    9 ++-
 scrub/scrub.c   |  198 ++++++++++++++++++++++++++++---------------------------
 2 files changed, 108 insertions(+), 99 deletions(-)


diff --git a/libfrog/scrub.c b/libfrog/scrub.c
index e9671da2..d900bf2a 100644
--- a/libfrog/scrub.c
+++ b/libfrog/scrub.c
@@ -137,10 +137,17 @@ const struct xfrog_scrub_descr xfrog_scrubbers[XFS_SCRUB_TYPE_NR] = {
 	},
 };
 
+/* Invoke the scrub ioctl.  Returns zero or negative error code. */
 int
 xfrog_scrub_metadata(
 	struct xfs_fd			*xfd,
 	struct xfs_scrub_metadata	*meta)
 {
-	return ioctl(xfd->fd, XFS_IOC_SCRUB_METADATA, meta);
+	int				ret;
+
+	ret = ioctl(xfd->fd, XFS_IOC_SCRUB_METADATA, meta);
+	if (ret)
+		return -errno;
+
+	return 0;
 }
diff --git a/scrub/scrub.c b/scrub/scrub.c
index 5eb1b276..f7677499 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -127,7 +127,6 @@ xfs_check_metadata(
 {
 	DEFINE_DESCR(dsc, ctx, format_scrub_descr);
 	unsigned int			tries = 0;
-	int				code;
 	int				error;
 
 	assert(!debug_tweak_on("XFS_SCRUB_NO_KERNEL"));
@@ -136,41 +135,41 @@ xfs_check_metadata(
 
 	dbg_printf("check %s flags %xh\n", descr_render(&dsc), meta->sm_flags);
 retry:
-	error = xfrog_scrub_metadata(&ctx->mnt, meta);
+	error = -xfrog_scrub_metadata(&ctx->mnt, meta);
 	if (debug_tweak_on("XFS_SCRUB_FORCE_REPAIR") && !error)
 		meta->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
-	if (error) {
-		code = errno;
-		switch (code) {
-		case ENOENT:
-			/* Metadata not present, just skip it. */
-			return CHECK_DONE;
-		case ESHUTDOWN:
-			/* FS already crashed, give up. */
-			str_info(ctx, descr_render(&dsc),
+	switch (error) {
+	case 0:
+		/* No operational errors encountered. */
+		break;
+	case ENOENT:
+		/* Metadata not present, just skip it. */
+		return CHECK_DONE;
+	case ESHUTDOWN:
+		/* FS already crashed, give up. */
+		str_info(ctx, descr_render(&dsc),
 _("Filesystem is shut down, aborting."));
-			return CHECK_ABORT;
-		case EIO:
-		case ENOMEM:
-			/* Abort on I/O errors or insufficient memory. */
-			str_errno(ctx, descr_render(&dsc));
-			return CHECK_ABORT;
-		case EDEADLOCK:
-		case EBUSY:
-		case EFSBADCRC:
-		case EFSCORRUPTED:
-			/*
-			 * The first two should never escape the kernel,
-			 * and the other two should be reported via sm_flags.
-			 */
-			str_info(ctx, descr_render(&dsc),
-_("Kernel bug!  errno=%d"), code);
-			/* fall through */
-		default:
-			/* Operational error. */
-			str_errno(ctx, descr_render(&dsc));
-			return CHECK_DONE;
-		}
+		return CHECK_ABORT;
+	case EIO:
+	case ENOMEM:
+		/* Abort on I/O errors or insufficient memory. */
+		str_errno(ctx, descr_render(&dsc));
+		return CHECK_ABORT;
+	case EDEADLOCK:
+	case EBUSY:
+	case EFSBADCRC:
+	case EFSCORRUPTED:
+		/*
+		 * The first two should never escape the kernel,
+		 * and the other two should be reported via sm_flags.
+		 */
+		str_info(ctx, descr_render(&dsc), _("Kernel bug!  errno=%d"),
+				error);
+		/* fall through */
+	default:
+		/* Operational error. */
+		str_errno(ctx, descr_render(&dsc));
+		return CHECK_DONE;
 	}
 
 	/*
@@ -578,10 +577,10 @@ __xfs_scrub_test(
 	meta.sm_type = type;
 	if (repair)
 		meta.sm_flags |= XFS_SCRUB_IFLAG_REPAIR;
-	error = xfrog_scrub_metadata(&ctx->mnt, &meta);
-	if (!error)
+	error = -xfrog_scrub_metadata(&ctx->mnt, &meta);
+	switch (error) {
+	case 0:
 		return true;
-	switch (errno) {
 	case EROFS:
 		str_info(ctx, ctx->mntpoint,
 _("Filesystem is mounted read-only; cannot proceed."));
@@ -707,74 +706,77 @@ xfs_repair_metadata(
 		str_info(ctx, descr_render(&dsc),
 				_("Attempting optimization."));
 
-	error = xfrog_scrub_metadata(&ctx->mnt, &meta);
-	if (error) {
-		switch (errno) {
-		case EDEADLOCK:
-		case EBUSY:
-			/* Filesystem is busy, try again later. */
-			if (debug || verbose)
-				str_info(ctx, descr_render(&dsc),
-_("Filesystem is busy, deferring repair."));
-			return CHECK_RETRY;
-		case ESHUTDOWN:
-			/* Filesystem is already shut down, abort. */
+	error = -xfrog_scrub_metadata(&ctx->mnt, &meta);
+	switch (error) {
+	case 0:
+		/* No operational errors encountered. */
+		break;
+	case EDEADLOCK:
+	case EBUSY:
+		/* Filesystem is busy, try again later. */
+		if (debug || verbose)
 			str_info(ctx, descr_render(&dsc),
+_("Filesystem is busy, deferring repair."));
+		return CHECK_RETRY;
+	case ESHUTDOWN:
+		/* Filesystem is already shut down, abort. */
+		str_info(ctx, descr_render(&dsc),
 _("Filesystem is shut down, aborting."));
-			return CHECK_ABORT;
-		case ENOTTY:
-		case EOPNOTSUPP:
-			/*
-			 * If we're in no-complain mode, requeue the check for
-			 * later.  It's possible that an error in another
-			 * component caused us to flag an error in this
-			 * component.  Even if the kernel didn't think it
-			 * could fix this, it's at least worth trying the scan
-			 * again to see if another repair fixed it.
-			 */
-			if (!(repair_flags & XRM_COMPLAIN_IF_UNFIXED))
-				return CHECK_RETRY;
-			/*
-			 * If we forced repairs or this is a preen, don't
-			 * error out if the kernel doesn't know how to fix.
-			 */
-			if (is_unoptimized(&oldm) ||
-			    debug_tweak_on("XFS_SCRUB_FORCE_REPAIR"))
-				return CHECK_DONE;
-			/* fall through */
-		case EINVAL:
-			/* Kernel doesn't know how to repair this? */
-			str_error(ctx, descr_render(&dsc),
-_("Don't know how to fix; offline repair required."));
+		return CHECK_ABORT;
+	case ENOTTY:
+	case EOPNOTSUPP:
+		/*
+		 * If we're in no-complain mode, requeue the check for
+		 * later.  It's possible that an error in another
+		 * component caused us to flag an error in this
+		 * component.  Even if the kernel didn't think it
+		 * could fix this, it's at least worth trying the scan
+		 * again to see if another repair fixed it.
+		 */
+		if (!(repair_flags & XRM_COMPLAIN_IF_UNFIXED))
+			return CHECK_RETRY;
+		/*
+		 * If we forced repairs or this is a preen, don't
+		 * error out if the kernel doesn't know how to fix.
+		 */
+		if (is_unoptimized(&oldm) ||
+		    debug_tweak_on("XFS_SCRUB_FORCE_REPAIR"))
 			return CHECK_DONE;
-		case EROFS:
-			/* Read-only filesystem, can't fix. */
-			if (verbose || debug || needs_repair(&oldm))
-				str_info(ctx, descr_render(&dsc),
+		/* fall through */
+	case EINVAL:
+		/* Kernel doesn't know how to repair this? */
+		str_error(ctx,
+_("%s: Don't know how to fix; offline repair required."),
+				descr_render(&dsc));
+		return CHECK_DONE;
+	case EROFS:
+		/* Read-only filesystem, can't fix. */
+		if (verbose || debug || needs_repair(&oldm))
+			str_info(ctx, descr_render(&dsc),
 _("Read-only filesystem; cannot make changes."));
+		return CHECK_DONE;
+	case ENOENT:
+		/* Metadata not present, just skip it. */
+		return CHECK_DONE;
+	case ENOMEM:
+	case ENOSPC:
+		/* Don't care if preen fails due to low resources. */
+		if (is_unoptimized(&oldm) && !needs_repair(&oldm))
 			return CHECK_DONE;
-		case ENOENT:
-			/* Metadata not present, just skip it. */
-			return CHECK_DONE;
-		case ENOMEM:
-		case ENOSPC:
-			/* Don't care if preen fails due to low resources. */
-			if (is_unoptimized(&oldm) && !needs_repair(&oldm))
-				return CHECK_DONE;
-			/* fall through */
-		default:
-			/*
-			 * Operational error.  If the caller doesn't want us
-			 * to complain about repair failures, tell the caller
-			 * to requeue the repair for later and don't say a
-			 * thing.  Otherwise, print error and bail out.
-			 */
-			if (!(repair_flags & XRM_COMPLAIN_IF_UNFIXED))
-				return CHECK_RETRY;
-			str_errno(ctx, descr_render(&dsc));
-			return CHECK_DONE;
-		}
+		/* fall through */
+	default:
+		/*
+		 * Operational error.  If the caller doesn't want us
+		 * to complain about repair failures, tell the caller
+		 * to requeue the repair for later and don't say a
+		 * thing.  Otherwise, print error and bail out.
+		 */
+		if (!(repair_flags & XRM_COMPLAIN_IF_UNFIXED))
+			return CHECK_RETRY;
+		str_liberror(ctx, error, descr_render(&dsc));
+		return CHECK_DONE;
 	}
+
 	if (repair_flags & XRM_COMPLAIN_IF_UNFIXED)
 		xfs_scrub_warn_incomplete_scrub(ctx, &dsc, &meta);
 	if (needs_repair(&meta)) {

[PATCH 7/7] libfrog: convert workqueue.c functions to negative error codes

[Darrick J. Wong]

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

Convert libfrog functions to return negative error codes like libxfs
does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libfrog/workqueue.c |   25 +++++++++++++------------
 repair/threads.c    |    6 +++---
 scrub/fscounters.c  |    6 +++---
 scrub/inodes.c      |    6 +++---
 scrub/phase2.c      |    8 ++++----
 scrub/phase4.c      |    6 +++---
 scrub/read_verify.c |    6 +++---
 scrub/spacemap.c    |   10 +++++-----
 scrub/vfs.c         |    6 +++---
 9 files changed, 40 insertions(+), 39 deletions(-)


diff --git a/libfrog/workqueue.c b/libfrog/workqueue.c
index 07f11a7b..2269c413 100644
--- a/libfrog/workqueue.c
+++ b/libfrog/workqueue.c
@@ -56,7 +56,7 @@ workqueue_thread(void *arg)
 	return NULL;
 }
 
-/* Allocate a work queue and threads. */
+/* Allocate a work queue and threads.  Returns zero or negative error code. */
 int
 workqueue_create(
 	struct workqueue	*wq,
@@ -67,10 +67,10 @@ workqueue_create(
 	int			err = 0;
 
 	memset(wq, 0, sizeof(*wq));
-	err = pthread_cond_init(&wq->wakeup, NULL);
+	err = -pthread_cond_init(&wq->wakeup, NULL);
 	if (err)
 		return err;
-	err = pthread_mutex_init(&wq->lock, NULL);
+	err = -pthread_mutex_init(&wq->lock, NULL);
 	if (err)
 		goto out_cond;
 
@@ -78,14 +78,14 @@ workqueue_create(
 	wq->thread_count = nr_workers;
 	wq->threads = malloc(nr_workers * sizeof(pthread_t));
 	if (!wq->threads) {
-		err = errno;
+		err = -errno;
 		goto out_mutex;
 	}
 	wq->terminate = false;
 	wq->terminated = false;
 
 	for (i = 0; i < nr_workers; i++) {
-		err = pthread_create(&wq->threads[i], NULL, workqueue_thread,
+		err = -pthread_create(&wq->threads[i], NULL, workqueue_thread,
 				wq);
 		if (err)
 			break;
@@ -107,8 +107,9 @@ workqueue_create(
 }
 
 /*
- * Create a work item consisting of a function and some arguments and
- * schedule the work item to be run via the thread pool.
+ * Create a work item consisting of a function and some arguments and schedule
+ * the work item to be run via the thread pool.  Returns zero or a negative
+ * error code.
  */
 int
 workqueue_add(
@@ -129,7 +130,7 @@ workqueue_add(
 
 	wi = malloc(sizeof(struct workqueue_item));
 	if (!wi)
-		return errno;
+		return -errno;
 
 	wi->function = func;
 	wi->index = index;
@@ -141,7 +142,7 @@ workqueue_add(
 	pthread_mutex_lock(&wq->lock);
 	if (wq->next_item == NULL) {
 		assert(wq->item_count == 0);
-		ret = pthread_cond_signal(&wq->wakeup);
+		ret = -pthread_cond_signal(&wq->wakeup);
 		if (ret)
 			goto out_item;
 		wq->next_item = wi;
@@ -160,7 +161,7 @@ workqueue_add(
 
 /*
  * Wait for all pending work items to be processed and tear down the
- * workqueue thread pool.
+ * workqueue thread pool.  Returns zero or a negative error code.
  */
 int
 workqueue_terminate(
@@ -173,12 +174,12 @@ workqueue_terminate(
 	wq->terminate = true;
 	pthread_mutex_unlock(&wq->lock);
 
-	ret = pthread_cond_broadcast(&wq->wakeup);
+	ret = -pthread_cond_broadcast(&wq->wakeup);
 	if (ret)
 		return ret;
 
 	for (i = 0; i < wq->thread_count; i++) {
-		ret = pthread_join(wq->threads[i], NULL);
+		ret = -pthread_join(wq->threads[i], NULL);
 		if (ret)
 			return ret;
 	}
diff --git a/repair/threads.c b/repair/threads.c
index 9b7241e3..45ca2dd5 100644
--- a/repair/threads.c
+++ b/repair/threads.c
@@ -31,7 +31,7 @@ create_work_queue(
 {
 	int			err;
 
-	err = workqueue_create(wq, mp, nworkers);
+	err = -workqueue_create(wq, mp, nworkers);
 	if (err)
 		do_error(_("cannot create worker threads, error = [%d] %s\n"),
 				err, strerror(err));
@@ -46,7 +46,7 @@ queue_work(
 {
 	int			err;
 
-	err = workqueue_add(wq, func, agno, arg);
+	err = -workqueue_add(wq, func, agno, arg);
 	if (err)
 		do_error(_("cannot allocate worker item, error = [%d] %s\n"),
 				err, strerror(err));
@@ -58,7 +58,7 @@ destroy_work_queue(
 {
 	int			err;
 
-	err = workqueue_terminate(wq);
+	err = -workqueue_terminate(wq);
 	if (err)
 		do_error(_("cannot terminate worker item, error = [%d] %s\n"),
 				err, strerror(err));
diff --git a/scrub/fscounters.c b/scrub/fscounters.c
index a6b62f34..f9d64f8c 100644
--- a/scrub/fscounters.c
+++ b/scrub/fscounters.c
@@ -86,18 +86,18 @@ scrub_count_all_inodes(
 	if (!ci)
 		return errno;
 
-	ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
+	ret = -workqueue_create(&wq, (struct xfs_mount *)ctx,
 			scrub_nproc_workqueue(ctx));
 	if (ret)
 		goto out_free;
 
 	for (agno = 0; agno < ctx->mnt.fsgeom.agcount && !ci->error; agno++) {
-		ret = workqueue_add(&wq, count_ag_inodes, agno, ci);
+		ret = -workqueue_add(&wq, count_ag_inodes, agno, ci);
 		if (ret)
 			break;
 	}
 
-	ret2 = workqueue_terminate(&wq);
+	ret2 = -workqueue_terminate(&wq);
 	if (!ret && ret2)
 		ret = ret2;
 	workqueue_destroy(&wq);
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 90d66c45..a0f62250 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -223,7 +223,7 @@ scrub_scan_all_inodes(
 	struct workqueue	wq;
 	int			ret;
 
-	ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
+	ret = -workqueue_create(&wq, (struct xfs_mount *)ctx,
 			scrub_nproc_workqueue(ctx));
 	if (ret) {
 		str_liberror(ctx, ret, _("creating bulkstat workqueue"));
@@ -231,7 +231,7 @@ scrub_scan_all_inodes(
 	}
 
 	for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
-		ret = workqueue_add(&wq, scan_ag_inodes, agno, &si);
+		ret = -workqueue_add(&wq, scan_ag_inodes, agno, &si);
 		if (ret) {
 			si.aborted = true;
 			str_liberror(ctx, ret, _("queueing bulkstat work"));
@@ -239,7 +239,7 @@ scrub_scan_all_inodes(
 		}
 	}
 
-	ret = workqueue_terminate(&wq);
+	ret = -workqueue_terminate(&wq);
 	if (ret) {
 		si.aborted = true;
 		str_liberror(ctx, ret, _("finishing bulkstat work"));
diff --git a/scrub/phase2.c b/scrub/phase2.c
index 45e0d712..c40d9d3b 100644
--- a/scrub/phase2.c
+++ b/scrub/phase2.c
@@ -128,7 +128,7 @@ phase2_func(
 	bool			aborted = false;
 	int			ret, ret2;
 
-	ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
+	ret = -workqueue_create(&wq, (struct xfs_mount *)ctx,
 			scrub_nproc_workqueue(ctx));
 	if (ret) {
 		str_liberror(ctx, ret, _("creating scrub workqueue"));
@@ -149,7 +149,7 @@ phase2_func(
 		goto out;
 
 	for (agno = 0; !aborted && agno < ctx->mnt.fsgeom.agcount; agno++) {
-		ret = workqueue_add(&wq, scan_ag_metadata, agno, &aborted);
+		ret = -workqueue_add(&wq, scan_ag_metadata, agno, &aborted);
 		if (ret) {
 			str_liberror(ctx, ret, _("queueing per-AG scrub work"));
 			goto out;
@@ -159,14 +159,14 @@ phase2_func(
 	if (aborted)
 		goto out;
 
-	ret = workqueue_add(&wq, scan_fs_metadata, 0, &aborted);
+	ret = -workqueue_add(&wq, scan_fs_metadata, 0, &aborted);
 	if (ret) {
 		str_liberror(ctx, ret, _("queueing per-FS scrub work"));
 		goto out;
 	}
 
 out:
-	ret2 = workqueue_terminate(&wq);
+	ret2 = -workqueue_terminate(&wq);
 	if (ret2) {
 		str_liberror(ctx, ret2, _("finishing scrub work"));
 		if (!ret && ret2)
diff --git a/scrub/phase4.c b/scrub/phase4.c
index 50c2dbb8..b4fc6406 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -70,7 +70,7 @@ repair_everything(
 	bool				aborted = false;
 	int				ret;
 
-	ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
+	ret = -workqueue_create(&wq, (struct xfs_mount *)ctx,
 			scrub_nproc_workqueue(ctx));
 	if (ret) {
 		str_liberror(ctx, ret, _("creating repair workqueue"));
@@ -80,14 +80,14 @@ repair_everything(
 		if (action_list_length(&ctx->action_lists[agno]) == 0)
 			continue;
 
-		ret = workqueue_add(&wq, repair_ag, agno, &aborted);
+		ret = -workqueue_add(&wq, repair_ag, agno, &aborted);
 		if (ret) {
 			str_liberror(ctx, ret, _("queueing repair work"));
 			break;
 		}
 	}
 
-	ret = workqueue_terminate(&wq);
+	ret = -workqueue_terminate(&wq);
 	if (ret)
 		str_liberror(ctx, ret, _("finishing repair work"));
 	workqueue_destroy(&wq);
diff --git a/scrub/read_verify.c b/scrub/read_verify.c
index b7e9eb91..58e4e951 100644
--- a/scrub/read_verify.c
+++ b/scrub/read_verify.c
@@ -119,7 +119,7 @@ read_verify_pool_alloc(
 			&rvp->rvstate);
 	if (ret)
 		goto out_counter;
-	ret = workqueue_create(&rvp->wq, (struct xfs_mount *)rvp,
+	ret = -workqueue_create(&rvp->wq, (struct xfs_mount *)rvp,
 			verifier_threads == 1 ? 0 : verifier_threads);
 	if (ret)
 		goto out_rvstate;
@@ -152,7 +152,7 @@ int
 read_verify_pool_flush(
 	struct read_verify_pool		*rvp)
 {
-	return workqueue_terminate(&rvp->wq);
+	return -workqueue_terminate(&rvp->wq);
 }
 
 /* Finish up any read verification work and tear it down. */
@@ -299,7 +299,7 @@ read_verify_queue(
 
 	memcpy(tmp, rv, sizeof(*tmp));
 
-	ret = workqueue_add(&rvp->wq, read_verify, 0, tmp);
+	ret = -workqueue_add(&rvp->wq, read_verify, 0, tmp);
 	if (ret) {
 		free(tmp);
 		rvp->errors_seen = ret;
diff --git a/scrub/spacemap.c b/scrub/spacemap.c
index e56f090d..d427049f 100644
--- a/scrub/spacemap.c
+++ b/scrub/spacemap.c
@@ -203,14 +203,14 @@ scrub_scan_all_spacemaps(
 	xfs_agnumber_t		agno;
 	int			ret;
 
-	ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
+	ret = -workqueue_create(&wq, (struct xfs_mount *)ctx,
 			scrub_nproc_workqueue(ctx));
 	if (ret) {
 		str_liberror(ctx, ret, _("creating fsmap workqueue"));
 		return ret;
 	}
 	if (ctx->fsinfo.fs_rt) {
-		ret = workqueue_add(&wq, scan_rt_rmaps,
+		ret = -workqueue_add(&wq, scan_rt_rmaps,
 				ctx->mnt.fsgeom.agcount + 1, &sbx);
 		if (ret) {
 			sbx.aborted = true;
@@ -219,7 +219,7 @@ scrub_scan_all_spacemaps(
 		}
 	}
 	if (ctx->fsinfo.fs_log) {
-		ret = workqueue_add(&wq, scan_log_rmaps,
+		ret = -workqueue_add(&wq, scan_log_rmaps,
 				ctx->mnt.fsgeom.agcount + 2, &sbx);
 		if (ret) {
 			sbx.aborted = true;
@@ -228,7 +228,7 @@ scrub_scan_all_spacemaps(
 		}
 	}
 	for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
-		ret = workqueue_add(&wq, scan_ag_rmaps, agno, &sbx);
+		ret = -workqueue_add(&wq, scan_ag_rmaps, agno, &sbx);
 		if (ret) {
 			sbx.aborted = true;
 			str_liberror(ctx, ret, _("queueing per-AG fsmap work"));
@@ -236,7 +236,7 @@ scrub_scan_all_spacemaps(
 		}
 	}
 out:
-	ret = workqueue_terminate(&wq);
+	ret = -workqueue_terminate(&wq);
 	if (ret) {
 		sbx.aborted = true;
 		str_liberror(ctx, ret, _("finishing fsmap work"));
diff --git a/scrub/vfs.c b/scrub/vfs.c
index c807c9b9..76920923 100644
--- a/scrub/vfs.c
+++ b/scrub/vfs.c
@@ -97,7 +97,7 @@ queue_subdir(
 	new_sftd->rootdir = is_rootdir;
 
 	inc_nr_dirs(sft);
-	error = workqueue_add(wq, scan_fs_dir, 0, new_sftd);
+	error = -workqueue_add(wq, scan_fs_dir, 0, new_sftd);
 	if (error) {
 		dec_nr_dirs(sft);
 		str_liberror(ctx, error, _("queueing directory scan work"));
@@ -242,7 +242,7 @@ scan_fs_tree(
 		goto out_mutex;
 	}
 
-	ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
+	ret = -workqueue_create(&wq, (struct xfs_mount *)ctx,
 			scrub_nproc_workqueue(ctx));
 	if (ret) {
 		str_liberror(ctx, ret, _("creating directory scan workqueue"));
@@ -268,7 +268,7 @@ scan_fs_tree(
 	assert(sft.nr_dirs == 0);
 	pthread_mutex_unlock(&sft.lock);
 
-	ret = workqueue_terminate(&wq);
+	ret = -workqueue_terminate(&wq);
 	if (ret) {
 		str_liberror(ctx, ret, _("finishing directory scan work"));
 		goto out_wq;

Re: [PATCH 1/7] libfrog: print library errors

[Christoph Hellwig]

Looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/7] libfrog: convert bitmap.c to negative error codes

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/7] libfrog: convert fsgeom.c functions to negative error codes

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/7] libfrog: convert bulkstat.c functions to negative error codes

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 5/7] libfrog: convert ptvar.c functions to negative error codes

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 6/7] libfrog: convert scrub.c functions to negative error codes

[Christoph Hellwig]

On Wed, Sep 25, 2019 at 02:40:38PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Convert libfrog functions to return negative error codes like libxfs
> does.

Looks fine, although in the places where you touch the whole switch
statement I wonder why we just switch those to negative errnos instead
of inverting and then checking for positive values..

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 7/7] libfrog: convert workqueue.c functions to negative error codes

[Christoph Hellwig]

Looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 6/7] libfrog: convert scrub.c functions to negative error codes

[Darrick J. Wong]

On Mon, Sep 30, 2019 at 12:54:41AM -0700, Christoph Hellwig wrote:
> On Wed, Sep 25, 2019 at 02:40:38PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Convert libfrog functions to return negative error codes like libxfs
> > does.
> 
> Looks fine, although in the places where you touch the whole switch
> statement I wonder why we just switch those to negative errnos instead
> of inverting and then checking for positive values..

This series only converts libfrog functions and callsites.  Next I'll
start converting utilities one by one, but there are already too many
patches out so I'm holding off until that after a series or two get
merged.

> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks for the review!

--D

[PATCH 3/7] libfrog: convert fsgeom.c functions to negative error codes

[Darrick J. Wong]

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

Convert libfrog functions to return negative error codes like libxfs
does.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fsr/xfs_fsr.c       |    4 ++--
 growfs/xfs_growfs.c |    4 ++--
 io/bmap.c           |    2 +-
 io/bulkstat.c       |    6 +++---
 io/fsmap.c          |    4 ++--
 io/open.c           |    2 +-
 io/stat.c           |    2 +-
 libfrog/bulkstat.c  |    2 +-
 libfrog/fsgeom.c    |   18 +++++++++---------
 quota/free.c        |    2 +-
 quota/quot.c        |    2 +-
 repair/xfs_repair.c |    2 +-
 rtcp/xfs_rtcp.c     |    2 +-
 scrub/phase1.c      |    4 ++--
 spaceman/file.c     |    2 +-
 spaceman/health.c   |    2 +-
 16 files changed, 30 insertions(+), 30 deletions(-)


diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index af5d6169..3e9ba27c 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -602,7 +602,7 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
 		return -1;
 	}
 
-	ret = xfd_open(&fsxfd, mntdir, O_RDONLY);
+	ret = -xfd_open(&fsxfd, mntdir, O_RDONLY);
 	if (ret) {
 		fsrprintf(_("unable to open XFS file: %s: %s\n"),
 		          mntdir, strerror(ret));
@@ -748,7 +748,7 @@ fsrfile(
 	 * Need to open something on the same filesystem as the
 	 * file.  Open the parent.
 	 */
-	error = xfd_open(&fsxfd, getparent(fname), O_RDONLY);
+	error = -xfd_open(&fsxfd, getparent(fname), O_RDONLY);
 	if (error) {
 		fsrprintf(_("unable to open sys handle for XFS file %s: %s\n"),
 			fname, strerror(error));
diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index eab15984..d1908046 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -166,7 +166,7 @@ main(int argc, char **argv)
 	}
 
 	/* get the current filesystem size & geometry */
-	ret = xfrog_geometry(ffd, &geo);
+	ret = -xfrog_geometry(ffd, &geo);
 	if (ret) {
 		fprintf(stderr,
 	_("%s: cannot determine geometry of filesystem mounted at %s: %s\n"),
@@ -352,7 +352,7 @@ main(int argc, char **argv)
 		}
 	}
 
-	ret = xfrog_geometry(ffd, &ngeo);
+	ret = -xfrog_geometry(ffd, &ngeo);
 	if (ret) {
 		fprintf(stderr, _("%s: XFS_IOC_FSGEOMETRY xfsctl failed: %s\n"),
 			progname, strerror(ret));
diff --git a/io/bmap.c b/io/bmap.c
index cf4ea12b..f838840e 100644
--- a/io/bmap.c
+++ b/io/bmap.c
@@ -106,7 +106,7 @@ bmap_f(
 		bmv_iflags &= ~(BMV_IF_PREALLOC|BMV_IF_NO_DMAPI_READ);
 
 	if (vflag) {
-		c = xfrog_geometry(file->fd, &fsgeo);
+		c = -xfrog_geometry(file->fd, &fsgeo);
 		if (c) {
 			fprintf(stderr,
 				_("%s: can't get geometry [\"%s\"]: %s\n"),
diff --git a/io/bulkstat.c b/io/bulkstat.c
index 9641370b..b081567f 100644
--- a/io/bulkstat.c
+++ b/io/bulkstat.c
@@ -163,7 +163,7 @@ bulkstat_f(
 		return 0;
 	}
 
-	ret = xfd_prepare_geometry(&xfd);
+	ret = -xfd_prepare_geometry(&xfd);
 	if (ret) {
 		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
@@ -271,7 +271,7 @@ bulkstat_single_f(
 		}
 	}
 
-	ret = xfd_prepare_geometry(&xfd);
+	ret = -xfd_prepare_geometry(&xfd);
 	if (ret) {
 		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
@@ -419,7 +419,7 @@ inumbers_f(
 		return 0;
 	}
 
-	ret = xfd_prepare_geometry(&xfd);
+	ret = -xfd_prepare_geometry(&xfd);
 	if (ret) {
 		xfrog_perror(ret, "xfd_prepare_geometry");
 		exitcode = 1;
diff --git a/io/fsmap.c b/io/fsmap.c
index 12ec1e44..feacb264 100644
--- a/io/fsmap.c
+++ b/io/fsmap.c
@@ -448,11 +448,11 @@ fsmap_f(
 	}
 
 	if (vflag) {
-		c = xfrog_geometry(file->fd, &fsgeo);
+		c = -xfrog_geometry(file->fd, &fsgeo);
 		if (c) {
 			fprintf(stderr,
 				_("%s: can't get geometry [\"%s\"]: %s\n"),
-				progname, file->name, strerror(errno));
+				progname, file->name, strerror(c));
 			exitcode = 1;
 			return 0;
 		}
diff --git a/io/open.c b/io/open.c
index a5192e87..464bcad9 100644
--- a/io/open.c
+++ b/io/open.c
@@ -124,7 +124,7 @@ openfile(
 	} else {
 		int	ret;
 
-		ret = xfrog_geometry(fd, geom);
+		ret = -xfrog_geometry(fd, geom);
 		if (ret) {
 			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 			close(fd);
diff --git a/io/stat.c b/io/stat.c
index db335780..d125a0f7 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -197,7 +197,7 @@ statfs_f(
 	}
 	if (file->flags & IO_FOREIGN)
 		return 0;
-	ret = xfrog_geometry(file->fd, &fsgeo);
+	ret = -xfrog_geometry(file->fd, &fsgeo);
 	if (ret) {
 		xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 	} else {
diff --git a/libfrog/bulkstat.c b/libfrog/bulkstat.c
index 538b5197..38d634f7 100644
--- a/libfrog/bulkstat.c
+++ b/libfrog/bulkstat.c
@@ -39,7 +39,7 @@ xfrog_bulkstat_prep_v1_emulation(
 	if (xfd->fsgeom.blocksize > 0)
 		return 0;
 
-	return xfd_prepare_geometry(xfd);
+	return -xfd_prepare_geometry(xfd);
 }
 
 /* Bulkstat a single inode using v5 ioctl. */
diff --git a/libfrog/fsgeom.c b/libfrog/fsgeom.c
index 3ea91e3f..19a4911f 100644
--- a/libfrog/fsgeom.c
+++ b/libfrog/fsgeom.c
@@ -69,7 +69,7 @@ xfs_report_geom(
 			(unsigned long long)geo->rtextents);
 }
 
-/* Try to obtain the xfs geometry.  On error returns a positive error code. */
+/* Try to obtain the xfs geometry.  On error returns a negative error code. */
 int
 xfrog_geometry(
 	int			fd,
@@ -91,12 +91,12 @@ xfrog_geometry(
 	if (!ret)
 		return 0;
 
-	return errno;
+	return -errno;
 }
 
 /*
  * Prepare xfs_fd structure for future ioctl operations by computing the xfs
- * geometry for @xfd->fd.  Returns zero or a positive error code.
+ * geometry for @xfd->fd.  Returns zero or a negative error code.
  */
 int
 xfd_prepare_geometry(
@@ -117,7 +117,7 @@ xfd_prepare_geometry(
 	return 0;
 }
 
-/* Open a file on an XFS filesystem.  Returns zero or a positive error code. */
+/* Open a file on an XFS filesystem.  Returns zero or a negative error code. */
 int
 xfd_open(
 	struct xfs_fd		*xfd,
@@ -128,7 +128,7 @@ xfd_open(
 
 	xfd->fd = open(pathname, flags);
 	if (xfd->fd < 0)
-		return errno;
+		return -errno;
 
 	ret = xfd_prepare_geometry(xfd);
 	if (ret) {
@@ -141,7 +141,7 @@ xfd_open(
 
 /*
  * Release any resources associated with this xfs_fd structure.  Returns zero
- * or a positive error code.
+ * or a negative error code.
  */
 int
 xfd_close(
@@ -155,12 +155,12 @@ xfd_close(
 	ret = close(xfd->fd);
 	xfd->fd = -1;
 	if (ret < 0)
-		return errno;
+		return -errno;
 
 	return 0;
 }
 
-/* Try to obtain an AG's geometry.  Returns zero or a positive error code. */
+/* Try to obtain an AG's geometry.  Returns zero or a negative error code. */
 int
 xfrog_ag_geometry(
 	int			fd,
@@ -172,6 +172,6 @@ xfrog_ag_geometry(
 	ageo->ag_number = agno;
 	ret = ioctl(fd, XFS_IOC_AG_GEOMETRY, ageo);
 	if (ret)
-		return errno;
+		return -errno;
 	return 0;
 }
diff --git a/quota/free.c b/quota/free.c
index 45ce2ceb..ea9c112f 100644
--- a/quota/free.c
+++ b/quota/free.c
@@ -69,7 +69,7 @@ mount_free_space_data(
 	}
 
 	if (!(mount->fs_flags & FS_FOREIGN)) {
-		ret = xfrog_geometry(fd, &fsgeo);
+		ret = -xfrog_geometry(fd, &fsgeo);
 		if (ret) {
 			xfrog_perror(ret, "XFS_IOC_FSGEOMETRY");
 			close(fd);
diff --git a/quota/quot.c b/quota/quot.c
index 0f69fabd..df3825f2 100644
--- a/quota/quot.c
+++ b/quota/quot.c
@@ -146,7 +146,7 @@ quot_bulkstat_mount(
 			*dp = NULL;
 	ndu[0] = ndu[1] = ndu[2] = 0;
 
-	ret = xfd_open(&fsxfd, fsdir, O_RDONLY);
+	ret = -xfd_open(&fsxfd, fsdir, O_RDONLY);
 	if (ret) {
 		xfrog_perror(ret, fsdir);
 		return;
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index 3338a7b8..9295673d 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -642,7 +642,7 @@ check_fs_vs_host_sectsize(
 
 	fd = libxfs_device_to_fd(x.ddev);
 
-	ret = xfrog_geometry(fd, &geom);
+	ret = -xfrog_geometry(fd, &geom);
 	if (ret) {
 		do_log(_("Cannot get host filesystem geometry.\n"
 	"Repair may fail if there is a sector size mismatch between\n"
diff --git a/rtcp/xfs_rtcp.c b/rtcp/xfs_rtcp.c
index a5737699..7c4197b1 100644
--- a/rtcp/xfs_rtcp.c
+++ b/rtcp/xfs_rtcp.c
@@ -378,7 +378,7 @@ xfsrtextsize( char *path)
 			progname, path, strerror(errno));
 		return -1;
 	}
-	rval = xfrog_geometry(fd, &geo);
+	rval = -xfrog_geometry(fd, &geo);
 	close(fd);
 	if (rval)
 		return -1;
diff --git a/scrub/phase1.c b/scrub/phase1.c
index e0382b04..6125d324 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -60,7 +60,7 @@ scrub_cleanup(
 	if (ctx->datadev)
 		disk_close(ctx->datadev);
 	fshandle_destroy();
-	error = xfd_close(&ctx->mnt);
+	error = -xfd_close(&ctx->mnt);
 	if (error)
 		str_liberror(ctx, error, _("closing mountpoint fd"));
 	fs_table_destroy();
@@ -84,7 +84,7 @@ phase1_func(
 	 * CAP_SYS_ADMIN, which we probably need to do anything fancy
 	 * with the (XFS driver) kernel.
 	 */
-	error = xfd_open(&ctx->mnt, ctx->mntpoint,
+	error = -xfd_open(&ctx->mnt, ctx->mntpoint,
 			O_RDONLY | O_NOATIME | O_DIRECTORY);
 	if (error) {
 		if (error == EPERM)
diff --git a/spaceman/file.c b/spaceman/file.c
index b7794328..eec7ee9f 100644
--- a/spaceman/file.c
+++ b/spaceman/file.c
@@ -52,7 +52,7 @@ openfile(
 	struct fs_path	*fsp;
 	int		ret;
 
-	ret = xfd_open(xfd, path, O_RDONLY);
+	ret = -xfd_open(xfd, path, O_RDONLY);
 	if (ret) {
 		if (ret == ENOTTY)
 			fprintf(stderr,
diff --git a/spaceman/health.c b/spaceman/health.c
index a0079bd7..c6d936fb 100644
--- a/spaceman/health.c
+++ b/spaceman/health.c
@@ -192,7 +192,7 @@ report_ag_sick(
 	char			descr[256];
 	int			ret;
 
-	ret = xfrog_ag_geometry(file->xfd.fd, agno, &ageo);
+	ret = -xfrog_ag_geometry(file->xfd.fd, agno, &ageo);
 	if (ret) {
 		xfrog_perror(ret, "ag_geometry");
 		return 1;