linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups
@ 2016-11-04 18:30 Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 01/11] xfs_repair: fix some potential null pointer deferences Darrick J. Wong
                   ` (10 more replies)
  0 siblings, 11 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

Hi all,

This is a patchset that aims to fix various discrepancies between the
kernel and xfsprogs' copies of libxfs/.  They shouldn't affect the
behavior of either codebase while bringing the benefit that libxfs-apply
will run more smoothly.

The first two patches fix some problems that Coverity found with the
initial commit of userspace support for reflink.  After that are nine
patches that fix a few places where the kernel and xfsprogs' copies of
libxfs have gotten out of sync.  The penultimate patch refactors the
directory freescan code so that xfs_repair and kernel can use the same
code without the macro wrappers.  The final patch is a script to show
the differences between any file that appears in both libxfs/
directories.

--Darrick

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/11] xfs_repair: fix some potential null pointer deferences
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
@ 2016-11-04 18:30 ` Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 02/11] xfs_repair: fix bogus rmapbt record owner check Darrick J. Wong
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

Fix some potential NULL pointer deferences that Coverity pointed out,
and remove a trivial dead integer check.

Coverity-id: 1375789, 1375790, 1375791, 1375792
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/phase5.c |    2 +-
 repair/rmap.c   |    2 +-
 repair/slab.h   |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)


diff --git a/repair/phase5.c b/repair/phase5.c
index 3604d1d..cbda556 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -1925,7 +1925,7 @@ _("Insufficient memory to construct refcount cursor."));
 	refc_rec = pop_slab_cursor(refc_cur);
 	lptr = &btree_curs->level[0];
 
-	for (i = 0; i < lptr->num_blocks; i++)  {
+	for (i = 0; i < lptr->num_blocks && refc_rec != NULL; i++)  {
 		/*
 		 * block initialization, lay in block header
 		 */
diff --git a/repair/rmap.c b/repair/rmap.c
index 45e183a..7508973 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -790,7 +790,7 @@ compute_refcounts(
 		mark_inode_rl(mp, stack_top);
 
 		/* Set nbno to the bno of the next refcount change */
-		if (n < slab_count(rmaps))
+		if (n < slab_count(rmaps) && array_cur)
 			nbno = array_cur->rm_startblock;
 		else
 			nbno = NULLAGBLOCK;
diff --git a/repair/slab.h b/repair/slab.h
index 4aa5512..a2201f1 100644
--- a/repair/slab.h
+++ b/repair/slab.h
@@ -54,7 +54,7 @@ extern void *bag_item(struct xfs_bag *, size_t);
 
 #define foreach_bag_ptr_reverse(bag, idx, ptr) \
 	for ((idx) = bag_count(bag) - 1, (ptr) = bag_item((bag), (idx)); \
-	     (idx) >= 0 && (ptr) != NULL; \
+	     (ptr) != NULL; \
 	     (idx)--, (ptr) = bag_item((bag), (idx)))
 
 #endif /* SLAB_H_ */


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/11] xfs_repair: fix bogus rmapbt record owner check
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 01/11] xfs_repair: fix some potential null pointer deferences Darrick J. Wong
@ 2016-11-04 18:30 ` Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 03/11] xfs_io: fix libxfs naming violation Darrick J. Wong
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

Make the reverse mapping owner check actually validate inode numbers.

Coverity-id: 1371628
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/scan.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


diff --git a/repair/scan.c b/repair/scan.c
index 0e13581..b9ef4dc 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -1052,8 +1052,12 @@ _("%s rmap btree block claimed (state %d), agno %d, bno %d, suspect %d\n"),
 			}
 
 			/* Look for impossible owners. */
-			if (!(owner > 0 || (owner > XFS_RMAP_OWN_MIN &&
-					    owner <= XFS_RMAP_OWN_FS)))
+			if (!((owner > XFS_RMAP_OWN_MIN &&
+			       owner <= XFS_RMAP_OWN_FS) ||
+			      (XFS_INO_TO_AGNO(mp, owner) < mp->m_sb.sb_agcount &&
+			       XFS_AGINO_TO_AGBNO(mp,
+					XFS_INO_TO_AGINO(mp, owner)) <
+					mp->m_sb.sb_agblocks)))
 				do_warn(
 	_("invalid owner in rmap btree record %d (%"PRId64" %u) block %u/%u\n"),
 						i, owner, len, agno, bno);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/11] xfs_io: fix libxfs naming violation
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 01/11] xfs_repair: fix some potential null pointer deferences Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 02/11] xfs_repair: fix bogus rmapbt record owner check Darrick J. Wong
@ 2016-11-04 18:30 ` Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 04/11] libxfs: remove unnecessary hascrc test in btree verifiers Darrick J. Wong
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

