* [PATCH 01/12] libxfs: fix attr include mess
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
@ 2019-05-20 23:16 ` Darrick J. Wong
2019-05-21 16:30 ` Eric Sandeen
2019-05-20 23:16 ` [PATCH 02/12] libxfs: set m_finobt_nores when initializing library Darrick J. Wong
` (10 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:16 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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"
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 01/12] libxfs: fix attr include mess
2019-05-20 23:16 ` [PATCH 01/12] libxfs: fix attr include mess Darrick J. Wong
@ 2019-05-21 16:30 ` Eric Sandeen
0 siblings, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 16:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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"
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 02/12] libxfs: set m_finobt_nores when initializing library
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
2019-05-20 23:16 ` [PATCH 01/12] libxfs: fix attr include mess Darrick J. Wong
@ 2019-05-20 23:16 ` Darrick J. Wong
2019-05-21 16:33 ` Eric Sandeen
2019-05-20 23:17 ` [PATCH 03/12] libxfs: refactor online geometry queries Darrick J. Wong
` (9 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:16 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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);
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 02/12] libxfs: set m_finobt_nores when initializing library
2019-05-20 23:16 ` [PATCH 02/12] libxfs: set m_finobt_nores when initializing library Darrick J. Wong
@ 2019-05-21 16:33 ` Eric Sandeen
0 siblings, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 16:33 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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);
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 03/12] libxfs: refactor online geometry queries
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
2019-05-20 23:16 ` [PATCH 01/12] libxfs: fix attr include mess Darrick J. Wong
2019-05-20 23:16 ` [PATCH 02/12] libxfs: set m_finobt_nores when initializing library Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-21 16:38 ` Eric Sandeen
2019-05-20 23:17 ` [PATCH 04/12] libxfs: refactor open-coded bulkstat calls Darrick J. Wong
` (8 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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,
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 03/12] libxfs: refactor online geometry queries
2019-05-20 23:17 ` [PATCH 03/12] libxfs: refactor online geometry queries Darrick J. Wong
@ 2019-05-21 16:38 ` Eric Sandeen
2019-05-21 16:58 ` Darrick J. Wong
0 siblings, 1 reply; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 16:38 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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)
...
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 03/12] libxfs: refactor online geometry queries
2019-05-21 16:38 ` Eric Sandeen
@ 2019-05-21 16:58 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-21 16:58 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
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)
>
> ...
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 04/12] libxfs: refactor open-coded bulkstat calls
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (2 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 03/12] libxfs: refactor online geometry queries Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-20 23:17 ` [PATCH 05/12] libxfs: refactor open-coded INUMBERS calls Darrick J. Wong
` (7 subsequent siblings)
11 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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));
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 05/12] libxfs: refactor open-coded INUMBERS calls
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (3 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 04/12] libxfs: refactor open-coded bulkstat calls Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-20 23:17 ` [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t Darrick J. Wong
` (6 subsequent siblings)
11 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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:
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (4 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 05/12] libxfs: refactor open-coded INUMBERS calls Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-21 16:43 ` Eric Sandeen
2019-05-20 23:17 ` [PATCH 07/12] libfrog: fix bitmap return values Darrick J. Wong
` (5 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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);
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t
2019-05-20 23:17 ` [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t Darrick J. Wong
@ 2019-05-21 16:43 ` Eric Sandeen
2019-05-21 16:58 ` Darrick J. Wong
0 siblings, 1 reply; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 16:43 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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);
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t
2019-05-21 16:43 ` Eric Sandeen
@ 2019-05-21 16:58 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-21 16:58 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
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);
> >
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 07/12] libfrog: fix bitmap return values
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (5 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 06/12] misc: remove all use of xfs_fsop_geom_t Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-21 16:54 ` Eric Sandeen
2019-05-22 16:23 ` Eric Sandeen
2019-05-20 23:17 ` [PATCH 08/12] xfs_repair: refactor namecheck functions Darrick J. Wong
` (4 subsequent siblings)
11 siblings, 2 replies; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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;
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-20 23:17 ` [PATCH 07/12] libfrog: fix bitmap return values Darrick J. Wong
@ 2019-05-21 16:54 ` Eric Sandeen
2019-05-21 17:01 ` Darrick J. Wong
2019-05-22 16:23 ` Eric Sandeen
1 sibling, 1 reply; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 16:54 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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;
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-21 16:54 ` Eric Sandeen
@ 2019-05-21 17:01 ` Darrick J. Wong
2019-05-21 18:59 ` Eric Sandeen
0 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-21 17:01 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
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;
> >
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-21 17:01 ` Darrick J. Wong
@ 2019-05-21 18:59 ` Eric Sandeen
2019-05-21 19:19 ` Christoph Hellwig
0 siblings, 1 reply; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 18:59 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-21 18:59 ` Eric Sandeen
@ 2019-05-21 19:19 ` Christoph Hellwig
2019-05-21 19:20 ` Eric Sandeen
0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2019-05-21 19:19 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Darrick J. Wong, linux-xfs
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..
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-21 19:19 ` Christoph Hellwig
@ 2019-05-21 19:20 ` Eric Sandeen
2019-05-21 19:28 ` Christoph Hellwig
0 siblings, 1 reply; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Darrick J. Wong, linux-xfs
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-21 19:20 ` Eric Sandeen
@ 2019-05-21 19:28 ` Christoph Hellwig
2019-05-21 19:33 ` Eric Sandeen
0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2019-05-21 19:28 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, Darrick J. Wong, linux-xfs
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-21 19:28 ` Christoph Hellwig
@ 2019-05-21 19:33 ` Eric Sandeen
0 siblings, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Darrick J. Wong, linux-xfs
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 07/12] libfrog: fix bitmap return values
2019-05-20 23:17 ` [PATCH 07/12] libfrog: fix bitmap return values Darrick J. Wong
2019-05-21 16:54 ` Eric Sandeen
@ 2019-05-22 16:23 ` Eric Sandeen
1 sibling, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-22 16:23 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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;
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 08/12] xfs_repair: refactor namecheck functions
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (6 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 07/12] libfrog: fix bitmap return values Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-21 19:16 ` Eric Sandeen
2019-05-20 23:17 ` [PATCH 09/12] xfs_scrub: fix background-mode sleep throttling Darrick J. Wong
` (3 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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\": "),
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 08/12] xfs_repair: refactor namecheck functions
2019-05-20 23:17 ` [PATCH 08/12] xfs_repair: refactor namecheck functions Darrick J. Wong
@ 2019-05-21 19:16 ` Eric Sandeen
0 siblings, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:16 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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\": "),
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 09/12] xfs_scrub: fix background-mode sleep throttling
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (7 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 08/12] xfs_repair: refactor namecheck functions Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-21 19:18 ` Eric Sandeen
2019-05-20 23:17 ` [PATCH 10/12] mkfs: allow setting dax flag on root directory Darrick J. Wong
` (2 subsequent siblings)
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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);
}
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 09/12] xfs_scrub: fix background-mode sleep throttling
2019-05-20 23:17 ` [PATCH 09/12] xfs_scrub: fix background-mode sleep throttling Darrick J. Wong
@ 2019-05-21 19:18 ` Eric Sandeen
0 siblings, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:18 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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);
> }
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 10/12] mkfs: allow setting dax flag on root directory
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (8 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 09/12] xfs_scrub: fix background-mode sleep throttling Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-21 19:19 ` Eric Sandeen
2019-05-20 23:17 ` [PATCH 11/12] mkfs: validate start and end of aligned logs Darrick J. Wong
2019-05-20 23:18 ` [PATCH 12/12] mkfs: enable reflink by default Darrick J. Wong
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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;
}
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 10/12] mkfs: allow setting dax flag on root directory
2019-05-20 23:17 ` [PATCH 10/12] mkfs: allow setting dax flag on root directory Darrick J. Wong
@ 2019-05-21 19:19 ` Eric Sandeen
0 siblings, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:19 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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;
> }
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 11/12] mkfs: validate start and end of aligned logs
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (9 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 10/12] mkfs: allow setting dax flag on root directory Darrick J. Wong
@ 2019-05-20 23:17 ` Darrick J. Wong
2019-05-21 19:24 ` Eric Sandeen
2019-05-20 23:18 ` [PATCH 12/12] mkfs: enable reflink by default Darrick J. Wong
11 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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"),
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 11/12] mkfs: validate start and end of aligned logs
2019-05-20 23:17 ` [PATCH 11/12] mkfs: validate start and end of aligned logs Darrick J. Wong
@ 2019-05-21 19:24 ` Eric Sandeen
2019-05-22 16:42 ` Darrick J. Wong
0 siblings, 1 reply; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:24 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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"),
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 11/12] mkfs: validate start and end of aligned logs
2019-05-21 19:24 ` Eric Sandeen
@ 2019-05-22 16:42 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-22 16:42 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
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"),
> >
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 12/12] mkfs: enable reflink by default
2019-05-20 23:16 [PATCH 00/12] xfsprogs-5.1: fix various problems Darrick J. Wong
` (10 preceding siblings ...)
2019-05-20 23:17 ` [PATCH 11/12] mkfs: validate start and end of aligned logs Darrick J. Wong
@ 2019-05-20 23:18 ` Darrick J. Wong
2019-05-21 19:27 ` Eric Sandeen
2019-05-21 19:30 ` [PATCH 12/12 V2] " Eric Sandeen
11 siblings, 2 replies; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:18 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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,
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 12/12] mkfs: enable reflink by default
2019-05-20 23:18 ` [PATCH 12/12] mkfs: enable reflink by default Darrick J. Wong
@ 2019-05-21 19:27 ` Eric Sandeen
2019-05-21 19:30 ` [PATCH 12/12 V2] " Eric Sandeen
1 sibling, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:27 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 12/12 V2] mkfs: enable reflink by default
2019-05-20 23:18 ` [PATCH 12/12] mkfs: enable reflink by default Darrick J. Wong
2019-05-21 19:27 ` Eric Sandeen
@ 2019-05-21 19:30 ` Eric Sandeen
2019-05-22 16:44 ` Darrick J. Wong
1 sibling, 1 reply; 36+ messages in thread
From: Eric Sandeen @ 2019-05-21 19:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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,
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 12/12 V2] mkfs: enable reflink by default
2019-05-21 19:30 ` [PATCH 12/12 V2] " Eric Sandeen
@ 2019-05-22 16:44 ` Darrick J. Wong
2019-05-22 16:46 ` Eric Sandeen
0 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2019-05-22 16:44 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
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,
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 12/12 V2] mkfs: enable reflink by default
2019-05-22 16:44 ` Darrick J. Wong
@ 2019-05-22 16:46 ` Eric Sandeen
0 siblings, 0 replies; 36+ messages in thread
From: Eric Sandeen @ 2019-05-22 16:46 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
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,
>>
>>
>
^ permalink raw reply [flat|nested] 36+ messages in thread