linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fix "fsck -A" failure on a completely clean fs
@ 2019-05-10 14:19 Denys Vlasenko
  2019-05-10 19:50 ` Theodore Ts'o
  0 siblings, 1 reply; 2+ messages in thread
From: Denys Vlasenko @ 2019-05-10 14:19 UTC (permalink / raw)
  To: linux-ext4

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

Before remounting root fs and mounting local filesystems
in /etc/fstab, my boot scripts check them for errors with:

if ! fsck -A; then
         echo "fsck exit code: $?. Boot will not continue."
         while true; do sleep 9999; done
fi
mount -o remount,rw /
mount -a

Looks like something very straightforward, right?

I have two filesystems in /etc/fstab:
/dev/sda2 /                       ext4    defaults        1 1
/dev/sda1 /boot                   ext4    defaults        1 2

If I use fsck from util-linux-2.33.2-1.fc31.x86_64 (IOW: rather recent code),
it starts two instances of fsck.ext4 (all is fine with it).

The second one's stdio is redirected (probably to /dev/null),
it is no longer the tty. (Which is fine too).

But now we hit a problem. Second fsck.ext4 flat out refuses to do its job,
even before it looks at the filesystem.

Therefore, this condition does not trigger:
         if (getenv("E2FSCK_FORCE_INTERACTIVE") || (isatty(0) && isatty(1))) {
                 ctx->interactive = 1;
         }
and ctx->interactive stays 0.
Therefore, later in main() fsck.ext4 dies with this message:
         if (!(ctx->options & E2F_OPT_PREEN) &&
             !(ctx->options & E2F_OPT_NO) &&
             !(ctx->options & E2F_OPT_YES)) {
                 if (!ctx->interactive)
                         fatal_error(ctx,
                                     _("need terminal for interactive repairs"));
         }

This happens BEFORE any repairs are deemed necessary, IOW: it happens ALWAYS,
even if filesystem is completely fine.

IOW: "fsck -A" is *completely unusable* in this scenario.
I believe this is wrong. It is intended to be the generic, fs-agnostic way
to run fsck's on all /etc/fstab filesystems.

I propose to change the code so that this abort happens only if we
indeed need to interactively ask something.

Tested patch attached.

Fedora BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1702342


[-- Attachment #2: interactive_bugout.patch --]
[-- Type: text/x-patch, Size: 1231 bytes --]

diff -uprN e2fsprogs-1.44.4.orig/e2fsck/unix.c e2fsprogs-1.44.4/e2fsck/unix.c
--- e2fsprogs-1.44.4.orig/e2fsck/unix.c	2018-08-19 04:26:58.000000000 +0200
+++ e2fsprogs-1.44.4/e2fsck/unix.c	2019-04-23 15:38:55.890507270 +0200
@@ -1439,13 +1439,6 @@ int main (int argc, char *argv[])
 
 	check_mount(ctx);
 
-	if (!(ctx->options & E2F_OPT_PREEN) &&
-	    !(ctx->options & E2F_OPT_NO) &&
-	    !(ctx->options & E2F_OPT_YES)) {
-		if (!ctx->interactive)
-			fatal_error(ctx,
-				    _("need terminal for interactive repairs"));
-	}
 	ctx->superblock = ctx->use_superblock;
 
 	flags = EXT2_FLAG_SKIP_MMP;
diff -uprN e2fsprogs-1.44.4.orig/e2fsck/util.c e2fsprogs-1.44.4/e2fsck/util.c
--- e2fsprogs-1.44.4.orig/e2fsck/util.c	2018-08-19 04:26:58.000000000 +0200
+++ e2fsprogs-1.44.4/e2fsck/util.c	2019-04-23 15:39:27.571448855 +0200
@@ -203,6 +203,14 @@ int ask_yn(e2fsck_t ctx, const char * st
 	const char	*extra_prompt = "";
 	static int	yes_answers;
 
+	if (!(ctx->options & E2F_OPT_PREEN) &&
+	    !(ctx->options & E2F_OPT_NO) &&
+	    !(ctx->options & E2F_OPT_YES)) {
+		if (!ctx->interactive)
+			fatal_error(ctx,
+				    _("need terminal for interactive repairs"));
+	}
+
 #ifdef HAVE_TERMIOS_H
 	struct termios	termios, tmp;
 

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

* Re: [PATCH] fix "fsck -A" failure on a completely clean fs
  2019-05-10 14:19 [PATCH] fix "fsck -A" failure on a completely clean fs Denys Vlasenko
@ 2019-05-10 19:50 ` Theodore Ts'o
  0 siblings, 0 replies; 2+ messages in thread
From: Theodore Ts'o @ 2019-05-10 19:50 UTC (permalink / raw)
  To: Denys Vlasenko; +Cc: linux-ext4