All the calls to libxfs code should start with 'libxfs'
per libxfs_api_defs.h.

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


diff --git a/db/io.c b/db/io.c
index 8610f4d..f398195 100644
--- a/db/io.c
+++ b/db/io.c
@@ -504,7 +504,7 @@ write_cur(void)
 	/* If we didn't write the crc automatically, re-check inode validity */
 	if (xfs_sb_version_hascrc(&mp->m_sb) &&
 	    skip_crc && iocur_top->ino_buf) {
-		iocur_top->ino_crc_ok = xfs_verify_cksum(iocur_top->data,
+		iocur_top->ino_crc_ok = libxfs_verify_cksum(iocur_top->data,
 						mp->m_sb.sb_inodesize,
 						XFS_DINODE_CRC_OFF);
 	}


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/11] libxfs: remove unnecessary hascrc test in btree verifiers
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (2 preceding siblings ...)
  2016-11-04 18:30 ` [PATCH 03/11] xfs_io: fix libxfs naming violation Darrick J. Wong
@ 2016-11-04 18:30 ` Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 05/11] xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get Eric Sandeen
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

xfs_btree_sblock_v5hdr_verify already checks _hascrc, so we can
remove it from the verifier functions.  For whatever reason this
change made it into the kernel but not xfsprogs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/xfs_alloc_btree.c  |    4 ----
 libxfs/xfs_ialloc_btree.c |    2 --
 2 files changed, 6 deletions(-)


diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
index f993b87..ff4bae4 100644
--- a/libxfs/xfs_alloc_btree.c
+++ b/libxfs/xfs_alloc_btree.c
@@ -278,8 +278,6 @@ xfs_allocbt_verify(
 	level = be16_to_cpu(block->bb_level);
 	switch (block->bb_magic) {
 	case cpu_to_be32(XFS_ABTB_CRC_MAGIC):
-		if (!xfs_sb_version_hascrc(&mp->m_sb))
-			return false;
 		if (!xfs_btree_sblock_v5hdr_verify(bp))
 			return false;
 		/* fall through */
@@ -291,8 +289,6 @@ xfs_allocbt_verify(
 			return false;
 		break;
 	case cpu_to_be32(XFS_ABTC_CRC_MAGIC):
-		if (!xfs_sb_version_hascrc(&mp->m_sb))
-			return false;
 		if (!xfs_btree_sblock_v5hdr_verify(bp))
 			return false;
 		/* fall through */
diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index 8ccabcc..7bf6040 100644
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -225,8 +225,6 @@ xfs_inobt_verify(
 	switch (block->bb_magic) {
 	case cpu_to_be32(XFS_IBT_CRC_MAGIC):
 	case cpu_to_be32(XFS_FIBT_CRC_MAGIC):
-		if (!xfs_sb_version_hascrc(&mp->m_sb))
-			return false;
 		if (!xfs_btree_sblock_v5hdr_verify(bp))
 			return false;
 		/* fall through */


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/11] xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (3 preceding siblings ...)
  2016-11-04 18:30 ` [PATCH 04/11] libxfs: remove unnecessary hascrc test in btree verifiers Darrick J. Wong
@ 2016-11-04 18:30 ` Eric Sandeen
  2016-11-06 22:20   ` Dave Chinner
  2016-11-04 18:30 ` [PATCH 06/11] libxfs: refactor btree crc verifier Darrick J. Wong
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 13+ messages in thread
From: Eric Sandeen @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs, Dave Chinner

>From c400ee3ed1b13d45adde68e12254dc6ab6977b59 Mon Sep 17 00:00:00 2001

It's entirely possible for userspace to ask for an xattr which
does not exist.

Normally, there is no problem whatsoever when we ask for such
a thing, but when we look at an obfuscated metadump image
on a debug kernel with selinux, we trip over this ASSERT in
xfs_da3_path_shift():

        *result = -ENOENT;      /* we're out of our tree */
        ASSERT(args->op_flags & XFS_DA_OP_OKNOENT);

It (more or less) only shows up in the above scenario, because
xfs_metadump obfuscates attr names, but chooses names which
keep the same hash value - and xfs_da3_node_lookup_int does:

        if (((retval == -ENOENT) || (retval == -ENOATTR)) &&
            (blk->hashval == args->hashval)) {
                error = xfs_da3_path_shift(state, &state->path, 1, 1,
                                                 &retval);

IOWS, we only get down to the xfs_da3_path_shift() ASSERT
if we are looking for an xattr which doesn't exist, but we
find xattrs on disk which have the same hash, and so might be
a hash collision, so we try the path shift.  When *that*
fails to find what we're looking for, we hit the assert about
XFS_DA_OP_OKNOENT.

Simply setting XFS_DA_OP_OKNOENT in xfs_attr_get solves this
rather corner-case problem with no ill side effects.  It's
fine for an attr name lookup to fail.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
 libxfs/xfs_attr.c |    2 ++
 1 file changed, 2 insertions(+)


diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index c7f0afa..60513f9 100644
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -135,6 +135,8 @@ xfs_attr_get(
 
 	args.value = value;
 	args.valuelen = *valuelenp;
+	/* Entirely possible to look up a name which doesn't exist */
+	args.op_flags = XFS_DA_OP_OKNOENT;
 
 	lock_mode = xfs_ilock_attr_map_shared(ip);
 	if (!xfs_inode_hasattr(ip))


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/11] libxfs: refactor btree crc verifier
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (4 preceding siblings ...)
  2016-11-04 18:30 ` [PATCH 05/11] xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get Eric Sandeen
@ 2016-11-04 18:30 ` Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 07/11] libxfs: fix whitespace to match the kernel Darrick J. Wong
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

In a65d8d293b ("libxfs: validate metadata LSNs against log on v5
superblocks") the hascrc check was modified to use the helper mp
variable in the kernel.  This was left out of the xfsprogs patch, so
change it here too.

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


diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index 1d41265..3dea6bd 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -244,7 +244,7 @@ xfs_btree_lblock_verify_crc(
 	struct xfs_btree_block	*block = XFS_BUF_TO_BLOCK(bp);
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if (xfs_sb_version_hascrc(&bp->b_target->bt_mount->m_sb)) {
+	if (xfs_sb_version_hascrc(&mp->m_sb)) {
 		if (!xfs_log_check_lsn(mp, be64_to_cpu(block->bb_u.l.bb_lsn)))
 			return false;
 		return xfs_buf_verify_cksum(bp, XFS_BTREE_LBLOCK_CRC_OFF);
@@ -282,7 +282,7 @@ xfs_btree_sblock_verify_crc(
 	struct xfs_btree_block  *block = XFS_BUF_TO_BLOCK(bp);
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if (xfs_sb_version_hascrc(&bp->b_target->bt_mount->m_sb)) {
+	if (xfs_sb_version_hascrc(&mp->m_sb)) {
 		if (!xfs_log_check_lsn(mp, be64_to_cpu(block->bb_u.s.bb_lsn)))
 			return false;
 		return xfs_buf_verify_cksum(bp, XFS_BTREE_SBLOCK_CRC_OFF);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/11] libxfs: fix whitespace to match the kernel
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (5 preceding siblings ...)
  2016-11-04 18:30 ` [PATCH 06/11] libxfs: refactor btree crc verifier Darrick J. Wong
@ 2016-11-04 18:30 ` Darrick J. Wong
  2016-11-04 18:30 ` [PATCH 08/11] libxfs: return bool from sb_version_hasmetauuid Darrick J. Wong
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

Fix some minor whitespace errors so that my automated libxfs diff
scanning will stop reporting this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/xfs_dir2_node.c  |    2 ++
 libxfs/xfs_format.h     |    1 -
 libxfs/xfs_inode_buf.c  |    1 -
 libxfs/xfs_inode_fork.c |    1 +
 4 files changed, 3 insertions(+), 2 deletions(-)


diff --git a/libxfs/xfs_dir2_node.c b/libxfs/xfs_dir2_node.c
index df599f9..b75b432 100644
--- a/libxfs/xfs_dir2_node.c
+++ b/libxfs/xfs_dir2_node.c
@@ -2146,12 +2146,14 @@ xfs_dir2_node_replace(
 	state = xfs_da_state_alloc();
 	state->args = args;
 	state->mp = args->dp->i_mount;
+
 	/*
 	 * We have to save new inode number and ftype since
 	 * xfs_da3_node_lookup_int() is going to overwrite them
 	 */
 	inum = args->inumber;
 	ftype = args->filetype;
+
 	/*
 	 * Lookup the entry to change in the btree.
 	 */
diff --git a/libxfs/xfs_format.h b/libxfs/xfs_format.h
index 172fdfb..8628bab 100644
--- a/libxfs/xfs_format.h
+++ b/libxfs/xfs_format.h
@@ -473,7 +473,6 @@ xfs_sb_has_ro_compat_feature(
 #define XFS_SB_FEAT_INCOMPAT_FTYPE	(1 << 0)	/* filetype in dirent */
 #define XFS_SB_FEAT_INCOMPAT_SPINODES	(1 << 1)	/* sparse inode chunks */
 #define XFS_SB_FEAT_INCOMPAT_META_UUID	(1 << 2)	/* metadata UUID */
-
 #define XFS_SB_FEAT_INCOMPAT_ALL \
 		(XFS_SB_FEAT_INCOMPAT_FTYPE|	\
 		 XFS_SB_FEAT_INCOMPAT_SPINODES|	\
diff --git a/libxfs/xfs_inode_buf.c b/libxfs/xfs_inode_buf.c
index 4796a59..8732359 100644
--- a/libxfs/xfs_inode_buf.c
+++ b/libxfs/xfs_inode_buf.c
@@ -537,7 +537,6 @@ xfs_iread(
 	}
 
 	ASSERT(ip->i_d.di_version >= 2);
-
 	ip->i_delayed_blks = 0;
 
 	/*
diff --git a/libxfs/xfs_inode_fork.c b/libxfs/xfs_inode_fork.c
index 8212f77..2799e2d 100644
--- a/libxfs/xfs_inode_fork.c
+++ b/libxfs/xfs_inode_fork.c
@@ -29,6 +29,7 @@
 #include "xfs_attr_sf.h"
 #include "xfs_da_format.h"
 
+
 kmem_zone_t *xfs_ifork_zone;
 
 STATIC int xfs_iformat_local(xfs_inode_t *, xfs_dinode_t *, int, int);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/11] libxfs: return bool from sb_version_hasmetauuid
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (6 preceding siblings ...)
  2016-11-04 18:30 ` [PATCH 07/11] libxfs: fix whitespace to match the kernel Darrick J. Wong
@ 2016-11-04 18:30 ` Darrick J. Wong
  2016-11-04 18:31 ` [PATCH 09/11] xfs: fix btree cursor error cleanups Brian Foster
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:30 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

The kernel's version of this function returns bool, so do so here too.

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


diff --git a/libxfs/xfs_format.h b/libxfs/xfs_format.h
index 8628bab..e14f964 100644
--- a/libxfs/xfs_format.h
+++ b/libxfs/xfs_format.h
@@ -536,7 +536,7 @@ static inline bool xfs_sb_version_hassparseinodes(struct xfs_sb *sbp)
  * user-visible UUID to be changed on V5 filesystems which have a
  * filesystem UUID stamped into every piece of metadata.
  */
-static inline int xfs_sb_version_hasmetauuid(xfs_sb_t *sbp)
+static inline bool xfs_sb_version_hasmetauuid(struct xfs_sb *sbp)
 {
 	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) &&
 		(sbp->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_META_UUID);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/11] xfs: fix btree cursor error cleanups
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (7 preceding siblings ...)
  2016-11-04 18:30 ` [PATCH 08/11] libxfs: return bool from sb_version_hasmetauuid Darrick J. Wong
@ 2016-11-04 18:31 ` Brian Foster
  2016-11-04 18:31 ` [PATCH 10/11] libxfs: clean up _dir2_data_freescan Darrick J. Wong
  2016-11-04 18:31 ` [PATCH 11/11] tools: create libxfs-diff to compare libxfses Darrick J. Wong
  10 siblings, 0 replies; 13+ messages in thread
From: Brian Foster @ 2016-11-04 18:31 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs, Christoph Hellwig

>From f307080a626569f89bc8fbad9f936b307aded877 Mon Sep 17 00:00:00 2001

The btree cursor cleanup function takes an error parameter that
affects how buffers are released from the cursor. All buffers are
released in the event of error. Several callers do not specify the
XFS_BTREE_ERROR flag in the event of error, however. This can cause
buffers to hang around locked or with an elevated hold count and
thus lead to umount hangs in the event of errors.

Fix up the xfs_btree_del_cursor() callers to pass XFS_BTREE_ERROR if
the cursor is being torn down due to error.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
 libxfs/xfs_ialloc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c
index f0f243e..5c88b0b 100644
--- a/libxfs/xfs_ialloc.c
+++ b/libxfs/xfs_ialloc.c
@@ -2232,7 +2232,7 @@ xfs_imap_lookup(
 	}
 
 	xfs_trans_brelse(tp, agbp);
-	xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+	xfs_btree_del_cursor(cur, error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
 	if (error)
 		return error;
 


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/11] libxfs: clean up _dir2_data_freescan
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (8 preceding siblings ...)
  2016-11-04 18:31 ` [PATCH 09/11] xfs: fix btree cursor error cleanups Brian Foster
@ 2016-11-04 18:31 ` Darrick J. Wong
  2016-11-04 18:31 ` [PATCH 11/11] tools: create libxfs-diff to compare libxfses Darrick J. Wong
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:31 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

Refactor the implementations of xfs_dir2_data_freescan into a
routine that takes the raw directory block parameters and
a second function that figures out the raw parameters from the
directory inode.  This enables us to use the exact same code
for both userspace and the kernel, since repair knows exactly
which directory block geometry parameters it needs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/libxfs_api_defs.h |    2 +-
 libxfs/libxfs_priv.h     |    8 --------
 libxfs/xfs_dir2.h        |    4 +++-
 libxfs/xfs_dir2_data.c   |   12 +++++++++++-
 repair/dir2.c            |    4 ++--
 repair/phase6.c          |    3 ++-
 6 files changed, 19 insertions(+), 14 deletions(-)


diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index d60b6c2..12a668e 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -88,7 +88,7 @@
 #define xfs_dir_replace			libxfs_dir_replace
 #define xfs_dir2_isblock		libxfs_dir2_isblock
 #define xfs_dir2_isleaf			libxfs_dir2_isleaf
-#define __xfs_dir2_data_freescan	libxfs_dir2_data_freescan
+#define xfs_dir2_data_freescan_hdr	libxfs_dir2_data_freescan_hdr
 #define xfs_dir2_data_log_entry		libxfs_dir2_data_log_entry
 #define xfs_dir2_data_log_header	libxfs_dir2_data_log_header
 #define xfs_dir2_data_make_free		libxfs_dir2_data_make_free
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index a101a00..f4db183 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -49,14 +49,6 @@
 #ifndef __LIBXFS_INTERNAL_XFS_H__
 #define __LIBXFS_INTERNAL_XFS_H__
 
-/*
- * Repair doesn't have a inode when it calls libxfs_dir2_data_freescan,
- * so we to work around this internally for now.
- */
-#define xfs_dir2_data_freescan(ip, hdr, loghead) \
-	__xfs_dir2_data_freescan((ip)->i_mount->m_dir_geo, \
-				 (ip)->d_ops, hdr, loghead)
-
 #include "libxfs_api_defs.h"
 #include "platform_defs.h"
 #include "xfs.h"
diff --git a/libxfs/xfs_dir2.h b/libxfs/xfs_dir2.h
index 89b9e24..385d6d5 100644
--- a/libxfs/xfs_dir2.h
+++ b/libxfs/xfs_dir2.h
@@ -157,9 +157,11 @@ extern int xfs_dir2_isleaf(struct xfs_da_args *args, int *r);
 extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db,
 				struct xfs_buf *bp);
 
-extern void __xfs_dir2_data_freescan(struct xfs_da_geometry *geo,
+extern void xfs_dir2_data_freescan_hdr(struct xfs_da_geometry *geo,
 		const struct xfs_dir_ops *ops,
 		struct xfs_dir2_data_hdr *hdr, int *loghead);
+extern void xfs_dir2_data_freescan(struct xfs_inode *dp,
+		struct xfs_dir2_data_hdr *hdr, int *loghead);
 extern void xfs_dir2_data_log_entry(struct xfs_da_args *args,
 		struct xfs_buf *bp, struct xfs_dir2_data_entry *dep);
 extern void xfs_dir2_data_log_header(struct xfs_da_args *args,
diff --git a/libxfs/xfs_dir2_data.c b/libxfs/xfs_dir2_data.c
index 6ae5cd2..1dc0fa6 100644
--- a/libxfs/xfs_dir2_data.c
+++ b/libxfs/xfs_dir2_data.c
@@ -502,7 +502,7 @@ xfs_dir2_data_freeremove(
  * Given a data block, reconstruct its bestfree map.
  */
 void
-__xfs_dir2_data_freescan(
+xfs_dir2_data_freescan_hdr(
 	struct xfs_da_geometry	*geo,
 	const struct xfs_dir_ops *ops,
 	struct xfs_dir2_data_hdr *hdr,
@@ -562,6 +562,16 @@ __xfs_dir2_data_freescan(
 	}
 }
 
+void
+xfs_dir2_data_freescan(
+	struct xfs_inode	*dp,
+	struct xfs_dir2_data_hdr *hdr,
+	int			*loghead)
+{
+	return xfs_dir2_data_freescan_hdr(dp->i_mount->m_dir_geo, dp->d_ops,
+			hdr, loghead);
+}
+
 /*
  * Initialize a data block at the given block number in the directory.
  * Give back the buffer for the created block.
diff --git a/repair/dir2.c b/repair/dir2.c
index 61912d1..10c693e 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -921,8 +921,8 @@ _("bad bestfree table in block %u in directory inode %" PRIu64 ": "),
 			da_bno, ino);
 		if (!no_modify) {
 			do_warn(_("repairing table\n"));
-			libxfs_dir2_data_freescan(mp->m_dir_geo, M_DIROPS(mp),
-						  d, &i);
+			libxfs_dir2_data_freescan_hdr(mp->m_dir_geo,
+					M_DIROPS(mp), d, &i);
 			*dirty = 1;
 		} else {
 			do_warn(_("would repair table\n"));
diff --git a/repair/phase6.c b/repair/phase6.c
index 06eed16..1411d2e 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -1883,7 +1883,8 @@ _("entry \"%s\" in dir inode %" PRIu64 " inconsistent with .. value (%" PRIu64 "
 	}
 	*num_illegal += nbad;
 	if (needscan)
-		libxfs_dir2_data_freescan(mp->m_dir_geo, M_DIROPS(mp), d, &i);
+		libxfs_dir2_data_freescan_hdr(mp->m_dir_geo, M_DIROPS(mp),
+				d, &i);
 	if (needlog)
 		libxfs_dir2_data_log_header(&da, bp);
 	libxfs_defer_finish(&tp, &dfops, ip);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 11/11] tools: create libxfs-diff to compare libxfses
  2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
                   ` (9 preceding siblings ...)
  2016-11-04 18:31 ` [PATCH 10/11] libxfs: clean up _dir2_data_freescan Darrick J. Wong
@ 2016-11-04 18:31 ` Darrick J. Wong
  10 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2016-11-04 18:31 UTC (permalink / raw)
  To: david, darrick.wong; +Cc: linux-xfs

Create a script to compare every file in libxfs to the same files
in another libxfs.  This is useful for comparing upstream kernel
and user progs to look for unported changes.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tools/libxfs-diff |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100755 tools/libxfs-diff


diff --git a/tools/libxfs-diff b/tools/libxfs-diff
new file mode 100755
index 0000000..ad69c85
--- /dev/null
+++ b/tools/libxfs-diff
@@ -0,0 +1,25 @@
+#!/bin/bash
+
+if [ -z "$1" ] || [ ! -d "$1" ] || [ "$1" = "--help" ]; then
+	echo "Usage: $0 kernel_libxfs_dir"
+	exit 1
+fi
+
+if [ ! -d "libxfs/" ]; then
+	echo "$0: Must be run from the top level directory."
+	exit 2
+fi
+
+dir="$1"
+
+if [ ! -e "${dir}/xfs_format.h" ]; then
+	echo "${dir}: This doesn't seem to be a libxfs/ directory."
+	exit 3
+fi
+
+dir="$(readlink -m "${dir}/..")"
+
+for i in libxfs/xfs*.[ch]; do
+	kfile="${dir}/$i"
+	diff -Naurpw --label "$i" <(sed -e '/#include/d' "$i") --label "${kfile}" <(sed -e '/#include/d' "${kfile}")
+done


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 05/11] xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get
  2016-11-04 18:30 ` [PATCH 05/11] xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get Eric Sandeen
@ 2016-11-06 22:20   ` Dave Chinner
  0 siblings, 0 replies; 13+ messages in thread
From: Dave Chinner @ 2016-11-06 22:20 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: darrick.wong, linux-xfs, Dave Chinner

Darrick,

There's something whacky with what you are doing to create these
commit messages. The from address on the email itself is not from
you (how did that get through SPF checks?) and instead there's this:

On Fri, Nov 04, 2016 at 11:30:36AM -0700, Eric Sandeen wrote:
> >From c400ee3ed1b13d45adde68e12254dc6ab6977b59 Mon Sep 17 00:00:00 2001

Which tells me nothing useful about the origin of the patch and it
means I have to mangle the commit message before committing it.

If you look at the libxfs sync commits that I applied for the
4.9-rc1 sync, the libxfs-apply script ends up formating the commits
like this:

ommit ece930fa14a3439e40cd1b43063cab5b85ab9407
Author: Christoph Hellwig <hch@lst.de>
Date:   Tue Oct 25 12:59:46 2016 +1100

    xfs: refactor xfs_bunmapi_cow
    
    Source kernel commit: fa5c836ca8eb5bad6316ddfc066acbc4e2485356
    
    Split out two helpers for deleting delayed or real extents from the COW fork.
    This allows to call them directly from xfs_reflink_cow_end_io once that
    function is refactored to iterate the extent tree.  It will also allow
    to reuse the delalloc deletion from xfs_bunmapi in the future.
    
    Signed-off-by: Christoph Hellwig <hch@lst.de>
    Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
    Reviewed-by: Brian Foster <bfoster@redhat.com>
    Signed-off-by: Dave Chinner <david@fromorbit.com>


So there's a clear line where the orginal commit came from. It does
this automatically now (see the fixup_header_format() function) so
that this sort of things doesn't need to be cleaned up after the
fact. If the commit is from xfsprogs and going to the kernel, it
will say "Source xfsprogs commit: ....." instead.  This way it is
clear to the reader that this commit is simply a libxfs sync commit,
and that the sob and rvb tags applied to the commit in the source
repository.

Perhaps you are using an older version of the script to generate
these commits?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-11-06 22:20 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-04 18:30 [PATCH 00/11] xfsprogs: miscellaneous libxfs cleanups Darrick J. Wong
2016-11-04 18:30 ` [PATCH 01/11] xfs_repair: fix some potential null pointer deferences Darrick J. Wong
2016-11-04 18:30 ` [PATCH 02/11] xfs_repair: fix bogus rmapbt record owner check Darrick J. Wong
2016-11-04 18:30 ` [PATCH 03/11] xfs_io: fix libxfs naming violation Darrick J. Wong
2016-11-04 18:30 ` [PATCH 04/11] libxfs: remove unnecessary hascrc test in btree verifiers Darrick J. Wong
2016-11-04 18:30 ` [PATCH 05/11] xfs: set XFS_DA_OP_OKNOENT in xfs_attr_get Eric Sandeen
2016-11-06 22:20   ` Dave Chinner
2016-11-04 18:30 ` [PATCH 06/11] libxfs: refactor btree crc verifier Darrick J. Wong
2016-11-04 18:30 ` [PATCH 07/11] libxfs: fix whitespace to match the kernel Darrick J. Wong
2016-11-04 18:30 ` [PATCH 08/11] libxfs: return bool from sb_version_hasmetauuid Darrick J. Wong
2016-11-04 18:31 ` [PATCH 09/11] xfs: fix btree cursor error cleanups Brian Foster
2016-11-04 18:31 ` [PATCH 10/11] libxfs: clean up _dir2_data_freescan Darrick J. Wong
2016-11-04 18:31 ` [PATCH 11/11] tools: create libxfs-diff to compare libxfses Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).