| Message ID | b7c93630-9b74-994a-8a82-8ab827ca5a2d@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] e2fsck: Avoid changes on recovery flags when jbd2_journal_recover() failed | expand |
friendly ping.. On 2021/3/6 15:27, Haotian Li wrote: > jbd2_journal_recover() may fail when some error occers such > as ENOMEM and EIO. However, jsb->s_start is still cleared > by func e2fsck_journal_release(). This may break consistency > between metadata and data in disk. Sometimes, failure in > jbd2_journal_recover() is temporary but retry e2fsck will > skip the journal recovery when the temporary problem is fixed. > > Following harshad shirwadkar's suggestion，we add an option > "recovery_error_behavior" with default value "continue" to > e2fsck.conf. User may set it to "retry" or "exit" to adopt > different behavior when such journal recovery errors occur. > > Reported-by: Liangyun <liangyun2@huawei.com> > Signed-off-by: Haotian Li <lihaotian9@huawei.com> > Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> > --- > e2fsck/e2fsck.h | 11 +++++++++++ > e2fsck/journal.c | 33 +++++++++++++++++++++++++++++++-- > e2fsck/unix.c | 13 ++++++++++++- > 3 files changed, 54 insertions(+), 3 deletions(-) > > diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h > index 15d043ee..22f9ad11 100644 > --- a/e2fsck/e2fsck.h > +++ b/e2fsck/e2fsck.h > @@ -451,6 +451,9 @@ struct e2fsck_struct { > > /* Fast commit replay state */ > struct e2fsck_fc_replay_state fc_replay_state; > + > + /* Behavior when journal recovery fails */ > + int recovery_error_behavior; > }; > > /* Data structures to evaluate whether an extent tree needs rebuilding. */ > @@ -474,6 +477,14 @@ typedef struct region_struct *region_t; > extern int e2fsck_strnlen(const char * s, int count); > #endif > > +/* Different behaviors when journal recovery fails */ > +#define RECOVERY_ERROR_CONTINUE 0 > +#define RECOVERY_ERROR_RETRY 1 > +#define RECOVERY_ERROR_EXIT 2 > + > +/* Journal retry times if RECOVERY_ERROR_RETRY is set*/ > +#define RECOVERY_TIMES_LIMIT 3 > + > /* > * Procedure declarations > */ > diff --git a/e2fsck/journal.c b/e2fsck/journal.c > index a425bbd1..c1c6f6ee 100644 > --- a/e2fsck/journal.c > +++ b/e2fsck/journal.c > @@ -1600,11 +1600,26 @@ no_has_journal: > return retval; > } > > +static void set_recovery_error_behavior(e2fsck_t ctx, const char *recovery_behavior) > +{ > + if (!recovery_behavior) { > + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; > + return; > + } > + if (strcmp(recovery_behavior, "retry") == 0) > + ctx->recovery_error_behavior = RECOVERY_ERROR_RETRY; > + else if (strcmp(recovery_behavior, "exit") == 0) > + ctx->recovery_error_behavior = RECOVERY_ERROR_EXIT; > + else > + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; > +} > + > static errcode_t recover_ext3_journal(e2fsck_t ctx) > { > struct problem_context pctx; > journal_t *journal; > errcode_t retval; > + char *recovery_behavior = 0; > > clear_problem_context(&pctx); > > @@ -1629,8 +1644,12 @@ static errcode_t recover_ext3_journal(e2fsck_t ctx) > goto errout; > > retval = -jbd2_journal_recover(journal); > - if (retval) > + if (retval) { > + profile_get_string(ctx->profile, "options", "recovery_error_behavior", > + 0, "continue", &recovery_behavior); > + set_recovery_error_behavior(ctx, recovery_behavior); > goto errout; > + } > > if (journal->j_failed_commit) { > pctx.ino = journal->j_failed_commit; > @@ -1645,7 +1664,15 @@ errout: > jbd2_journal_destroy_revoke(journal); > jbd2_journal_destroy_revoke_record_cache(); > jbd2_journal_destroy_revoke_table_cache(); > - e2fsck_journal_release(ctx, journal, 1, 0); > + if (retval == 0 || ctx->recovery_error_behavior == RECOVERY_ERROR_CONTINUE) > + e2fsck_journal_release(ctx, journal, 1, 0); > + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_EXIT) { > + ctx->fs->flags &= ~EXT2_FLAG_VALID; > + com_err(ctx->program_name, 0, > + _("Journal recovery failed " > + "on %s\n"), ctx->device_name); > + fatal_error(ctx, 0); > + } > return retval; > } > > @@ -1697,6 +1724,8 @@ errcode_t e2fsck_run_ext3_journal(e2fsck_t ctx) > > /* Set the superblock flags */ > e2fsck_clear_recover(ctx, recover_retval != 0); > + if (recover_retval != 0 && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) > + ext2fs_set_feature_journal_needs_recovery(ctx->fs->super); > > /* > * Do one last sanity check, and propagate journal->s_errno to > diff --git a/e2fsck/unix.c b/e2fsck/unix.c > index c5f9e441..25978471 100644 > --- a/e2fsck/unix.c > +++ b/e2fsck/unix.c > @@ -1068,6 +1068,8 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx) > if (c) > ctx->options |= E2F_OPT_ICOUNT_FULLMAP; > > + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; > + > if (ctx->readahead_kb == ~0ULL) { > profile_get_integer(ctx->profile, "options", > "readahead_mem_pct", 0, -1, &c); > @@ -1776,6 +1778,7 @@ failure: > "doing a read-only filesystem check.\n")); > io_channel_flush(ctx->fs->io); > } else { > + int recovery_retry_times = 0; > if (ctx->flags & E2F_FLAG_RESTARTED) { > /* > * Whoops, we attempted to run the > @@ -1788,7 +1791,15 @@ failure: > "on %s\n"), ctx->device_name); > fatal_error(ctx, 0); > } > - retval = e2fsck_run_ext3_journal(ctx); > + while (recovery_retry_times++ < RECOVERY_TIMES_LIMIT) { > + retval = e2fsck_run_ext3_journal(ctx); > + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) { > + log_out(ctx, _("Try to recovery Journal " > + "again in %s\n"), > + ctx->device_name); > + } else > + break; > + } > if (retval == EFSBADCRC) { > log_out(ctx, _("Journal checksum error " > "found in %s\n"),
Thanks for the updated patch. Just have some a couple of minor nits but other than that this looks good. Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> On Thu, Mar 11, 2021 at 3:50 AM Zhiqiang Liu <liuzhiqiang26@huawei.com> wrote: > > friendly ping.. > > On 2021/3/6 15:27, Haotian Li wrote: > > jbd2_journal_recover() may fail when some error occers such > > as ENOMEM and EIO. However, jsb->s_start is still cleared > > by func e2fsck_journal_release(). This may break consistency > > between metadata and data in disk. Sometimes, failure in > > jbd2_journal_recover() is temporary but retry e2fsck will > > skip the journal recovery when the temporary problem is fixed. > > > > Following harshad shirwadkar's suggestion，we add an option > > "recovery_error_behavior" with default value "continue" to > > e2fsck.conf. User may set it to "retry" or "exit" to adopt > > different behavior when such journal recovery errors occur. > > > > Reported-by: Liangyun <liangyun2@huawei.com> > > Signed-off-by: Haotian Li <lihaotian9@huawei.com> > > Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> > > --- > > e2fsck/e2fsck.h | 11 +++++++++++ > > e2fsck/journal.c | 33 +++++++++++++++++++++++++++++++-- > > e2fsck/unix.c | 13 ++++++++++++- > > 3 files changed, 54 insertions(+), 3 deletions(-) > > > > diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h > > index 15d043ee..22f9ad11 100644 > > --- a/e2fsck/e2fsck.h > > +++ b/e2fsck/e2fsck.h > > @@ -451,6 +451,9 @@ struct e2fsck_struct { > > > > /* Fast commit replay state */ > > struct e2fsck_fc_replay_state fc_replay_state; > > + > > + /* Behavior when journal recovery fails */ > > + int recovery_error_behavior; > > }; > > > > /* Data structures to evaluate whether an extent tree needs rebuilding. */ > > @@ -474,6 +477,14 @@ typedef struct region_struct *region_t; > > extern int e2fsck_strnlen(const char * s, int count); > > #endif > > > > +/* Different behaviors when journal recovery fails */ > > +#define RECOVERY_ERROR_CONTINUE 0 > > +#define RECOVERY_ERROR_RETRY 1 > > +#define RECOVERY_ERROR_EXIT 2 > > + > > +/* Journal retry times if RECOVERY_ERROR_RETRY is set*/ > > +#define RECOVERY_TIMES_LIMIT 3 > > + > > /* > > * Procedure declarations > > */ > > diff --git a/e2fsck/journal.c b/e2fsck/journal.c > > index a425bbd1..c1c6f6ee 100644 > > --- a/e2fsck/journal.c > > +++ b/e2fsck/journal.c > > @@ -1600,11 +1600,26 @@ no_has_journal: > > return retval; > > } > > > > +static void set_recovery_error_behavior(e2fsck_t ctx, const char *recovery_behavior) > > +{ > > + if (!recovery_behavior) { > > + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; > > + return; > > + } > > + if (strcmp(recovery_behavior, "retry") == 0) > > + ctx->recovery_error_behavior = RECOVERY_ERROR_RETRY; > > + else if (strcmp(recovery_behavior, "exit") == 0) > > + ctx->recovery_error_behavior = RECOVERY_ERROR_EXIT; > > + else > > + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; > > +} > > + > > static errcode_t recover_ext3_journal(e2fsck_t ctx) > > { > > struct problem_context pctx; > > journal_t *journal; > > errcode_t retval; > > + char *recovery_behavior = 0; > > > > clear_problem_context(&pctx); > > > > @@ -1629,8 +1644,12 @@ static errcode_t recover_ext3_journal(e2fsck_t ctx) > > goto errout; > > > > retval = -jbd2_journal_recover(journal); > > - if (retval) > > + if (retval) { > > + profile_get_string(ctx->profile, "options", "recovery_error_behavior", > > + 0, "continue", &recovery_behavior); > > + set_recovery_error_behavior(ctx, recovery_behavior); > > goto errout; > > + } > > > > if (journal->j_failed_commit) { > > pctx.ino = journal->j_failed_commit; > > @@ -1645,7 +1664,15 @@ errout: > > jbd2_journal_destroy_revoke(journal); > > jbd2_journal_destroy_revoke_record_cache(); > > jbd2_journal_destroy_revoke_table_cache(); > > - e2fsck_journal_release(ctx, journal, 1, 0); > > + if (retval == 0 || ctx->recovery_error_behavior == RECOVERY_ERROR_CONTINUE) > > + e2fsck_journal_release(ctx, journal, 1, 0); > > + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_EXIT) { > > + ctx->fs->flags &= ~EXT2_FLAG_VALID; > > + com_err(ctx->program_name, 0, > > + _("Journal recovery failed " > > + "on %s\n"), ctx->device_name); > > + fatal_error(ctx, 0); > > + } > > return retval; > > } > > > > @@ -1697,6 +1724,8 @@ errcode_t e2fsck_run_ext3_journal(e2fsck_t ctx) > > > > /* Set the superblock flags */ > > e2fsck_clear_recover(ctx, recover_retval != 0); > > + if (recover_retval != 0 && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) > > + ext2fs_set_feature_journal_needs_recovery(ctx->fs->super); > > > > /* > > * Do one last sanity check, and propagate journal->s_errno to > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c > > index c5f9e441..25978471 100644 > > --- a/e2fsck/unix.c > > +++ b/e2fsck/unix.c > > @@ -1068,6 +1068,8 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx) > > if (c) > > ctx->options |= E2F_OPT_ICOUNT_FULLMAP; > > > > + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; > > + > > if (ctx->readahead_kb == ~0ULL) { > > profile_get_integer(ctx->profile, "options", > > "readahead_mem_pct", 0, -1, &c); > > @@ -1776,6 +1778,7 @@ failure: > > "doing a read-only filesystem check.\n")); > > io_channel_flush(ctx->fs->io); > > } else { > > + int recovery_retry_times = 0; > > if (ctx->flags & E2F_FLAG_RESTARTED) { > > /* > > * Whoops, we attempted to run the > > @@ -1788,7 +1791,15 @@ failure: > > "on %s\n"), ctx->device_name); > > fatal_error(ctx, 0); > > } > > - retval = e2fsck_run_ext3_journal(ctx); > > + while (recovery_retry_times++ < RECOVERY_TIMES_LIMIT) { > > + retval = e2fsck_run_ext3_journal(ctx); > > + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) { > > + log_out(ctx, _("Try to recovery Journal " > > + "again in %s\n"), (nit) I think there's no reason to break the string into 2 lines. This will make this string searchable. > > + ctx->device_name); > > + } else > > + break; (style) Since you have {} brackets for if condition, please add it for else too (or remove it for if condition too) Thanks, Harshad > > + } > > if (retval == EFSBADCRC) { > > log_out(ctx, _("Journal checksum error " > > "found in %s\n"), >
Thanks for your review. New v3 patch will be resent. 在 2021/3/16 0:53, harshad shirwadkar 写道: > Thanks for the updated patch. Just have some a couple of minor nits > but other than that this looks good. > > Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> > > On Thu, Mar 11, 2021 at 3:50 AM Zhiqiang Liu <liuzhiqiang26@huawei.com> wrote: >> >> friendly ping.. >> >> On 2021/3/6 15:27, Haotian Li wrote: >>> jbd2_journal_recover() may fail when some error occers such >>> as ENOMEM and EIO. However, jsb->s_start is still cleared >>> by func e2fsck_journal_release(). This may break consistency >>> between metadata and data in disk. Sometimes, failure in >>> jbd2_journal_recover() is temporary but retry e2fsck will >>> skip the journal recovery when the temporary problem is fixed. >>> >>> Following harshad shirwadkar's suggestion，we add an option >>> "recovery_error_behavior" with default value "continue" to >>> e2fsck.conf. User may set it to "retry" or "exit" to adopt >>> different behavior when such journal recovery errors occur. >>> >>> Reported-by: Liangyun <liangyun2@huawei.com> >>> Signed-off-by: Haotian Li <lihaotian9@huawei.com> >>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> >>> --- >>> e2fsck/e2fsck.h | 11 +++++++++++ >>> e2fsck/journal.c | 33 +++++++++++++++++++++++++++++++-- >>> e2fsck/unix.c | 13 ++++++++++++- >>> 3 files changed, 54 insertions(+), 3 deletions(-) >>> >>> diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h >>> index 15d043ee..22f9ad11 100644 >>> --- a/e2fsck/e2fsck.h >>> +++ b/e2fsck/e2fsck.h >>> @@ -451,6 +451,9 @@ struct e2fsck_struct { >>> >>> /* Fast commit replay state */ >>> struct e2fsck_fc_replay_state fc_replay_state; >>> + >>> + /* Behavior when journal recovery fails */ >>> + int recovery_error_behavior; >>> }; >>> >>> /* Data structures to evaluate whether an extent tree needs rebuilding. */ >>> @@ -474,6 +477,14 @@ typedef struct region_struct *region_t; >>> extern int e2fsck_strnlen(const char * s, int count); >>> #endif >>> >>> +/* Different behaviors when journal recovery fails */ >>> +#define RECOVERY_ERROR_CONTINUE 0 >>> +#define RECOVERY_ERROR_RETRY 1 >>> +#define RECOVERY_ERROR_EXIT 2 >>> + >>> +/* Journal retry times if RECOVERY_ERROR_RETRY is set*/ >>> +#define RECOVERY_TIMES_LIMIT 3 >>> + >>> /* >>> * Procedure declarations >>> */ >>> diff --git a/e2fsck/journal.c b/e2fsck/journal.c >>> index a425bbd1..c1c6f6ee 100644 >>> --- a/e2fsck/journal.c >>> +++ b/e2fsck/journal.c >>> @@ -1600,11 +1600,26 @@ no_has_journal: >>> return retval; >>> } >>> >>> +static void set_recovery_error_behavior(e2fsck_t ctx, const char *recovery_behavior) >>> +{ >>> + if (!recovery_behavior) { >>> + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; >>> + return; >>> + } >>> + if (strcmp(recovery_behavior, "retry") == 0) >>> + ctx->recovery_error_behavior = RECOVERY_ERROR_RETRY; >>> + else if (strcmp(recovery_behavior, "exit") == 0) >>> + ctx->recovery_error_behavior = RECOVERY_ERROR_EXIT; >>> + else >>> + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; >>> +} >>> + >>> static errcode_t recover_ext3_journal(e2fsck_t ctx) >>> { >>> struct problem_context pctx; >>> journal_t *journal; >>> errcode_t retval; >>> + char *recovery_behavior = 0; >>> >>> clear_problem_context(&pctx); >>> >>> @@ -1629,8 +1644,12 @@ static errcode_t recover_ext3_journal(e2fsck_t ctx) >>> goto errout; >>> >>> retval = -jbd2_journal_recover(journal); >>> - if (retval) >>> + if (retval) { >>> + profile_get_string(ctx->profile, "options", "recovery_error_behavior", >>> + 0, "continue", &recovery_behavior); >>> + set_recovery_error_behavior(ctx, recovery_behavior); >>> goto errout; >>> + } >>> >>> if (journal->j_failed_commit) { >>> pctx.ino = journal->j_failed_commit; >>> @@ -1645,7 +1664,15 @@ errout: >>> jbd2_journal_destroy_revoke(journal); >>> jbd2_journal_destroy_revoke_record_cache(); >>> jbd2_journal_destroy_revoke_table_cache(); >>> - e2fsck_journal_release(ctx, journal, 1, 0); >>> + if (retval == 0 || ctx->recovery_error_behavior == RECOVERY_ERROR_CONTINUE) >>> + e2fsck_journal_release(ctx, journal, 1, 0); >>> + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_EXIT) { >>> + ctx->fs->flags &= ~EXT2_FLAG_VALID; >>> + com_err(ctx->program_name, 0, >>> + _("Journal recovery failed " >>> + "on %s\n"), ctx->device_name); >>> + fatal_error(ctx, 0); >>> + } >>> return retval; >>> } >>> >>> @@ -1697,6 +1724,8 @@ errcode_t e2fsck_run_ext3_journal(e2fsck_t ctx) >>> >>> /* Set the superblock flags */ >>> e2fsck_clear_recover(ctx, recover_retval != 0); >>> + if (recover_retval != 0 && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) >>> + ext2fs_set_feature_journal_needs_recovery(ctx->fs->super); >>> >>> /* >>> * Do one last sanity check, and propagate journal->s_errno to >>> diff --git a/e2fsck/unix.c b/e2fsck/unix.c >>> index c5f9e441..25978471 100644 >>> --- a/e2fsck/unix.c >>> +++ b/e2fsck/unix.c >>> @@ -1068,6 +1068,8 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx) >>> if (c) >>> ctx->options |= E2F_OPT_ICOUNT_FULLMAP; >>> >>> + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; >>> + >>> if (ctx->readahead_kb == ~0ULL) { >>> profile_get_integer(ctx->profile, "options", >>> "readahead_mem_pct", 0, -1, &c); >>> @@ -1776,6 +1778,7 @@ failure: >>> "doing a read-only filesystem check.\n")); >>> io_channel_flush(ctx->fs->io); >>> } else { >>> + int recovery_retry_times = 0; >>> if (ctx->flags & E2F_FLAG_RESTARTED) { >>> /* >>> * Whoops, we attempted to run the >>> @@ -1788,7 +1791,15 @@ failure: >>> "on %s\n"), ctx->device_name); >>> fatal_error(ctx, 0); >>> } >>> - retval = e2fsck_run_ext3_journal(ctx); >>> + while (recovery_retry_times++ < RECOVERY_TIMES_LIMIT) { >>> + retval = e2fsck_run_ext3_journal(ctx); >>> + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) { >>> + log_out(ctx, _("Try to recovery Journal " >>> + "again in %s\n"), > (nit) I think there's no reason to break the string into 2 lines. This > will make this string searchable. >>> + ctx->device_name); >>> + } else >>> + break; > (style) Since you have {} brackets for if condition, please add it for > else too (or remove it for if condition too) > > Thanks, > Harshad >>> + } >>> if (retval == EFSBADCRC) { >>> log_out(ctx, _("Journal checksum error " >>> "found in %s\n"), >> > . >
diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h index 15d043ee..22f9ad11 100644 --- a/e2fsck/e2fsck.h +++ b/e2fsck/e2fsck.h @@ -451,6 +451,9 @@ struct e2fsck_struct { /* Fast commit replay state */ struct e2fsck_fc_replay_state fc_replay_state; + + /* Behavior when journal recovery fails */ + int recovery_error_behavior; }; /* Data structures to evaluate whether an extent tree needs rebuilding. */ @@ -474,6 +477,14 @@ typedef struct region_struct *region_t; extern int e2fsck_strnlen(const char * s, int count); #endif +/* Different behaviors when journal recovery fails */ +#define RECOVERY_ERROR_CONTINUE 0 +#define RECOVERY_ERROR_RETRY 1 +#define RECOVERY_ERROR_EXIT 2 + +/* Journal retry times if RECOVERY_ERROR_RETRY is set*/ +#define RECOVERY_TIMES_LIMIT 3 + /* * Procedure declarations */ diff --git a/e2fsck/journal.c b/e2fsck/journal.c index a425bbd1..c1c6f6ee 100644 --- a/e2fsck/journal.c +++ b/e2fsck/journal.c @@ -1600,11 +1600,26 @@ no_has_journal: return retval; } +static void set_recovery_error_behavior(e2fsck_t ctx, const char *recovery_behavior) +{ + if (!recovery_behavior) { + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; + return; + } + if (strcmp(recovery_behavior, "retry") == 0) + ctx->recovery_error_behavior = RECOVERY_ERROR_RETRY; + else if (strcmp(recovery_behavior, "exit") == 0) + ctx->recovery_error_behavior = RECOVERY_ERROR_EXIT; + else + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; +} + static errcode_t recover_ext3_journal(e2fsck_t ctx) { struct problem_context pctx; journal_t *journal; errcode_t retval; + char *recovery_behavior = 0; clear_problem_context(&pctx); @@ -1629,8 +1644,12 @@ static errcode_t recover_ext3_journal(e2fsck_t ctx) goto errout; retval = -jbd2_journal_recover(journal); - if (retval) + if (retval) { + profile_get_string(ctx->profile, "options", "recovery_error_behavior", + 0, "continue", &recovery_behavior); + set_recovery_error_behavior(ctx, recovery_behavior); goto errout; + } if (journal->j_failed_commit) { pctx.ino = journal->j_failed_commit; @@ -1645,7 +1664,15 @@ errout: jbd2_journal_destroy_revoke(journal); jbd2_journal_destroy_revoke_record_cache(); jbd2_journal_destroy_revoke_table_cache(); - e2fsck_journal_release(ctx, journal, 1, 0); + if (retval == 0 || ctx->recovery_error_behavior == RECOVERY_ERROR_CONTINUE) + e2fsck_journal_release(ctx, journal, 1, 0); + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_EXIT) { + ctx->fs->flags &= ~EXT2_FLAG_VALID; + com_err(ctx->program_name, 0, + _("Journal recovery failed " + "on %s\n"), ctx->device_name); + fatal_error(ctx, 0); + } return retval; } @@ -1697,6 +1724,8 @@ errcode_t e2fsck_run_ext3_journal(e2fsck_t ctx) /* Set the superblock flags */ e2fsck_clear_recover(ctx, recover_retval != 0); + if (recover_retval != 0 && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) + ext2fs_set_feature_journal_needs_recovery(ctx->fs->super); /* * Do one last sanity check, and propagate journal->s_errno to diff --git a/e2fsck/unix.c b/e2fsck/unix.c index c5f9e441..25978471 100644 --- a/e2fsck/unix.c +++ b/e2fsck/unix.c @@ -1068,6 +1068,8 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx) if (c) ctx->options |= E2F_OPT_ICOUNT_FULLMAP; + ctx->recovery_error_behavior = RECOVERY_ERROR_CONTINUE; + if (ctx->readahead_kb == ~0ULL) { profile_get_integer(ctx->profile, "options", "readahead_mem_pct", 0, -1, &c); @@ -1776,6 +1778,7 @@ failure: "doing a read-only filesystem check.\n")); io_channel_flush(ctx->fs->io); } else { + int recovery_retry_times = 0; if (ctx->flags & E2F_FLAG_RESTARTED) { /* * Whoops, we attempted to run the @@ -1788,7 +1791,15 @@ failure: "on %s\n"), ctx->device_name); fatal_error(ctx, 0); } - retval = e2fsck_run_ext3_journal(ctx); + while (recovery_retry_times++ < RECOVERY_TIMES_LIMIT) { + retval = e2fsck_run_ext3_journal(ctx); + if (retval && ctx->recovery_error_behavior == RECOVERY_ERROR_RETRY) { + log_out(ctx, _("Try to recovery Journal " + "again in %s\n"), + ctx->device_name); + } else + break; + } if (retval == EFSBADCRC) { log_out(ctx, _("Journal checksum error " "found in %s\n"),