I've replied on the bug:

	https://bugzilla.redhat.com/show_bug.cgi?id=1702342#c9

TL;DR: NACK

In theory you might be able to patch up e2fsck so that fsck -A
actually would work for ext[234].  It is not obvious to me that it
will work for other file systems, however --- due to some very subtle
issues such as the ones I pointed out in my reply.

Cheers,

					- Ted


On Fri, May 10, 2019 at 04:19:47PM +0200, Denys Vlasenko wrote:
> Before remounting root fs and mounting local filesystems
> in /etc/fstab, my boot scripts check them for errors with:
> 
> if ! fsck -A; then
>         echo "fsck exit code: $?. Boot will not continue."
>         while true; do sleep 9999; done
> fi
> mount -o remount,rw /
> mount -a
> 
> Looks like something very straightforward, right?
> 
> I have two filesystems in /etc/fstab:
> /dev/sda2 /                       ext4    defaults        1 1
> /dev/sda1 /boot                   ext4    defaults        1 2
> 
> If I use fsck from util-linux-2.33.2-1.fc31.x86_64 (IOW: rather recent code),
> it starts two instances of fsck.ext4 (all is fine with it).
> 
> The second one's stdio is redirected (probably to /dev/null),
> it is no longer the tty. (Which is fine too).
> 
> But now we hit a problem. Second fsck.ext4 flat out refuses to do its job,
> even before it looks at the filesystem.
> 
> Therefore, this condition does not trigger:
>         if (getenv("E2FSCK_FORCE_INTERACTIVE") || (isatty(0) && isatty(1))) {
>                 ctx->interactive = 1;
>         }
> and ctx->interactive stays 0.
> Therefore, later in main() fsck.ext4 dies with this message:
>         if (!(ctx->options & E2F_OPT_PREEN) &&
>             !(ctx->options & E2F_OPT_NO) &&
>             !(ctx->options & E2F_OPT_YES)) {
>                 if (!ctx->interactive)
>                         fatal_error(ctx,
>                                     _("need terminal for interactive repairs"));
>         }
> 
> This happens BEFORE any repairs are deemed necessary, IOW: it happens ALWAYS,
> even if filesystem is completely fine.
> 
> IOW: "fsck -A" is *completely unusable* in this scenario.
> I believe this is wrong. It is intended to be the generic, fs-agnostic way
> to run fsck's on all /etc/fstab filesystems.
> 
> I propose to change the code so that this abort happens only if we
> indeed need to interactively ask something.
> 
> Tested patch attached.
> 
> Fedora BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1702342
> 

> diff -uprN e2fsprogs-1.44.4.orig/e2fsck/unix.c e2fsprogs-1.44.4/e2fsck/unix.c
> --- e2fsprogs-1.44.4.orig/e2fsck/unix.c	2018-08-19 04:26:58.000000000 +0200
> +++ e2fsprogs-1.44.4/e2fsck/unix.c	2019-04-23 15:38:55.890507270 +0200
> @@ -1439,13 +1439,6 @@ int main (int argc, char *argv[])
>  
>  	check_mount(ctx);
>  
> -	if (!(ctx->options & E2F_OPT_PREEN) &&
> -	    !(ctx->options & E2F_OPT_NO) &&
> -	    !(ctx->options & E2F_OPT_YES)) {
> -		if (!ctx->interactive)
> -			fatal_error(ctx,
> -				    _("need terminal for interactive repairs"));
> -	}
>  	ctx->superblock = ctx->use_superblock;
>  
>  	flags = EXT2_FLAG_SKIP_MMP;
> diff -uprN e2fsprogs-1.44.4.orig/e2fsck/util.c e2fsprogs-1.44.4/e2fsck/util.c
> --- e2fsprogs-1.44.4.orig/e2fsck/util.c	2018-08-19 04:26:58.000000000 +0200
> +++ e2fsprogs-1.44.4/e2fsck/util.c	2019-04-23 15:39:27.571448855 +0200
> @@ -203,6 +203,14 @@ int ask_yn(e2fsck_t ctx, const char * st
>  	const char	*extra_prompt = "";
>  	static int	yes_answers;
>  
> +	if (!(ctx->options & E2F_OPT_PREEN) &&
> +	    !(ctx->options & E2F_OPT_NO) &&
> +	    !(ctx->options & E2F_OPT_YES)) {
> +		if (!ctx->interactive)
> +			fatal_error(ctx,
> +				    _("need terminal for interactive repairs"));
> +	}
> +
>  #ifdef HAVE_TERMIOS_H
>  	struct termios	termios, tmp;
>  


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

end of thread, other threads:[~2019-05-10 19:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-10 14:19 [PATCH] fix "fsck -A" failure on a completely clean fs Denys Vlasenko
2019-05-10 19:50 ` Theodore Ts'o

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).