Hello Andreas, This issue was discussed on weekly ext4 developer concall (can not remain the date, months ago). If I understood Theodore right way , moving this code out of exported from ext2fs library was the main requirements for this new feature to be accepted. Theodore, I am right? Thanks. Artem Blagodarenko. > On 4 Jun 2018, at 20:58, Andreas Dilger <adilger@xxxxxxxxx> wrote: > > On May 26, 2018, at 4:46 PM, Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx> wrote: >> >> e2image and e2fsck automatically try to open some backup superblocks, >> if only blocksize is set or passed superblock can't be opened. >> Try few backup superblocks (e.g. {1, 3, 5, 7, 9} * blocksize * 8). >> >> This code is moved to lib/support/. > > I don't recall seeing an explanation of why this is in lib/support/ > instead of lib/ext2fs? Maybe as ext2fs_open_backups(), or is there > some reason we don't want to export this function? > >> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx> >> --- >> >> diff --git a/e2fsck/message.c b/e2fsck/message.c >> index 727f71d5..415d7609 100644 >> --- a/e2fsck/message.c >> +++ b/e2fsck/message.c >> @@ -465,7 +465,8 @@ static _INLINE_ void expand_percent_expression(FILE *f, ext2_filsys fs, >> fprintf(f, "%*lld", width, (long long) ctx->blkcount); >> break; >> case 'S': >> - fprintf(f, "%llu", get_backup_sb(NULL, fs, NULL, NULL)); >> + fprintf(f, "%llu", get_first_backup_sb(NULL, NULL, fs, >> + NULL, NULL)); > > (style) align after '(' > >> diff --git a/e2fsck/unix.c b/e2fsck/unix.c >> index 491e9eb6..1a5e556f 100644 >> --- a/e2fsck/unix.c >> +++ b/e2fsck/unix.c >> @@ -1479,11 +1479,19 @@ restart: >> retval ? _("Superblock invalid,") : >> _("Group descriptors look bad...")); >> orig_superblock = ctx->superblock; >> - get_backup_sb(ctx, fs, ctx->filesystem_name, io_ptr); >> - if (fs) >> - ext2fs_close_free(&fs); >> orig_retval = retval; >> - retval = try_open_fs(ctx, flags, io_ptr, &fs); >> + retval = try_backups(ctx->filesystem_name, >> + ctx->io_options, >> + flags, >> + &ctx->superblock, >> + &ctx->blocksize, io_ptr, >> + &fs); > > (style) align after '(', pack onto fewest lines possible > >> diff --git a/lib/support/sb_backup.c b/lib/support/sb_backup.c >> new file mode 100644 >> index 00000000..5660e237 >> --- /dev/null >> +++ b/lib/support/sb_backup.c > > (style) remove extra blank lines > >> +blk64_t get_first_backup_sb(blk64_t *superblock, unsigned int *block_size, >> + ext2_filsys fs, const char *name, >> + io_manager manager) > > (style) align after '(' > >> +{ >> + struct ext2_super_block *sb; >> + io_channel io = NULL; >> + void *buf = NULL; >> + int try_blocksize; >> + blk64_t try_superblock, ret_sb = 8193; >> + >> + /* superblock and block_size can be NULL if fs->super is passed */ >> + if (fs && fs->super) { >> + ret_sb = (fs->super->s_blocks_per_group + >> + fs->super->s_first_data_block); > > (style) no need for parenthesis here > >> + if (superblock) >> + *superblock = ret_sb; >> + if(block_size) > > (style) space after "if" > >> +} >> + >> + > > (style) remove extra blank line > >> +errcode_t try_backups(const char *name, const char *io_options, >> + int flags, blk64_t *superblock, >> + unsigned int *block_size, io_manager manager, >> + ext2_filsys *ret_fs) > > (style) align after '(' > >> +{ >> + errcode_t retval; >> + blk64_t try_block_number; >> + unsigned int i; >> + >> + /* >> + * Get first superblock location based on heuristic. >> + * Blocksize is also returned and used to find next >> + * superblock copy location. >> + */ >> + try_block_number = get_first_backup_sb(superblock, block_size, >> + *ret_fs, name, manager); > > (style) align after '(' > >> + >> +} >> + >> + > > (style) remove blank lines at end > >> diff --git a/lib/support/sb_backup.h b/lib/support/sb_backup.h >> new file mode 100644 >> index 00000000..f1d48bff >> --- /dev/null >> +++ b/lib/support/sb_backup.h >> @@ -0,0 +1,20 @@ >> +blk64_t get_first_backup_sb(blk64_t *superblock, unsigned int *block_size, >> + ext2_filsys fs, const char *name, >> + io_manager manager); >> + >> +errcode_t try_backups(const char *name, const char *io_options, >> + int flags, blk64_t *superblock, >> + unsigned int *block_size, io_manager manager, >> + ext2_filsys *ret_fs); > > (style) align after '( > >> diff --git a/misc/e2image.c b/misc/e2image.c >> index 727e3876..d8569068 100644 >> --- a/misc/e2image.c >> +++ b/misc/e2image.c >> @@ -1612,6 +1612,13 @@ int main (int argc, char ** argv) >> sprintf(offset_opt, "offset=%llu", source_offset); >> retval = ext2fs_open2(device_name, offset_opt, open_flag, >> superblock, blocksize, unix_io_manager, &fs); >> + if (retval & (superblock | blocksize)) { >> + printf(_("Try backups in other location.\n")); >> + retval = try_backups(device_name, offset_opt, open_flag, >> + &superblock, &blocksize, >> + unix_io_manager, &fs); > > (style) align after '(' > > > Cheers, Andreas > > > > >