All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time
@ 2015-03-18 20:15 Eric Sandeen
  2015-03-18 20:40 ` Jan Kara
  2015-03-29  2:32 ` Theodore Ts'o
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Sandeen @ 2015-03-18 20:15 UTC (permalink / raw)
  To: ext4 development; +Cc: Jan Kara

In some cases, particularly with RTC set to localtime, we may have
timestamps in the superblock off by up to 24 hours.

Today, even if accept_time_fudge is set, we find the problem, alert
the user, fix it, and set E2F_FLAG_PROBLEMS_FIXED in fix_problem().

This flag causes check_if_skip() to return (not exit), and a full
check ensues.  This will happen on every single boot.

This runs counter to what the man page implies; the text there sounds
like time offsets of up to 24h will be ignored.  But in truth, the
only difference accept_time_fudge makes today is to change the message
presented to the user.  Actual action taken by e2fsck is identical.

So: Change the code so that last-mount and last-write times are
completely ignored if they are within 24h (the fudge_time), and
clarify the man page to show that this is the case.

If accept_fudge_time is set to false, ctx->fudge_time will be 0,
and no leeway will be given.

(As a historical note, both Fedora and OpenSUSE had set
broken_system_clock to 1 to work around this behavior in the past,
but that defeated all time-based checks).

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/e2fsck/e2fsck.conf.5.in b/e2fsck/e2fsck.conf.5.in
index 9ebfbbf..515af37 100644
--- a/e2fsck/e2fsck.conf.5.in
+++ b/e2fsck/e2fsck.conf.5.in
@@ -103,6 +103,7 @@ buggy or misconfigured virtualization manager or the
 installer not having access to a network time server
 during the installation process.  So by default, we allow
 the superblock times to be fudged by up to 24 hours.
+No repair action will be taken in this case.
 This can be disabled by setting
 .I accept_time_fudge
 to the
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index a63e61c..c05ba4b 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -383,18 +383,6 @@ static struct e2fsck_problem problem_table[] = {
 	  N_("The test_fs flag is set (and ext4 is available).  "),
 	  PROMPT_CLEAR, PR_PREEN_OK },
 
