linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] libxfs: add a flags argument to libxfs_iget
@ 2017-07-18 19:06 Darrick J. Wong
  2017-07-18 19:06 ` [PATCH 2/2] libxfs: allow xfs_repair to bypass inode fork read verifiers Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Darrick J. Wong @ 2017-07-18 19:06 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs, bfoster

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

Add a flags argument to libxfs_iget to bring it up to date with the
kernel xfs_iget.  We will use the flags argument in the next patch
to enable xfs_repair to shut off inode fork verifiers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/attrset.c        |    4 ++--
 include/xfs_inode.h |    2 +-
 libxfs/rdwr.c       |   15 ++++++++++-----
 libxfs/trans.c      |    4 ++--
 repair/phase6.c     |   10 +++++-----
 5 files changed, 20 insertions(+), 15 deletions(-)


diff --git a/db/attrset.c b/db/attrset.c
index ad3c8f3..ddd72ed 100644
--- a/db/attrset.c
+++ b/db/attrset.c
@@ -151,7 +151,7 @@ attr_set_f(
 		value = NULL;
 	}
 
-	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip)) {
+	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, 0, &ip)) {
 		dbprintf(_("failed to iget inode %llu\n"),
 			(unsigned long long)iocur_top->ino);
 		goto out;
@@ -226,7 +226,7 @@ attr_remove_f(
 
 	name = argv[optind];
 
-	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip)) {
+	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, 0, &ip)) {
 		dbprintf(_("failed to iget inode %llu\n"),
 			(unsigned long long)iocur_top->ino);
 		goto out;
diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 8766024..5d5158e 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -149,7 +149,7 @@ extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
 
 /* Inode Cache Interfaces */
 extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
-				uint, struct xfs_inode **);
+				uint, uint, struct xfs_inode **);
 extern void	libxfs_iput(struct xfs_inode *);
 
 #define IRELE(ip) libxfs_iput(ip)
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 43b4f1d..cf156ba 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1329,11 +1329,16 @@ extern kmem_zone_t	*xfs_ili_zone;
 extern kmem_zone_t	*xfs_inode_zone;
 
 int
-libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
-		xfs_inode_t **ipp)
+libxfs_iget(
+	struct xfs_mount	*mp,
+	struct xfs_trans	*tp,
+	xfs_ino_t		ino,
+	uint			flags,
+	uint			lock_flags,
+	struct xfs_inode	**ipp)
 {
-	xfs_inode_t	*ip;
-	int		error = 0;
+	struct xfs_inode	*ip;
+	int			error = 0;
 
 	ip = kmem_zone_zalloc(xfs_inode_zone, 0);
 	if (!ip)
@@ -1341,7 +1346,7 @@ libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
 
 	ip->i_ino = ino;
 	ip->i_mount = mp;
-	error = xfs_iread(mp, tp, ip, 0);
+	error = xfs_iread(mp, tp, ip, flags);
 	if (error) {
 		kmem_zone_free(xfs_inode_zone, ip);
 		*ipp = NULL;
diff --git a/libxfs/trans.c b/libxfs/trans.c
index e161c28..fe22cb9 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -246,9 +246,9 @@ libxfs_trans_iget(
 	xfs_inode_log_item_t	*iip;
 
 	if (tp == NULL)
-		return libxfs_iget(mp, tp, ino, lock_flags, ipp);
+		return libxfs_iget(mp, tp, ino, flags, lock_flags, ipp);
 
-	error = libxfs_iget(mp, tp, ino, lock_flags, &ip);
+	error = libxfs_iget(mp, tp, ino, flags, lock_flags, &ip);
 	if (error)
 		return error;
 	ASSERT(ip != NULL);
diff --git a/repair/phase6.c b/repair/phase6.c
index 373b1a5..011bcdf 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -925,7 +925,7 @@ mk_orphanage(xfs_mount_t *mp)
 	 * would have been cleared in phase3 and phase4.
 	 */
 
-	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, &pip)))
+	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, 0, &pip)))
 		do_error(_("%d - couldn't iget root inode to obtain %s\n"),
 			i, ORPHANAGE);
 
@@ -949,7 +949,7 @@ mk_orphanage(xfs_mount_t *mp)
 	 * use iget/ijoin instead of trans_iget because the ialloc
 	 * wrapper can commit the transaction and start a new one
 	 */
-/*	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, &pip)))
+/*	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, 0, &pip)))
 		do_error(_("%d - couldn't iget root inode to make %s\n"),
 			i, ORPHANAGE);*/
 
@@ -1063,7 +1063,7 @@ mv_orphanage(
 	xname.len = snprintf((char *)fname, sizeof(fname), "%llu",
 				(unsigned long long)ino);
 
-	err = -libxfs_iget(mp, NULL, orphanage_ino, 0, &orphanage_ip);
+	err = -libxfs_iget(mp, NULL, orphanage_ino, 0, 0, &orphanage_ip);
 	if (err)
 		do_error(_("%d - couldn't iget orphanage inode\n"), err);
 	/*
@@ -1075,7 +1075,7 @@ mv_orphanage(
 		xname.len = snprintf((char *)fname, sizeof(fname), "%llu.%d",
 					(unsigned long long)ino, ++incr);
 
-	if ((err = -libxfs_iget(mp, NULL, ino, 0, &ino_p)))
+	if ((err = -libxfs_iget(mp, NULL, ino, 0, 0, &ino_p)))
 		do_error(_("%d - couldn't iget disconnected inode\n"), err);
 
 	xname.type = xfs_mode_to_ftype(VFS_I(ino_p)->i_mode);
@@ -2817,7 +2817,7 @@ process_dir_inode(
 
 	ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
 
-	error = -libxfs_iget(mp, NULL, ino, 0, &ip);
+	error = -libxfs_iget(mp, NULL, ino, 0, 0, &ip);
 	if (error) {
 		if (!no_modify)
 			do_error(


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

* [PATCH 2/2] libxfs: allow xfs_repair to bypass inode fork read verifiers
  2017-07-18 19:06 [PATCH 1/2] libxfs: add a flags argument to libxfs_iget Darrick J. Wong
@ 2017-07-18 19:06 ` Darrick J. Wong
  2017-07-18 19:51   ` Allison Henderson
  2017-07-18 19:50 ` [PATCH 1/2] libxfs: add a flags argument to libxfs_iget Allison Henderson
  2017-07-19  2:24 ` Darrick J. Wong
  2 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2017-07-18 19:06 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs, bfoster

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

In xfs_repair phase 6, we want to be able to check and fix short form
directory inodes.  However, if the directory is already corrupt, the
read verifier prevents us from loading the inode, which prevents us from
fixing it.  Therefore, allow _iget callers to bypass the read verifier.
Note that we do /not/ allow bypassing of the write verifier, ever.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/xfs_inode.h     |    3 +++
 libxfs/libxfs_priv.h    |    3 ---
 libxfs/xfs_inode_buf.c  |    3 ++-
 libxfs/xfs_inode_fork.c |   15 ++++++++-------
 libxfs/xfs_inode_fork.h |    2 +-
 repair/phase6.c         |    2 +-
 6 files changed, 15 insertions(+), 13 deletions(-)


diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 5d5158e..b2f2af5 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -148,6 +148,9 @@ extern void	libxfs_trans_ichgtime(struct xfs_trans *,
 extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
 
 /* Inode Cache Interfaces */
+#define XFS_IGET_CREATE			0x1
+#define XFS_IGET_UNTRUSTED		0x2
+#define XFS_IGET_SKIP_FORK_VERIFY	0x4
 extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
 				uint, uint, struct xfs_inode **);
 extern void	libxfs_iput(struct xfs_inode *);
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 4f5ba7c..da1cc20 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -103,9 +103,6 @@ extern char    *progname;
 #define ATTR_KERNOTIME			0
 #define ATTR_KERNOVAL			0
 
-#define XFS_IGET_CREATE			0x1
-#define XFS_IGET_UNTRUSTED		0x2
-
 extern void cmn_err(int, char *, ...);
 enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
 
diff --git a/libxfs/xfs_inode_buf.c b/libxfs/xfs_inode_buf.c
index b0f2c0d..174a24e 100644
--- a/libxfs/xfs_inode_buf.c
+++ b/libxfs/xfs_inode_buf.c
@@ -522,7 +522,8 @@ xfs_iread(
 	 */
 	if (dip->di_mode) {
 		xfs_inode_from_disk(ip, dip);
-		error = xfs_iformat_fork(ip, dip);
+		error = xfs_iformat_fork(ip, dip,
+				!(iget_flags & XFS_IGET_SKIP_FORK_VERIFY));
 		if (error)  {
 #ifdef DEBUG
 			xfs_alert(mp, "%s: xfs_iformat() returned error %d",
diff --git a/libxfs/xfs_inode_fork.c b/libxfs/xfs_inode_fork.c
index 0ef989d..e8379e1 100644
--- a/libxfs/xfs_inode_fork.c
+++ b/libxfs/xfs_inode_fork.c
@@ -50,13 +50,14 @@ STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int);
  */
 int
 xfs_iformat_fork(
-	xfs_inode_t		*ip,
-	xfs_dinode_t		*dip)
+	struct xfs_inode		*ip,
+	struct xfs_dinode		*dip,
+	bool				verify_fork)
 {
-	xfs_attr_shortform_t	*atp;
-	int			size;
-	int			error = 0;
-	xfs_fsize_t             di_size;
+	struct xfs_attr_shortform	*atp;
+	int				size;
+	int				error = 0;
+	xfs_fsize_t			di_size;
 
 	if (unlikely(be32_to_cpu(dip->di_nextents) +
 		     be16_to_cpu(dip->di_anextents) >
@@ -181,7 +182,7 @@ xfs_iformat_fork(
 		return error;
 
 	/* Check inline dir contents. */
-	if (S_ISDIR(VFS_I(ip)->i_mode) &&
+	if (verify_fork && S_ISDIR(VFS_I(ip)->i_mode) &&
 	    dip->di_format == XFS_DINODE_FMT_LOCAL) {
 		error = xfs_dir2_sf_verify(ip);
 		if (error) {
diff --git a/libxfs/xfs_inode_fork.h b/libxfs/xfs_inode_fork.h
index 7fb8365..7a24288 100644
--- a/libxfs/xfs_inode_fork.h
+++ b/libxfs/xfs_inode_fork.h
@@ -139,7 +139,7 @@ typedef struct xfs_ifork {
 
 struct xfs_ifork *xfs_iext_state_to_fork(struct xfs_inode *ip, int state);
 
-int		xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *);
+int		xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *, bool);
 void		xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
 				struct xfs_inode_log_item *, int);
 void		xfs_idestroy_fork(struct xfs_inode *, int);
diff --git a/repair/phase6.c b/repair/phase6.c
index 011bcdf..5edfd29 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -2817,7 +2817,7 @@ process_dir_inode(
 
 	ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
 
-	error = -libxfs_iget(mp, NULL, ino, 0, 0, &ip);
+	error = -libxfs_iget(mp, NULL, ino, XFS_IGET_SKIP_FORK_VERIFY, 0, &ip);
 	if (error) {
 		if (!no_modify)
 			do_error(


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

* Re: [PATCH 1/2] libxfs: add a flags argument to libxfs_iget
  2017-07-18 19:06 [PATCH 1/2] libxfs: add a flags argument to libxfs_iget Darrick J. Wong
  2017-07-18 19:06 ` [PATCH 2/2] libxfs: allow xfs_repair to bypass inode fork read verifiers Darrick J. Wong
