Re: [PATCH 20/28] xfs: report iunlink recovery failure upwards

[Chandan Babu R <chandanrlinux@gmail.com>]

On Tuesday 5 May 2020 6:42:41 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> If we fail to recover unlinked inodes due to corruption or whatnot, we
> should report this upwards and fail the mount instead of continuing on
> like nothing's wrong.  Eventually the user will trip over the busted
> AGI anyway.

The changes look good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_log_recover.h |    2 +-
>  fs/xfs/xfs_log.c                |    4 +++-
>  fs/xfs/xfs_log_recover.c        |    7 ++++++-
>  fs/xfs/xfs_unlink_recover.c     |    4 +++-
>  4 files changed, 13 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
> index 33c14dd22b77..d4d6d4f84fda 100644
> --- a/fs/xfs/libxfs/xfs_log_recover.h
> +++ b/fs/xfs/libxfs/xfs_log_recover.h
> @@ -124,6 +124,6 @@ bool xlog_add_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len);
>  bool xlog_is_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len);
>  bool xlog_put_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len);
>  void xlog_recover_iodone(struct xfs_buf *bp);
> -void xlog_recover_process_unlinked(struct xlog *log);
> +int xlog_recover_process_unlinked(struct xlog *log);
>  
>  #endif	/* __XFS_LOG_RECOVER_H__ */
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 00fda2e8e738..8203b9b0fd08 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -727,6 +727,8 @@ xfs_log_mount_finish(
>  		xfs_log_work_queue(mp);
>  	mp->m_super->s_flags &= ~SB_ACTIVE;
>  	evict_inodes(mp->m_super);
> +	if (error)
> +		return error;
>  
>  	/*
>  	 * Drain the buffer LRU after log recovery. This is required for v4
> @@ -737,7 +739,7 @@ xfs_log_mount_finish(
>  	 * Don't push in the error case because the AIL may have pending intents
>  	 * that aren't removed until recovery is cancelled.
>  	 */
> -	if (!error && recovered) {
> +	if (recovered) {
>  		xfs_log_force(mp, XFS_LOG_SYNC);
>  		xfs_ail_push_all_sync(mp->m_ail);
>  	}
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 362296b34490..0ccc09c004f1 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3399,7 +3399,12 @@ xlog_recover_finish(
>  		 */
>  		xfs_log_force(log->l_mp, XFS_LOG_SYNC);
>  
> -		xlog_recover_process_unlinked(log);
> +		error = xlog_recover_process_unlinked(log);
> +		if (error) {
> +			xfs_alert(log->l_mp,
> +					"Failed to recover unlinked metadata");
> +			return error;
> +		}
>  
>  		xlog_recover_check_summary(log);
>  
> diff --git a/fs/xfs/xfs_unlink_recover.c b/fs/xfs/xfs_unlink_recover.c
> index 413b34085640..fe7fa3d623f2 100644
> --- a/fs/xfs/xfs_unlink_recover.c
> +++ b/fs/xfs/xfs_unlink_recover.c
> @@ -195,7 +195,7 @@ xlog_recover_process_iunlinked(
>  	return 0;
>  }
>  
> -void
> +int
>  xlog_recover_process_unlinked(
>  	struct xlog		*log)
>  {
> @@ -208,4 +208,6 @@ xlog_recover_process_unlinked(
>  		if (error)
>  			break;
>  	}
> +
> +	return error;
>  }
> 
> 


-- 
chandan