Re: [PATCH 1/7] e2fsck: Clarify overflow link count error message

[Andreas Dilger <adilger@dilger.ca>]

[-- Attachment #1: Type: text/plain, Size: 3580 bytes --]

On Feb 13, 2020, at 3:15 AM, Jan Kara <jack@suse.cz> wrote:
> 
> When directory link count is set to overflow value (1) but during pass 4
> we find out the exact link count would fit, we either silently fix this
> (which is not great because e2fsck then reports the fs was modified but
> output doesn't indicate why in any way), or we report that link count is
> wrong and ask whether we should fix it (in case -n option was
> specified). The second case is even more misleading because it suggests
> non-trivial fs corruption which then gets silently fixed on the next
> run. Similarly to how we fix up other non-problems, just create a new
> error message for the case directory link count is not overflown anymore
> and always report it to clarify what is going on.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> e2fsck/pass4.c   | 20 ++++++++++++++++----
> e2fsck/problem.c |  5 +++++
> e2fsck/problem.h |  3 +++
> 3 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/e2fsck/pass4.c b/e2fsck/pass4.c
> index 10be7f87180d..8c2d2f1fca12 100644
> --- a/e2fsck/pass4.c
> +++ b/e2fsck/pass4.c
> @@ -237,6 +237,8 @@ void e2fsck_pass4(e2fsck_t ctx)
> 			link_counted = 1;
> 		}
> 		if (link_counted != link_count) {
> +			int fix_nlink = 0;
> +
> 			e2fsck_read_inode_full(ctx, i, EXT2_INODE(inode),
> 					       inode_size, "pass4");
> 			pctx.ino = i;
> @@ -250,10 +252,20 @@ void e2fsck_pass4(e2fsck_t ctx)
> 			pctx.num = link_counted;
> 			/* i_link_count was previously exceeded, but no longer
> 			 * is, fix this but don't consider it an error */
> -			if ((isdir && link_counted > 1 &&
> -			     (inode->i_flags & EXT2_INDEX_FL) &&
> -			     link_count == 1 && !(ctx->options & E2F_OPT_NO)) ||
> -			    fix_problem(ctx, PR_4_BAD_REF_COUNT, &pctx)) {
> +			if (isdir && link_counted > 1 &&
> +			    (inode->i_flags & EXT2_INDEX_FL) &&
> +			    link_count == 1) {
> +				if ((ctx->options & E2F_OPT_READONLY) == 0) {
> +					fix_nlink =
> +						fix_problem(ctx,
> +							PR_4_DIR_OVERFLOW_REF_COUNT,
> +							&pctx);
> +				}
> +			} else {
> +				fix_nlink = fix_problem(ctx, PR_4_BAD_REF_COUNT,
> +						&pctx);
> +			}
> +			if (fix_nlink) {
> 				inode->i_links_count = link_counted;
> 				e2fsck_write_inode_full(ctx, i,
> 							EXT2_INODE(inode),
> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index c7c0ba986006..e79c853b2096 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -2035,6 +2035,11 @@ static struct e2fsck_problem problem_table[] = {
> 	  N_("@d exceeds max links, but no DIR_NLINK feature in @S.\n"),
> 	  PROMPT_FIX, 0, 0, 0, 0 },
> 
> +	/* Directory inode ref count set to overflow but could be exact value */
> +	{ PR_4_DIR_OVERFLOW_REF_COUNT,
> +	  N_("@d @i %i ref count set to overflow but could be exact value %N.  "),
> +	  PROMPT_FIX, PR_PREEN_OK, 0, 0, 0 },
> +
> 	/* Pass 5 errors */
> 
> 	/* Pass 5: Checking group summary information */
> diff --git a/e2fsck/problem.h b/e2fsck/problem.h
> index c7f65f6dee0f..4185e5175cab 100644
> --- a/e2fsck/problem.h
> +++ b/e2fsck/problem.h
> @@ -1164,6 +1164,9 @@ struct problem_context {
> /* directory exceeds max links, but no DIR_NLINK feature in superblock */
> #define PR_4_DIR_NLINK_FEATURE		0x040006
> 
> +/* Directory ref count set to overflow but it doesn't have to be */
> +#define PR_4_DIR_OVERFLOW_REF_COUNT	0x040007
> +
> /*
>  * Pass 5 errors
>  */
> --
> 2.16.4
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]