@ 2017-07-18 19:50 ` Allison Henderson
  2017-07-19  2:24 ` Darrick J. Wong
  2 siblings, 0 replies; 5+ messages in thread
From: Allison Henderson @ 2017-07-18 19:50 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs, bfoster

You can add

Reviewed by: Allison Henderson <allison.henderson@oracle.com>

Thanks!

On 07/18/2017 12:06 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Add a flags argument to libxfs_iget to bring it up to date with the
> kernel xfs_iget.  We will use the flags argument in the next patch
> to enable xfs_repair to shut off inode fork verifiers.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   db/attrset.c        |    4 ++--
>   include/xfs_inode.h |    2 +-
>   libxfs/rdwr.c       |   15 ++++++++++-----
>   libxfs/trans.c      |    4 ++--
>   repair/phase6.c     |   10 +++++-----
>   5 files changed, 20 insertions(+), 15 deletions(-)
>
>
> diff --git a/db/attrset.c b/db/attrset.c
> index ad3c8f3..ddd72ed 100644
> --- a/db/attrset.c
> +++ b/db/attrset.c
> @@ -151,7 +151,7 @@ attr_set_f(
>   		value = NULL;
>   	}
>   
> -	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip)) {
> +	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, 0, &ip)) {
>   		dbprintf(_("failed to iget inode %llu\n"),
>   			(unsigned long long)iocur_top->ino);
>   		goto out;
> @@ -226,7 +226,7 @@ attr_remove_f(
>   
>   	name = argv[optind];
>   
> -	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip)) {
> +	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, 0, &ip)) {
>   		dbprintf(_("failed to iget inode %llu\n"),
>   			(unsigned long long)iocur_top->ino);
>   		goto out;
> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
> index 8766024..5d5158e 100644
> --- a/include/xfs_inode.h
> +++ b/include/xfs_inode.h
> @@ -149,7 +149,7 @@ extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>   
>   /* Inode Cache Interfaces */
>   extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
> -				uint, struct xfs_inode **);
> +				uint, uint, struct xfs_inode **);
>   extern void	libxfs_iput(struct xfs_inode *);
>   
>   #define IRELE(ip) libxfs_iput(ip)
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 43b4f1d..cf156ba 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -1329,11 +1329,16 @@ extern kmem_zone_t	*xfs_ili_zone;
>   extern kmem_zone_t	*xfs_inode_zone;
>   
>   int
> -libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
> -		xfs_inode_t **ipp)
> +libxfs_iget(
> +	struct xfs_mount	*mp,
> +	struct xfs_trans	*tp,
> +	xfs_ino_t		ino,
> +	uint			flags,
> +	uint			lock_flags,
> +	struct xfs_inode	**ipp)
>   {
> -	xfs_inode_t	*ip;
> -	int		error = 0;
> +	struct xfs_inode	*ip;
> +	int			error = 0;
>   
>   	ip = kmem_zone_zalloc(xfs_inode_zone, 0);
>   	if (!ip)
> @@ -1341,7 +1346,7 @@ libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
>   
>   	ip->i_ino = ino;
>   	ip->i_mount = mp;
> -	error = xfs_iread(mp, tp, ip, 0);
> +	error = xfs_iread(mp, tp, ip, flags);
>   	if (error) {
>   		kmem_zone_free(xfs_inode_zone, ip);
>   		*ipp = NULL;
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index e161c28..fe22cb9 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -246,9 +246,9 @@ libxfs_trans_iget(
>   	xfs_inode_log_item_t	*iip;
>   
>   	if (tp == NULL)
> -		return libxfs_iget(mp, tp, ino, lock_flags, ipp);
> +		return libxfs_iget(mp, tp, ino, flags, lock_flags, ipp);
>   
> -	error = libxfs_iget(mp, tp, ino, lock_flags, &ip);
> +	error = libxfs_iget(mp, tp, ino, flags, lock_flags, &ip);
>   	if (error)
>   		return error;
>   	ASSERT(ip != NULL);
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 373b1a5..011bcdf 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -925,7 +925,7 @@ mk_orphanage(xfs_mount_t *mp)
>   	 * would have been cleared in phase3 and phase4.
>   	 */
>   
> -	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, &pip)))
> +	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, 0, &pip)))
>   		do_error(_("%d - couldn't iget root inode to obtain %s\n"),
>   			i, ORPHANAGE);
>   
> @@ -949,7 +949,7 @@ mk_orphanage(xfs_mount_t *mp)
>   	 * use iget/ijoin instead of trans_iget because the ialloc
>   	 * wrapper can commit the transaction and start a new one
>   	 */
> -/*	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, &pip)))
> +/*	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, 0, &pip)))
>   		do_error(_("%d - couldn't iget root inode to make %s\n"),
>   			i, ORPHANAGE);*/
>   
> @@ -1063,7 +1063,7 @@ mv_orphanage(
>   	xname.len = snprintf((char *)fname, sizeof(fname), "%llu",
>   				(unsigned long long)ino);
>   
> -	err = -libxfs_iget(mp, NULL, orphanage_ino, 0, &orphanage_ip);
> +	err = -libxfs_iget(mp, NULL, orphanage_ino, 0, 0, &orphanage_ip);
>   	if (err)
>   		do_error(_("%d - couldn't iget orphanage inode\n"), err);
>   	/*
> @@ -1075,7 +1075,7 @@ mv_orphanage(
>   		xname.len = snprintf((char *)fname, sizeof(fname), "%llu.%d",
>   					(unsigned long long)ino, ++incr);
>   
> -	if ((err = -libxfs_iget(mp, NULL, ino, 0, &ino_p)))
> +	if ((err = -libxfs_iget(mp, NULL, ino, 0, 0, &ino_p)))
>   		do_error(_("%d - couldn't iget disconnected inode\n"), err);
>   
>   	xname.type = xfs_mode_to_ftype(VFS_I(ino_p)->i_mode);
> @@ -2817,7 +2817,7 @@ process_dir_inode(
>   
>   	ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
>   
> -	error = -libxfs_iget(mp, NULL, ino, 0, &ip);
> +	error = -libxfs_iget(mp, NULL, ino, 0, 0, &ip);
>   	if (error) {
>   		if (!no_modify)
>   			do_error(
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH 2/2] libxfs: allow xfs_repair to bypass inode fork read verifiers
  2017-07-18 19:06 ` [PATCH 2/2] libxfs: allow xfs_repair to bypass inode fork read verifiers Darrick J. Wong
@ 2017-07-18 19:51   ` Allison Henderson
  0 siblings, 0 replies; 5+ messages in thread
From: Allison Henderson @ 2017-07-18 19:51 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs, bfoster

This one too.

Reviewed by: Allison Henderson <allison.henderson@oracle.com>

Thanks!

On 07/18/2017 12:06 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> In xfs_repair phase 6, we want to be able to check and fix short form
> directory inodes.  However, if the directory is already corrupt, the
> read verifier prevents us from loading the inode, which prevents us from
> fixing it.  Therefore, allow _iget callers to bypass the read verifier.
> Note that we do /not/ allow bypassing of the write verifier, ever.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   include/xfs_inode.h     |    3 +++
>   libxfs/libxfs_priv.h    |    3 ---
>   libxfs/xfs_inode_buf.c  |    3 ++-
>   libxfs/xfs_inode_fork.c |   15 ++++++++-------
>   libxfs/xfs_inode_fork.h |    2 +-
>   repair/phase6.c         |    2 +-
>   6 files changed, 15 insertions(+), 13 deletions(-)
>
>
> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
> index 5d5158e..b2f2af5 100644
> --- a/include/xfs_inode.h
> +++ b/include/xfs_inode.h
> @@ -148,6 +148,9 @@ extern void	libxfs_trans_ichgtime(struct xfs_trans *,
>   extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>   
>   /* Inode Cache Interfaces */
> +#define XFS_IGET_CREATE			0x1
> +#define XFS_IGET_UNTRUSTED		0x2
> +#define XFS_IGET_SKIP_FORK_VERIFY	0x4
>   extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
>   				uint, uint, struct xfs_inode **);
>   extern void	libxfs_iput(struct xfs_inode *);
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 4f5ba7c..da1cc20 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -103,9 +103,6 @@ extern char    *progname;
>   #define ATTR_KERNOTIME			0
>   #define ATTR_KERNOVAL			0
>   
> -#define XFS_IGET_CREATE			0x1
> -#define XFS_IGET_UNTRUSTED		0x2
> -
>   extern void cmn_err(int, char *, ...);
>   enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
>   
> diff --git a/libxfs/xfs_inode_buf.c b/libxfs/xfs_inode_buf.c
> index b0f2c0d..174a24e 100644
> --- a/libxfs/xfs_inode_buf.c
> +++ b/libxfs/xfs_inode_buf.c
> @@ -522,7 +522,8 @@ xfs_iread(
>   	 */
>   	if (dip->di_mode) {
>   		xfs_inode_from_disk(ip, dip);
> -		error = xfs_iformat_fork(ip, dip);
> +		error = xfs_iformat_fork(ip, dip,
> +				!(iget_flags & XFS_IGET_SKIP_FORK_VERIFY));
>   		if (error)  {
>   #ifdef DEBUG
>   			xfs_alert(mp, "%s: xfs_iformat() returned error %d",
> diff --git a/libxfs/xfs_inode_fork.c b/libxfs/xfs_inode_fork.c
> index 0ef989d..e8379e1 100644
> --- a/libxfs/xfs_inode_fork.c
> +++ b/libxfs/xfs_inode_fork.c
> @@ -50,13 +50,14 @@ STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int);
>    */
>   int
>   xfs_iformat_fork(
> -	xfs_inode_t		*ip,
> -	xfs_dinode_t		*dip)
> +	struct xfs_inode		*ip,
> +	struct xfs_dinode		*dip,
> +	bool				verify_fork)
>   {
> -	xfs_attr_shortform_t	*atp;
> -	int			size;
> -	int			error = 0;
> -	xfs_fsize_t             di_size;
> +	struct xfs_attr_shortform	*atp;
> +	int				size;
> +	int				error = 0;
> +	xfs_fsize_t			di_size;
>   
>   	if (unlikely(be32_to_cpu(dip->di_nextents) +
>   		     be16_to_cpu(dip->di_anextents) >
> @@ -181,7 +182,7 @@ xfs_iformat_fork(
>   		return error;
>   
>   	/* Check inline dir contents. */
> -	if (S_ISDIR(VFS_I(ip)->i_mode) &&
> +	if (verify_fork && S_ISDIR(VFS_I(ip)->i_mode) &&
>   	    dip->di_format == XFS_DINODE_FMT_LOCAL) {
>   		error = xfs_dir2_sf_verify(ip);
>   		if (error) {
> diff --git a/libxfs/xfs_inode_fork.h b/libxfs/xfs_inode_fork.h
> index 7fb8365..7a24288 100644
> --- a/libxfs/xfs_inode_fork.h
> +++ b/libxfs/xfs_inode_fork.h
> @@ -139,7 +139,7 @@ typedef struct xfs_ifork {
>   
>   struct xfs_ifork *xfs_iext_state_to_fork(struct xfs_inode *ip, int state);
>   
> -int		xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *);
> +int		xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *, bool);
>   void		xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
>   				struct xfs_inode_log_item *, int);
>   void		xfs_idestroy_fork(struct xfs_inode *, int);
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 011bcdf..5edfd29 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -2817,7 +2817,7 @@ process_dir_inode(
>   
>   	ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
>   
> -	error = -libxfs_iget(mp, NULL, ino, 0, 0, &ip);
> +	error = -libxfs_iget(mp, NULL, ino, XFS_IGET_SKIP_FORK_VERIFY, 0, &ip);
>   	if (error) {
>   		if (!no_modify)
>   			do_error(
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH 1/2] libxfs: add a flags argument to libxfs_iget
  2017-07-18 19:06 [PATCH 1/2] libxfs: add a flags argument to libxfs_iget Darrick J. Wong
  2017-07-18 19:06 ` [PATCH 2/2] libxfs: allow xfs_repair to bypass inode fork read verifiers Darrick J. Wong
  2017-07-18 19:50 ` [PATCH 1/2] libxfs: add a flags argument to libxfs_iget Allison Henderson
@ 2017-07-19  2:24 ` Darrick J. Wong
  2 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2017-07-19  2:24 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs, bfoster

On Tue, Jul 18, 2017 at 12:06:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a flags argument to libxfs_iget to bring it up to date with the
> kernel xfs_iget.  We will use the flags argument in the next patch
> to enable xfs_repair to shut off inode fork verifiers.

After chatting with Dave it occured to us that we really ought to have
verifiers for shortform attrs and inline symlinks.  From there, it seems
that the kernel could create a helper to verify an inode's inline forks.
This helper would be called from xfs_iget_cache_miss and xfs_iflush_int.

For xfsprogs we can modify libxfs_iget to take a pointer to a structure
full of ifork verifiers so that regular programs can just call the
library versions, and xfs_repair can supply its own for the special
sauce things it wants to do.  libxfs_iflush_int would of course call the
standard libxfs ifork verifiers the same as the kernel, as we never want
corrupt metadata hitting the disk.

Will send an rfc later...

--D

> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  db/attrset.c        |    4 ++--
>  include/xfs_inode.h |    2 +-
>  libxfs/rdwr.c       |   15 ++++++++++-----
>  libxfs/trans.c      |    4 ++--
>  repair/phase6.c     |   10 +++++-----
>  5 files changed, 20 insertions(+), 15 deletions(-)
> 
> 
> diff --git a/db/attrset.c b/db/attrset.c
> index ad3c8f3..ddd72ed 100644
> --- a/db/attrset.c
> +++ b/db/attrset.c
> @@ -151,7 +151,7 @@ attr_set_f(
>  		value = NULL;
>  	}
>  
> -	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip)) {
> +	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, 0, &ip)) {
>  		dbprintf(_("failed to iget inode %llu\n"),
>  			(unsigned long long)iocur_top->ino);
>  		goto out;
> @@ -226,7 +226,7 @@ attr_remove_f(
>  
>  	name = argv[optind];
>  
> -	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip)) {
> +	if (libxfs_iget(mp, NULL, iocur_top->ino, 0, 0, &ip)) {
>  		dbprintf(_("failed to iget inode %llu\n"),
>  			(unsigned long long)iocur_top->ino);
>  		goto out;
> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
> index 8766024..5d5158e 100644
> --- a/include/xfs_inode.h
> +++ b/include/xfs_inode.h
> @@ -149,7 +149,7 @@ extern int	libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>  
>  /* Inode Cache Interfaces */
>  extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
> -				uint, struct xfs_inode **);
> +				uint, uint, struct xfs_inode **);
>  extern void	libxfs_iput(struct xfs_inode *);
>  
>  #define IRELE(ip) libxfs_iput(ip)
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 43b4f1d..cf156ba 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -1329,11 +1329,16 @@ extern kmem_zone_t	*xfs_ili_zone;
>  extern kmem_zone_t	*xfs_inode_zone;
>  
>  int
> -libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
> -		xfs_inode_t **ipp)
> +libxfs_iget(
> +	struct xfs_mount	*mp,
> +	struct xfs_trans	*tp,
> +	xfs_ino_t		ino,
> +	uint			flags,
> +	uint			lock_flags,
> +	struct xfs_inode	**ipp)
>  {
> -	xfs_inode_t	*ip;
> -	int		error = 0;
> +	struct xfs_inode	*ip;
> +	int			error = 0;
>  
>  	ip = kmem_zone_zalloc(xfs_inode_zone, 0);
>  	if (!ip)
> @@ -1341,7 +1346,7 @@ libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
>  
>  	ip->i_ino = ino;
>  	ip->i_mount = mp;
> -	error = xfs_iread(mp, tp, ip, 0);
> +	error = xfs_iread(mp, tp, ip, flags);
>  	if (error) {
>  		kmem_zone_free(xfs_inode_zone, ip);
>  		*ipp = NULL;
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index e161c28..fe22cb9 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -246,9 +246,9 @@ libxfs_trans_iget(
>  	xfs_inode_log_item_t	*iip;
>  
>  	if (tp == NULL)
> -		return libxfs_iget(mp, tp, ino, lock_flags, ipp);
> +		return libxfs_iget(mp, tp, ino, flags, lock_flags, ipp);
>  
> -	error = libxfs_iget(mp, tp, ino, lock_flags, &ip);
> +	error = libxfs_iget(mp, tp, ino, flags, lock_flags, &ip);
>  	if (error)
>  		return error;
>  	ASSERT(ip != NULL);
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 373b1a5..011bcdf 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -925,7 +925,7 @@ mk_orphanage(xfs_mount_t *mp)
>  	 * would have been cleared in phase3 and phase4.
>  	 */
>  
> -	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, &pip)))
> +	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, 0, &pip)))
>  		do_error(_("%d - couldn't iget root inode to obtain %s\n"),
>  			i, ORPHANAGE);
>  
> @@ -949,7 +949,7 @@ mk_orphanage(xfs_mount_t *mp)
>  	 * use iget/ijoin instead of trans_iget because the ialloc
>  	 * wrapper can commit the transaction and start a new one
>  	 */
> -/*	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, &pip)))
> +/*	if ((i = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, 0, &pip)))
>  		do_error(_("%d - couldn't iget root inode to make %s\n"),
>  			i, ORPHANAGE);*/
>  
> @@ -1063,7 +1063,7 @@ mv_orphanage(
>  	xname.len = snprintf((char *)fname, sizeof(fname), "%llu",
>  				(unsigned long long)ino);
>  
> -	err = -libxfs_iget(mp, NULL, orphanage_ino, 0, &orphanage_ip);
> +	err = -libxfs_iget(mp, NULL, orphanage_ino, 0, 0, &orphanage_ip);
>  	if (err)
>  		do_error(_("%d - couldn't iget orphanage inode\n"), err);
>  	/*
> @@ -1075,7 +1075,7 @@ mv_orphanage(
>  		xname.len = snprintf((char *)fname, sizeof(fname), "%llu.%d",
>  					(unsigned long long)ino, ++incr);
>  
> -	if ((err = -libxfs_iget(mp, NULL, ino, 0, &ino_p)))
> +	if ((err = -libxfs_iget(mp, NULL, ino, 0, 0, &ino_p)))
>  		do_error(_("%d - couldn't iget disconnected inode\n"), err);
>  
>  	xname.type = xfs_mode_to_ftype(VFS_I(ino_p)->i_mode);
> @@ -2817,7 +2817,7 @@ process_dir_inode(
>  
>  	ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
>  
> -	error = -libxfs_iget(mp, NULL, ino, 0, &ip);
> +	error = -libxfs_iget(mp, NULL, ino, 0, 0, &ip);
>  	if (error) {
>  		if (!no_modify)
>  			do_error(
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2017-07-19  2:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-18 19:06 [PATCH 1/2] libxfs: add a flags argument to libxfs_iget Darrick J. Wong
2017-07-18 19:06 ` [PATCH 2/2] libxfs: allow xfs_repair to bypass inode fork read verifiers Darrick J. Wong
2017-07-18 19:51   ` Allison Henderson
2017-07-18 19:50 ` [PATCH 1/2] libxfs: add a flags argument to libxfs_iget Allison Henderson
2017-07-19  2:24 ` 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).