-	/* Last mount time is in the future (fudged) */
-	{ PR_0_FUTURE_SB_LAST_MOUNT_FUDGED,
-	  N_("@S last mount time is in the future.\n\t(by less than a day, "
-	     "probably due to the hardware clock being incorrectly set)  "),
-	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
-
-	/* Last write time is in the future (fudged) */
-	{ PR_0_FUTURE_SB_LAST_WRITE_FUDGED,
-	  N_("@S last write time is in the future.\n\t(by less than a day, "
-	     "probably due to the hardware clock being incorrectly set).  "),
-	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
-
 	/* Block group checksum (latch question) is invalid. */
 	{ PR_0_GDT_CSUM_LATCH,
 	  N_("One or more @b @g descriptor checksums are invalid.  "),
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index 3c28166..4e8d76e 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -219,11 +219,11 @@ struct problem_context {
 /* The test_fs filesystem flag is set and ext4 is available */
 #define PR_0_CLEAR_TESTFS_FLAG			0x00003B
 
-/* Last mount time is in the future (fudged) */
-#define PR_0_FUTURE_SB_LAST_MOUNT_FUDGED	0x00003C
+/* Last mount time is in the future (fudged) -- NO LONGER USED */
+/* #define PR_0_FUTURE_SB_LAST_MOUNT_FUDGED	0x00003C */
 
-/* Last write time is in the future (fudged) */
-#define PR_0_FUTURE_SB_LAST_WRITE_FUDGED	0x00003D
+/* Last write time is in the future (fudged) -- NO LONGER USED */
+/* #define PR_0_FUTURE_SB_LAST_WRITE_FUDGED	0x00003D */
 
 /* Block group checksum (latch question) */
 #define PR_0_GDT_CSUM_LATCH			0x00003E
diff --git a/e2fsck/super.c b/e2fsck/super.c
index 1e7e749..4d7a3bf 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -856,11 +856,9 @@ void check_super_block(e2fsck_t ctx)
 	 */
 	if (!broken_system_clock &&
 	    !(ctx->flags & E2F_FLAG_TIME_INSANE) &&
-	    fs->super->s_mtime > (__u32) ctx->now) {
+	    fs->super->s_mtime > (__u32) ctx->now + ctx->time_fudge) {
 		pctx.num = fs->super->s_mtime;
 		problem = PR_0_FUTURE_SB_LAST_MOUNT;
-		if (fs->super->s_mtime <= (__u32) ctx->now + ctx->time_fudge)
-			problem = PR_0_FUTURE_SB_LAST_MOUNT_FUDGED;
 		if (fix_problem(ctx, problem, &pctx)) {
 			fs->super->s_mtime = ctx->now;
 			fs->flags |= EXT2_FLAG_DIRTY;
@@ -868,11 +866,9 @@ void check_super_block(e2fsck_t ctx)
 	}
 	if (!broken_system_clock &&
 	    !(ctx->flags & E2F_FLAG_TIME_INSANE) &&
-	    fs->super->s_wtime > (__u32) ctx->now) {
+	    fs->super->s_wtime > (__u32) ctx->now + ctx->time_fudge) {
 		pctx.num = fs->super->s_wtime;
 		problem = PR_0_FUTURE_SB_LAST_WRITE;
-		if (fs->super->s_wtime <= (__u32) ctx->now + ctx->time_fudge)
-			problem = PR_0_FUTURE_SB_LAST_WRITE_FUDGED;
 		if (fix_problem(ctx, problem, &pctx)) {
 			fs->super->s_wtime = ctx->now;
 			fs->flags |= EXT2_FLAG_DIRTY;



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time
  2015-03-18 20:15 [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time Eric Sandeen
@ 2015-03-18 20:40 ` Jan Kara
  2015-03-29  2:32 ` Theodore Ts'o
  1 sibling, 0 replies; 6+ messages in thread
From: Jan Kara @ 2015-03-18 20:40 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: ext4 development, Jan Kara

On Wed 18-03-15 15:15:32, Eric Sandeen wrote:
> In some cases, particularly with RTC set to localtime, we may have
> timestamps in the superblock off by up to 24 hours.
> 
> Today, even if accept_time_fudge is set, we find the problem, alert
> the user, fix it, and set E2F_FLAG_PROBLEMS_FIXED in fix_problem().
> 
> This flag causes check_if_skip() to return (not exit), and a full
> check ensues.  This will happen on every single boot.
> 
> This runs counter to what the man page implies; the text there sounds
> like time offsets of up to 24h will be ignored.  But in truth, the
> only difference accept_time_fudge makes today is to change the message
> presented to the user.  Actual action taken by e2fsck is identical.
> 
> So: Change the code so that last-mount and last-write times are
> completely ignored if they are within 24h (the fudge_time), and
> clarify the man page to show that this is the case.
> 
> If accept_fudge_time is set to false, ctx->fudge_time will be 0,
> and no leeway will be given.
> 
> (As a historical note, both Fedora and OpenSUSE had set
> broken_system_clock to 1 to work around this behavior in the past,
> but that defeated all time-based checks).
> 
  Looks good. You can add:
Acked-by: Jan Kara <jack@suse.cz>

							Honza

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/e2fsck/e2fsck.conf.5.in b/e2fsck/e2fsck.conf.5.in
> index 9ebfbbf..515af37 100644
> --- a/e2fsck/e2fsck.conf.5.in
> +++ b/e2fsck/e2fsck.conf.5.in
> @@ -103,6 +103,7 @@ buggy or misconfigured virtualization manager or the
>  installer not having access to a network time server
>  during the installation process.  So by default, we allow
>  the superblock times to be fudged by up to 24 hours.
> +No repair action will be taken in this case.
>  This can be disabled by setting
>  .I accept_time_fudge
>  to the
> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index a63e61c..c05ba4b 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -383,18 +383,6 @@ static struct e2fsck_problem problem_table[] = {
>  	  N_("The test_fs flag is set (and ext4 is available).  "),
>  	  PROMPT_CLEAR, PR_PREEN_OK },
>  
> -	/* Last mount time is in the future (fudged) */
> -	{ PR_0_FUTURE_SB_LAST_MOUNT_FUDGED,
> -	  N_("@S last mount time is in the future.\n\t(by less than a day, "
> -	     "probably due to the hardware clock being incorrectly set)  "),
> -	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
> -
> -	/* Last write time is in the future (fudged) */
> -	{ PR_0_FUTURE_SB_LAST_WRITE_FUDGED,
> -	  N_("@S last write time is in the future.\n\t(by less than a day, "
> -	     "probably due to the hardware clock being incorrectly set).  "),
> -	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
> -
>  	/* Block group checksum (latch question) is invalid. */
>  	{ PR_0_GDT_CSUM_LATCH,
>  	  N_("One or more @b @g descriptor checksums are invalid.  "),
> diff --git a/e2fsck/problem.h b/e2fsck/problem.h
> index 3c28166..4e8d76e 100644
> --- a/e2fsck/problem.h
> +++ b/e2fsck/problem.h
> @@ -219,11 +219,11 @@ struct problem_context {
>  /* The test_fs filesystem flag is set and ext4 is available */
>  #define PR_0_CLEAR_TESTFS_FLAG			0x00003B
>  
> -/* Last mount time is in the future (fudged) */
> -#define PR_0_FUTURE_SB_LAST_MOUNT_FUDGED	0x00003C
> +/* Last mount time is in the future (fudged) -- NO LONGER USED */
> +/* #define PR_0_FUTURE_SB_LAST_MOUNT_FUDGED	0x00003C */
>  
> -/* Last write time is in the future (fudged) */
> -#define PR_0_FUTURE_SB_LAST_WRITE_FUDGED	0x00003D
> +/* Last write time is in the future (fudged) -- NO LONGER USED */
> +/* #define PR_0_FUTURE_SB_LAST_WRITE_FUDGED	0x00003D */
>  
>  /* Block group checksum (latch question) */
>  #define PR_0_GDT_CSUM_LATCH			0x00003E
> diff --git a/e2fsck/super.c b/e2fsck/super.c
> index 1e7e749..4d7a3bf 100644
> --- a/e2fsck/super.c
> +++ b/e2fsck/super.c
> @@ -856,11 +856,9 @@ void check_super_block(e2fsck_t ctx)
>  	 */
>  	if (!broken_system_clock &&
>  	    !(ctx->flags & E2F_FLAG_TIME_INSANE) &&
> -	    fs->super->s_mtime > (__u32) ctx->now) {
> +	    fs->super->s_mtime > (__u32) ctx->now + ctx->time_fudge) {
>  		pctx.num = fs->super->s_mtime;
>  		problem = PR_0_FUTURE_SB_LAST_MOUNT;
> -		if (fs->super->s_mtime <= (__u32) ctx->now + ctx->time_fudge)
> -			problem = PR_0_FUTURE_SB_LAST_MOUNT_FUDGED;
>  		if (fix_problem(ctx, problem, &pctx)) {
>  			fs->super->s_mtime = ctx->now;
>  			fs->flags |= EXT2_FLAG_DIRTY;
> @@ -868,11 +866,9 @@ void check_super_block(e2fsck_t ctx)
>  	}
>  	if (!broken_system_clock &&
>  	    !(ctx->flags & E2F_FLAG_TIME_INSANE) &&
> -	    fs->super->s_wtime > (__u32) ctx->now) {
> +	    fs->super->s_wtime > (__u32) ctx->now + ctx->time_fudge) {
>  		pctx.num = fs->super->s_wtime;
>  		problem = PR_0_FUTURE_SB_LAST_WRITE;
> -		if (fs->super->s_wtime <= (__u32) ctx->now + ctx->time_fudge)
> -			problem = PR_0_FUTURE_SB_LAST_WRITE_FUDGED;
>  		if (fix_problem(ctx, problem, &pctx)) {
>  			fs->super->s_wtime = ctx->now;
>  			fs->flags |= EXT2_FLAG_DIRTY;
> 
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time
  2015-03-18 20:15 [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time Eric Sandeen
  2015-03-18 20:40 ` Jan Kara
@ 2015-03-29  2:32 ` Theodore Ts'o
  2015-04-01 13:06   ` Jan Kara
  1 sibling, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2015-03-29  2:32 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: ext4 development, Jan Kara

I'm a bit more partial to this method of fixing the issue.

What do folks think?

      	       	       	       	      	 - Ted
					 
commit f096708126412c0569e40cfbd5740729976bf12a
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Sat Mar 28 21:39:54 2015 -0400

    e2fsck: use PROMPT_NONE for FUTURE_SB_LAST_*_FUDGED problems
    
    This allows us to print a message warning the user that there is
    something funny going on with their hardware clock (probably time zone
    issues caused by trying to be compatible with legacy OS's such as
    Windows), without triggering a full file system check.
    
    Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 12cf836..f442a33 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -386,14 +386,14 @@ static struct e2fsck_problem problem_table[] = {
 	/* Last mount time is in the future (fudged) */
 	{ PR_0_FUTURE_SB_LAST_MOUNT_FUDGED,
 	  N_("@S last mount time is in the future.\n\t(by less than a day, "
-	     "probably due to the hardware clock being incorrectly set)  "),
-	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
+	     "probably due to the hardware clock being incorrectly set)\n"),
+	  PROMPT_NONE, PR_PREEN_OK | PR_NO_OK },
 
 	/* Last write time is in the future (fudged) */
 	{ PR_0_FUTURE_SB_LAST_WRITE_FUDGED,
 	  N_("@S last write time is in the future.\n\t(by less than a day, "
-	     "probably due to the hardware clock being incorrectly set).  "),
-	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
+	     "probably due to the hardware clock being incorrectly set)\n"),
+	  PROMPT_NONE, PR_PREEN_OK | PR_NO_OK },
 
 	/* Block group checksum (latch question) is invalid. */
 	{ PR_0_GDT_CSUM_LATCH,

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time
  2015-03-29  2:32 ` Theodore Ts'o
@ 2015-04-01 13:06   ` Jan Kara
  2015-04-01 15:08     ` Theodore Ts'o
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2015-04-01 13:06 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Eric Sandeen, ext4 development, Jan Kara

On Sat 28-03-15 22:32:26, Ted Tso wrote:
> I'm a bit more partial to this method of fixing the issue.
> 
> What do folks think?
> 
>       	       	       	       	      	 - Ted
> 					 
> commit f096708126412c0569e40cfbd5740729976bf12a
> Author: Theodore Ts'o <tytso@mit.edu>
> Date:   Sat Mar 28 21:39:54 2015 -0400
> 
>     e2fsck: use PROMPT_NONE for FUTURE_SB_LAST_*_FUDGED problems
>     
>     This allows us to print a message warning the user that there is
>     something funny going on with their hardware clock (probably time zone
>     issues caused by trying to be compatible with legacy OS's such as
>     Windows), without triggering a full file system check.
>     
>     Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  OK, so this will just print the error but won't fix it unless in preen
or full check mode. Am I right. That sounds fine to me.

								Honza

> 
> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index 12cf836..f442a33 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -386,14 +386,14 @@ static struct e2fsck_problem problem_table[] = {
>  	/* Last mount time is in the future (fudged) */
>  	{ PR_0_FUTURE_SB_LAST_MOUNT_FUDGED,
>  	  N_("@S last mount time is in the future.\n\t(by less than a day, "
> -	     "probably due to the hardware clock being incorrectly set)  "),
> -	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
> +	     "probably due to the hardware clock being incorrectly set)\n"),
> +	  PROMPT_NONE, PR_PREEN_OK | PR_NO_OK },
>  
>  	/* Last write time is in the future (fudged) */
>  	{ PR_0_FUTURE_SB_LAST_WRITE_FUDGED,
>  	  N_("@S last write time is in the future.\n\t(by less than a day, "
> -	     "probably due to the hardware clock being incorrectly set).  "),
> -	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
> +	     "probably due to the hardware clock being incorrectly set)\n"),
> +	  PROMPT_NONE, PR_PREEN_OK | PR_NO_OK },
>  
>  	/* Block group checksum (latch question) is invalid. */
>  	{ PR_0_GDT_CSUM_LATCH,
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time
  2015-04-01 13:06   ` Jan Kara
@ 2015-04-01 15:08     ` Theodore Ts'o
  2015-04-01 18:08       ` Eric Sandeen
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2015-04-01 15:08 UTC (permalink / raw)
  To: Jan Kara; +Cc: Eric Sandeen, ext4 development

On Wed, Apr 01, 2015 at 03:06:42PM +0200, Jan Kara wrote:
> >     e2fsck: use PROMPT_NONE for FUTURE_SB_LAST_*_FUDGED problems
> >     
> >     This allows us to print a message warning the user that there is
> >     something funny going on with their hardware clock (probably time zone
> >     issues caused by trying to be compatible with legacy OS's such as
> >     Windows), without triggering a full file system check.
> >     
> >     Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>   OK, so this will just print the error but won't fix it unless in preen
> or full check mode. Am I right. That sounds fine to me.

Actually, it will fix it unless the run in -n mode, but it won't count
it as a "fix".  Hence, it won't trigger the full file system check.

      		       	  		    - Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time
  2015-04-01 15:08     ` Theodore Ts'o
@ 2015-04-01 18:08       ` Eric Sandeen
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2015-04-01 18:08 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Jan Kara, Eric Sandeen, ext4 development


> On Apr 1, 2015, at 11:08 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> On Wed, Apr 01, 2015 at 03:06:42PM +0200, Jan Kara wrote:
>>>    e2fsck: use PROMPT_NONE for FUTURE_SB_LAST_*_FUDGED problems
>>> 
>>>    This allows us to print a message warning the user that there is
>>>    something funny going on with their hardware clock (probably time zone
>>>    issues caused by trying to be compatible with legacy OS's such as
>>>    Windows), without triggering a full file system check.
>>> 
>>>    Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>>  OK, so this will just print the error but won't fix it unless in preen
>> or full check mode. Am I right. That sounds fine to me.
> 
> Actually, it will fix it unless the run in -n mode, but it won't count
> it as a "fix".  Hence, it won't trigger the full file system check.
> 
>                                       - Ted

Avoiding the full check is my only real concern so that's fine.  Docs might need updates.

-Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-01 18:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18 20:15 [PATCH] e2fsck: completely ignore last-mount and last-write within fudge_time Eric Sandeen
2015-03-18 20:40 ` Jan Kara
2015-03-29  2:32 ` Theodore Ts'o
2015-04-01 13:06   ` Jan Kara
2015-04-01 15:08     ` Theodore Ts'o
2015-04-01 18:08       ` Eric Sandeen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.