[PATCH 0/4] e2image -b option to pass superblock number

[PATCH 0/4] e2image -b option to pass superblock number

[Artem Blagodarenko]

There was a discussion about possibility of using
superblock backup for creating images with e2image.

https://www.spinics.net/lists/linux-ext4/msg30583.html

Now our customer faced with situation then it would be
useful to have image file from partition with broken
superblock.

Here is set of patches that adds this functionality.

Artem Blagodarenko (4):
  ext2fs: opening filesystem code refactoring
  e2image: add -b option to use supperblock backup
  tests: add test for e2image -b option
  man: add -b option to e2image

 e2fsck/unix.c               |   28 +++-------------------------
 lib/ext2fs/ext2fs.h         |    4 ++++
 lib/ext2fs/openfs.c         |   32 ++++++++++++++++++++++++++++++++
 misc/dumpe2fs.c             |   17 +++--------------
 misc/e2image.8.in           |   15 +++++++++++++++
 misc/e2image.c              |   38 ++++++++++++++++++++++++++++++++------
 tests/i_zero_super/expect   |   22 ++++++++++++++++++++++
 tests/i_zero_super/image.gz |  Bin 0 -> 13262 bytes
 tests/i_zero_super/script   |   36 ++++++++++++++++++++++++++++++++++++
 9 files changed, 147 insertions(+), 45 deletions(-)
 create mode 100644 tests/i_zero_super/expect
 create mode 100644 tests/i_zero_super/image.gz
 create mode 100644 tests/i_zero_super/script

[PATCH 1/4] ext2fs: opening filesystem code refactoring

[Artem Blagodarenko]

From: Artem Blagodarenko <artem.blagodarenko@seagate.com>

There are similar opening filesystem code in different utilities.

The patch moves this code to ext2fs_try_open_fs().
This function make one of the action based on parameters:
1) open filesystem with given superblock, superblock size
2) open filesystem with given superblock, but try to
find right block size
3) open filesystem with default superblock and block size

Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>
---
 e2fsck/unix.c       |   28 +++-------------------------
 lib/ext2fs/ext2fs.h |    4 ++++
 lib/ext2fs/openfs.c |   32 ++++++++++++++++++++++++++++++++
 misc/dumpe2fs.c     |   17 +++--------------
 4 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index ff96148..76f984b 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1116,31 +1116,9 @@ static errcode_t try_open_fs(e2fsck_t ctx, int flags, io_manager io_ptr,
 			     ext2_filsys *ret_fs)
 {
 	errcode_t retval;
-
-	*ret_fs = NULL;
-	if (ctx->superblock && ctx->blocksize) {
-		retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options,
-				      flags, ctx->superblock, ctx->blocksize,
-				      io_ptr, ret_fs);
-	} else if (ctx->superblock) {
-		int blocksize;
-		for (blocksize = EXT2_MIN_BLOCK_SIZE;
-		     blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) {
-			if (*ret_fs) {
-				ext2fs_free(*ret_fs);
-				*ret_fs = NULL;
-			}
-			retval = ext2fs_open2(ctx->filesystem_name,
-					      ctx->io_options, flags,
-					      ctx->superblock, blocksize,
-					      io_ptr, ret_fs);
-			if (!retval)
-				break;
-		}
-	} else
-		retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options,
-				      flags, 0, 0, io_ptr, ret_fs);
-
+	retval = ext2fs_try_open_fs(ctx->filesystem_name, ctx->io_options,
+			   ctx->superblock, ctx->blocksize, flags,
+			   io_ptr, ret_fs);
 	if (retval == 0) {
 		(*ret_fs)->priv_data = ctx;
 		e2fsck_set_bitmap_type(*ret_fs, EXT2FS_BMAP64_RBTREE,
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index c18ea5f..1163ab6 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1577,6 +1577,10 @@ extern errcode_t ext2fs_open2(const char *name, const char *io_options,
 			      int flags, int superblock,
 			      unsigned int block_size, io_manager manager,
 			      ext2_filsys *ret_fs);
+extern errcode_t ext2fs_try_open_fs(char *filesystem_name, char *io_options,
+			     blk64_t superblock, int blocksize, int flags,
+			     io_manager io_ptr, ext2_filsys *ret_fs);
+
 /*
  * The dgrp_t argument to these two functions is not actually a group number
  * but a block number offset within a group table!  Convert with the formula
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 93b02ed..a236b40 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -487,6 +487,38 @@ cleanup:
 	return retval;
 }
 
+errcode_t ext2fs_try_open_fs(char *filesystem_name, char *io_options,
+			     blk64_t superblock, int blocksize, int flags,
+			     io_manager io_ptr, ext2_filsys *ret_fs)
+{
+	errcode_t retval;
+
+	*ret_fs = NULL;
+	if (superblock && blocksize) {
+		retval = ext2fs_open2(filesystem_name, io_options,
+				      flags, superblock, blocksize,
+				      io_ptr, ret_fs);
+	} else if (superblock) {
+		int blocksize;
+		for (blocksize = EXT2_MIN_BLOCK_SIZE;
+		     blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) {
+			if (*ret_fs) {
+				ext2fs_free(*ret_fs);
+				*ret_fs = NULL;
+			}
+			retval = ext2fs_open2(filesystem_name,
+					      io_options, flags,
+					      superblock, blocksize,
+					      io_ptr, ret_fs);
+			if (!retval)
+				break;
+		}
+	} else
+		retval = ext2fs_open2(filesystem_name, io_options,
+				      flags, 0, 0, io_ptr, ret_fs);
+	return retval;
+}
+
 /*
  * Set/get the filesystem data I/O channel.
  *
diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
index 395ea9e..0f55cfb 100644
--- a/misc/dumpe2fs.c
+++ b/misc/dumpe2fs.c
@@ -568,20 +568,9 @@ int main (int argc, char ** argv)
 	if (image_dump)
 		flags |= EXT2_FLAG_IMAGE_FILE;
 try_open_again:
-	if (use_superblock && !use_blocksize) {
-		for (use_blocksize = EXT2_MIN_BLOCK_SIZE;
-		     use_blocksize <= EXT2_MAX_BLOCK_SIZE;
-		     use_blocksize *= 2) {
-			retval = ext2fs_open (device_name, flags,
-					      use_superblock,
-					      use_blocksize, unix_io_manager,
-					      &fs);
-			if (!retval)
-				break;
-		}
-	} else
-		retval = ext2fs_open (device_name, flags, use_superblock,
-				      use_blocksize, unix_io_manager, &fs);
+	retval = ext2fs_try_open_fs(device_name, 0,
+				    use_superblock, use_blocksize, flags,
+				    unix_io_manager, &fs);
 	if (retval && !(flags & EXT2_FLAG_IGNORE_CSUM_ERRORS)) {
 		flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
 		goto try_open_again;
-- 
1.7.1

Re: [PATCH 1/4] ext2fs: opening filesystem code refactoring

[Andreas Dilger]

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

On Jun 26, 2017, at 10:58 AM, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote:
> 
> From: Artem Blagodarenko <artem.blagodarenko@seagate.com>
> 
> There are similar opening filesystem code in different utilities.
> 
> The patch moves this code to ext2fs_try_open_fs().
> This function make one of the action based on parameters:
> 1) open filesystem with given superblock, superblock size
> 2) open filesystem with given superblock, but try to
> find right block size
> 3) open filesystem with default superblock and block size

I'm definitely in favour of this, but wonder if this should just become part
of ext2fs_open2(), so that other tools also get the benefit of this change?
That could be done only if the superblock number and blocksize are zero.

Should this also try at least a few backup superblocks (e.g. {1, 3, 5, 7, 9} *
blocksize * 8)?

Also, e2fsck/util.c::get_backup_sb() does something very similar.  In "main()"
this is called just before calling try_open_fs(), which you've updated below.
If ext2fs_try_open() also tried backup superblocks then get_backup_sb() wouldn't
be needed at all.

> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>
> ---
> e2fsck/unix.c       |   28 +++-------------------------
> lib/ext2fs/ext2fs.h |    4 ++++
> lib/ext2fs/openfs.c |   32 ++++++++++++++++++++++++++++++++
> misc/dumpe2fs.c     |   17 +++--------------
> 4 files changed, 42 insertions(+), 39 deletions(-)
> 
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index ff96148..76f984b 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -1116,31 +1116,9 @@ static errcode_t try_open_fs(e2fsck_t ctx, int flags, io_manager io_ptr,
> 			     ext2_filsys *ret_fs)
> {
> 	errcode_t retval;
> -
> -	*ret_fs = NULL;
> -	if (ctx->superblock && ctx->blocksize) {
> -		retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options,
> -				      flags, ctx->superblock, ctx->blocksize,
> -				      io_ptr, ret_fs);
> -	} else if (ctx->superblock) {
> -		int blocksize;
> -		for (blocksize = EXT2_MIN_BLOCK_SIZE;
> -		     blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) {
> -			if (*ret_fs) {
> -				ext2fs_free(*ret_fs);
> -				*ret_fs = NULL;
> -			}
> -			retval = ext2fs_open2(ctx->filesystem_name,
> -					      ctx->io_options, flags,
> -					      ctx->superblock, blocksize,
> -					      io_ptr, ret_fs);
> -			if (!retval)
> -				break;
> -		}
> -	} else
> -		retval = ext2fs_open2(ctx->filesystem_name, ctx->io_options,
> -				      flags, 0, 0, io_ptr, ret_fs);
> -
> +	retval = ext2fs_try_open_fs(ctx->filesystem_name, ctx->io_options,
> +			   ctx->superblock, ctx->blocksize, flags,
> +			   io_ptr, ret_fs);
> 	if (retval == 0) {
> 		(*ret_fs)->priv_data = ctx;
> 		e2fsck_set_bitmap_type(*ret_fs, EXT2FS_BMAP64_RBTREE,
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index c18ea5f..1163ab6 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -1577,6 +1577,10 @@ extern errcode_t ext2fs_open2(const char *name, const char *io_options,
> 			      int flags, int superblock,
> 			      unsigned int block_size, io_manager manager,
> 			      ext2_filsys *ret_fs);
> +extern errcode_t ext2fs_try_open_fs(char *filesystem_name, char *io_options,
> +			     blk64_t superblock, int blocksize, int flags,
> +			     io_manager io_ptr, ext2_filsys *ret_fs);
> +
> /*
>  * The dgrp_t argument to these two functions is not actually a group number
>  * but a block number offset within a group table!  Convert with the formula
> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
> index 93b02ed..a236b40 100644
> --- a/lib/ext2fs/openfs.c
> +++ b/lib/ext2fs/openfs.c
> @@ -487,6 +487,38 @@ cleanup:
> 	return retval;
> }
> 
> +errcode_t ext2fs_try_open_fs(char *filesystem_name, char *io_options,
> +			     blk64_t superblock, int blocksize, int flags,
> +			     io_manager io_ptr, ext2_filsys *ret_fs)
> +{
> +	errcode_t retval;
> +
> +	*ret_fs = NULL;
> +	if (superblock && blocksize) {
> +		retval = ext2fs_open2(filesystem_name, io_options,
> +				      flags, superblock, blocksize,
> +				      io_ptr, ret_fs);
> +	} else if (superblock) {
> +		int blocksize;
> +		for (blocksize = EXT2_MIN_BLOCK_SIZE;
> +		     blocksize <= EXT2_MAX_BLOCK_SIZE; blocksize *= 2) {
> +			if (*ret_fs) {
> +				ext2fs_free(*ret_fs);
> +				*ret_fs = NULL;
> +			}
> +			retval = ext2fs_open2(filesystem_name,
> +					      io_options, flags,
> +					      superblock, blocksize,
> +					      io_ptr, ret_fs);
> +			if (!retval)
> +				break;
> +		}
> +	} else
> +		retval = ext2fs_open2(filesystem_name, io_options,
> +				      flags, 0, 0, io_ptr, ret_fs);
> +	return retval;
> +}
> +
> /*
>  * Set/get the filesystem data I/O channel.
>  *
> diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
> index 395ea9e..0f55cfb 100644
> --- a/misc/dumpe2fs.c
> +++ b/misc/dumpe2fs.c
> @@ -568,20 +568,9 @@ int main (int argc, char ** argv)
> 	if (image_dump)
> 		flags |= EXT2_FLAG_IMAGE_FILE;
> try_open_again:
> -	if (use_superblock && !use_blocksize) {
> -		for (use_blocksize = EXT2_MIN_BLOCK_SIZE;
> -		     use_blocksize <= EXT2_MAX_BLOCK_SIZE;
> -		     use_blocksize *= 2) {
> -			retval = ext2fs_open (device_name, flags,
> -					      use_superblock,
> -					      use_blocksize, unix_io_manager,
> -					      &fs);
> -			if (!retval)
> -				break;
> -		}
> -	} else
> -		retval = ext2fs_open (device_name, flags, use_superblock,
> -				      use_blocksize, unix_io_manager, &fs);
> +	retval = ext2fs_try_open_fs(device_name, 0,
> +				    use_superblock, use_blocksize, flags,
> +				    unix_io_manager, &fs);
> 	if (retval && !(flags & EXT2_FLAG_IGNORE_CSUM_ERRORS)) {
> 		flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
> 		goto try_open_again;
> --
> 1.7.1
> 


Cheers, Andreas






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

[PATCH 2/4] e2image: add -b option to use supperblock backup

[Artem Blagodarenko]

From: Artem Blagodarenko <artem.blagodarenko@seagate.com>

e2image has no ability to use superblock backup to copy metadata.
This feature can be useful if someone wants to make partition
image and fix it using e2fsck utility.

New -b option allows to pass superblock number, like e2fsck utility do.
e2image doesn't change primary superblock and store is as is, so
it can be fixed using e2fsck latter.

Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>
---
 misc/e2image.c |   38 ++++++++++++++++++++++++++++++++------
 1 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/misc/e2image.c b/misc/e2image.c
index e0c3188..acc27d1 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -103,7 +103,8 @@ static int get_bits_from_size(size_t size)
 
 static void usage(void)
 {
-	fprintf(stderr, _("Usage: %s [ -r|Q ] [ -fr ] device image-file\n"),
+	fprintf(stderr, _("Usage: %s [ -r|Q ] [ -b superblock ] "
+			  "[ -fr ] device image-file\n"),
 		program_name);
 	fprintf(stderr, _("       %s -I device image-file\n"), program_name);
 	fprintf(stderr, _("       %s -ra  [  -cfnp  ] [ -o src_offset ] "
@@ -1255,7 +1256,7 @@ static void output_qcow2_meta_data_blocks(ext2_filsys fs, int fd)
 	free_qcow2_image(img);
 }
 
-static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags)
+static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags, blk64_t superblock)
 {
 	struct process_block_struct	pb;
 	struct ext2_inode		inode;
@@ -1283,6 +1284,22 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags)
 		}
 	}
 
+	if (superblock) {
+		int j;
+
+		ext2fs_mark_block_bitmap2(meta_block_map, superblock);
+		meta_blocks_count++;
+
+		/*
+		 * Mark the backup superblock descriptors
+		 */
+		for (j = 0; j < fs->desc_blocks; j++) {
+			ext2fs_mark_block_bitmap2(meta_block_map,
+			ext2fs_descriptor_block_loc2(fs, superblock, j));
+		}
+		meta_blocks_count += fs->desc_blocks;
+	}
+
 	mark_table_blocks(fs);
 	if (show_progress)
 		fprintf(stderr, "%s", _("Scanning inodes...\n"));
@@ -1462,6 +1479,7 @@ int main (int argc, char ** argv)
 	int ignore_rw_mount = 0;
 	int check = 0;
 	struct stat st;
+	blk64_t superblock = 0;
 
 #ifdef ENABLE_NLS
 	setlocale(LC_MESSAGES, "");
@@ -1475,7 +1493,7 @@ int main (int argc, char ** argv)
 	if (argc && *argv)
 		program_name = *argv;
 	add_error_table(&et_ext2_error_table);
-	while ((c = getopt(argc, argv, "nrsIQafo:O:pc")) != EOF)
+	while ((c = getopt(argc, argv, "nrsIQafo:O:pcb:")) != EOF)
 		switch (c) {
 		case 'I':
 			flags |= E2IMAGE_INSTALL_FLAG;
@@ -1514,6 +1532,9 @@ int main (int argc, char ** argv)
 		case 'c':
 			check = 1;
 			break;
+		case 'b':
+			superblock = strtoull(optarg, NULL, 0);
+			break;
 		default:
 			usage();
 		}
@@ -1528,6 +1549,11 @@ int main (int argc, char ** argv)
 						 "with raw or QCOW2 images."));
 		exit(1);
 	}
+	if (superblock && !img_type) {
+		com_err(program_name, 0, "%s", _("-b option can only be used "
+						 "with raw or QCOW2 images."));
+		exit(1);
+	}
 	if ((source_offset || dest_offset) && img_type != E2IMAGE_RAW) {
 		com_err(program_name, 0, "%s",
 			_("Offsets are only allowed with raw images."));
@@ -1578,8 +1604,8 @@ int main (int argc, char ** argv)
 		}
 	}
 	sprintf(offset_opt, "offset=%llu", source_offset);
-	retval = ext2fs_open2(device_name, offset_opt, open_flag, 0, 0,
-			      unix_io_manager, &fs);
+	retval = ext2fs_try_open_fs(device_name, offset_opt, superblock, 0,
+				    open_flag, unix_io_manager, &fs);
         if (retval) {
 		com_err (program_name, retval, _("while trying to open %s"),
 			 device_name);
@@ -1664,7 +1690,7 @@ skip_device:
 		exit(1);
 	}
 	if (img_type)
-		write_raw_image_file(fs, fd, img_type, flags);
+		write_raw_image_file(fs, fd, img_type, flags, superblock);
 	else
 		write_image_file(fs, fd);
 
-- 
1.7.1

Re: [PATCH 2/4] e2image: add -b option to use supperblock backup

[Andreas Dilger]

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


> On Jun 26, 2017, at 10:58 AM, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote:
> 
> From: Artem Blagodarenko <artem.blagodarenko@seagate.com>
> 
> e2image has no ability to use superblock backup to copy metadata.
> This feature can be useful if someone wants to make partition
> image and fix it using e2fsck utility.
> 
> New -b option allows to pass superblock number, like e2fsck utility do.
> e2image doesn't change primary superblock and store is as is, so
> it can be fixed using e2fsck latter.
> 
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>

One comment/question below.

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

> ---
> misc/e2image.c |   38 ++++++++++++++++++++++++++++++++------
> 1 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/misc/e2image.c b/misc/e2image.c
> index e0c3188..acc27d1 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -103,7 +103,8 @@ static int get_bits_from_size(size_t size)
> 
> static void usage(void)
> {
> -	fprintf(stderr, _("Usage: %s [ -r|Q ] [ -fr ] device image-file\n"),
> +	fprintf(stderr, _("Usage: %s [ -r|Q ] [ -b superblock ] "
> +			  "[ -fr ] device image-file\n"),

Does this also need to accept the "-B blocksize" option?  That said, this is
definitely an improvement.

> 		program_name);
> 	fprintf(stderr, _("       %s -I device image-file\n"), program_name);
> 	fprintf(stderr, _("       %s -ra  [  -cfnp  ] [ -o src_offset ] "
> @@ -1255,7 +1256,7 @@ static void output_qcow2_meta_data_blocks(ext2_filsys fs, int fd)
> 	free_qcow2_image(img);
> }
> 
> -static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags)
> +static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags, blk64_t superblock)
> {
> 	struct process_block_struct	pb;
> 	struct ext2_inode		inode;
> @@ -1283,6 +1284,22 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags)
> 		}
> 	}
> 
> +	if (superblock) {
> +		int j;
> +
> +		ext2fs_mark_block_bitmap2(meta_block_map, superblock);
> +		meta_blocks_count++;
> +
> +		/*
> +		 * Mark the backup superblock descriptors
> +		 */
> +		for (j = 0; j < fs->desc_blocks; j++) {
> +			ext2fs_mark_block_bitmap2(meta_block_map,
> +			ext2fs_descriptor_block_loc2(fs, superblock, j));
> +		}
> +		meta_blocks_count += fs->desc_blocks;
> +	}
> +
> 	mark_table_blocks(fs);
> 	if (show_progress)
> 		fprintf(stderr, "%s", _("Scanning inodes...\n"));
> @@ -1462,6 +1479,7 @@ int main (int argc, char ** argv)
> 	int ignore_rw_mount = 0;
> 	int check = 0;
> 	struct stat st;
> +	blk64_t superblock = 0;
> 
> #ifdef ENABLE_NLS
> 	setlocale(LC_MESSAGES, "");
> @@ -1475,7 +1493,7 @@ int main (int argc, char ** argv)
> 	if (argc && *argv)
> 		program_name = *argv;
> 	add_error_table(&et_ext2_error_table);
> -	while ((c = getopt(argc, argv, "nrsIQafo:O:pc")) != EOF)
> +	while ((c = getopt(argc, argv, "nrsIQafo:O:pcb:")) != EOF)
> 		switch (c) {
> 		case 'I':
> 			flags |= E2IMAGE_INSTALL_FLAG;
> @@ -1514,6 +1532,9 @@ int main (int argc, char ** argv)
> 		case 'c':
> 			check = 1;
> 			break;
> +		case 'b':
> +			superblock = strtoull(optarg, NULL, 0);
> +			break;
> 		default:
> 			usage();
> 		}
> @@ -1528,6 +1549,11 @@ int main (int argc, char ** argv)
> 						 "with raw or QCOW2 images."));
> 		exit(1);
> 	}
> +	if (superblock && !img_type) {
> +		com_err(program_name, 0, "%s", _("-b option can only be used "
> +						 "with raw or QCOW2 images."));
> +		exit(1);
> +	}
> 	if ((source_offset || dest_offset) && img_type != E2IMAGE_RAW) {
> 		com_err(program_name, 0, "%s",
> 			_("Offsets are only allowed with raw images."));
> @@ -1578,8 +1604,8 @@ int main (int argc, char ** argv)
> 		}
> 	}
> 	sprintf(offset_opt, "offset=%llu", source_offset);
> -	retval = ext2fs_open2(device_name, offset_opt, open_flag, 0, 0,
> -			      unix_io_manager, &fs);
> +	retval = ext2fs_try_open_fs(device_name, offset_opt, superblock, 0,
> +				    open_flag, unix_io_manager, &fs);
>         if (retval) {
> 		com_err (program_name, retval, _("while trying to open %s"),
> 			 device_name);
> @@ -1664,7 +1690,7 @@ skip_device:
> 		exit(1);
> 	}
> 	if (img_type)
> -		write_raw_image_file(fs, fd, img_type, flags);
> +		write_raw_image_file(fs, fd, img_type, flags, superblock);
> 	else
> 		write_image_file(fs, fd);
> 
> --
> 1.7.1
> 


Cheers, Andreas






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

[PATCH 3/4] tests: add test for e2image -b option

[Artem Blagodarenko]

From: Artem Blagodarenko <artem.blagodarenko@seagate.com>

The test makes raw image from partition with broken super block
and executes e2fsck.

Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>
---
 tests/i_zero_super/expect   |   22 ++++++++++++++++++++++
 tests/i_zero_super/image.gz |  Bin 0 -> 13262 bytes
 tests/i_zero_super/script   |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/tests/i_zero_super/expect b/tests/i_zero_super/expect
new file mode 100644
index 0000000..863d692
--- /dev/null
+++ b/tests/i_zero_super/expect
@@ -0,0 +1,22 @@
+test.img was not cleanly unmounted, check forced.
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+Free blocks count wrong for group #0 (7987, counted=7982).
+Fix? no
+
+Free blocks count wrong (11602, counted=11597).
+Fix? no
+
+Free inodes count wrong for group #0 (1493, counted=1488).
+Fix? no
+
+Free inodes count wrong (2997, counted=2992).
+Fix? no
+
+
+test.img: ********** WARNING: Filesystem still has errors **********
+
+test.img: 11/3008 files (0.0% non-contiguous), 398/12000 blocks
diff --git a/tests/i_zero_super/image.gz b/tests/i_zero_super/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..eea9140194f4dd9ec9dc8bb7b9e2215b950587d2
GIT binary patch
literal 13262
zcmeI&{WsHl902f@Tih#ASFE<IL%5VllFHLfbsepcr#yxVS=!QEX3V<Xij3TH)g@2m
zDS0T_=4nglT7;E{xfz>0G_ld9Vc5R+bpL^V>zwZSynpzd^M0St`F!5z^~3w8H;SqT
zqYs|hy;?=Ydxb*`Y=(_Uj;z^->s>9@r^5EDn{U+Cn|rW>b}kfCoqXW-K~s(F;j74#
zhqt?LGco2DyY?LC#w8Uk#19^?CI2?lfagqx1)ifG8HH6hag-M>b#n;a0ZPl9H5uQ*
zh;IX^QFd*|QbjWJu&O+~X7%S}AV@xtYOvP)CS>1lrK);rafz~F03E!AP48O_FHJ-S
zg=;|Z6CYQNv-ZtIAsw=E5_ROA?i#k~veE+)ipFdv$#ixO-jpJlhBw`i@Ze2U$ugXh
zEMdbbDH0)^B6nzm#&w;gg^SRCFE9f3*C-V-jA$$~oJK~np*>kNW!_c@&xu+YHyNXN
zp%GA9=BovWD%_K}kqu4M8l_Vdo`ZrjD`Rn~ny_y)f?E3F0+*`oZ`-HKopl65$&V?2
z6t#B;?@_*MZBA_RXBf6G+|DYp-XhkO$YO6^T9V`5bn#K^+MJ~w%?yMSFJ^6Sg0UPo
zH18)>^zs;IYTrH~YA<7Mr$64hU|~C{vPc@*cM^i?V|<|Jn9+tquql)qba_d&&!IrM
zN~Zac^0PwabLqMD!9ml85Ia=TGE1p+3h@zX%O9$xRb*SWAT&w23469Mvy?^fd1|g3
z2OBk4fRetx=bz*1p{n()`A%z>On+9V^<KoFScBS4+?9aD>5jQ%qmv|KEd9g7%{cya
z65*HM6(OZ+6L>k!K_u1|@&p_klxX(aQxoxcuK2-5NAEJvycMqbFk(JtZeOq(eaVqK
zE$C<*4!NVbAnmvsr0$0sDX)ZlCMriySRgmmUjO^tlevd#3)Zt$LqUQ_eDSM@rLm)g
z!XAD=Sx#gZABPJ)Up{;GKD|C}Y8VkwAwFuhGbzSGG-0EPN6htOi;<%tBC<{2D7M<c
zHgZ1}&X(q@H%g}2b?~jUzVP!H8P+O>Hvi^SK!zjPB6^PPltypW6U@ls8<!hHlm+Zo
zhV@ue6ayjtKI3?=--?Te84ndnH!`e)6ZcO$@NXO<7^J3z)N8$XWIo%2ak^=WHQZ6I
z@Q$yP_0qj<cs{(29;$C_Nk@Z?7kA3j_q<0a%gT$(DtpQ;?ZCRc%)YpelMq?#?(f$b
z_UamTFN+X&iEQDYQcs_9B*mswP5Tz!>I$Qi8V+~~sMW&Avh1qk{jsdUms4q%_I~j9
zAP1wBH!x%p{*lPf%!OjFcUM;S-X3L~(Ph>1KAIhBU*6R=9GDzLN>@M4QqZpW;86in
zU6u2kydgWZ5!3%Mj+USKnvbny#tWU>-s*8m*m3AEb(yr<){ZABVq)XGI#2cz42~T&
zGiMA2h%d5h8S#Z$(EIzA$Y}eQPcMd3koG%AZ~YY0O4M39@<xLdrBh?hKWLtF{hBCP
zKSKVfKI(kvuk_4HnqI|OAvLr|+>qm%pxYt5T=-)((!SH_-N)wOT5EJu-k$_?OS4By
z`-cNru4=m6(i&s&<}6`1-*Pu6zm4e05s8Xx>Q+>QMg+NP$urt|PT#~`t~3AhF_nSY
zvBHEWF`>oD!xz}9ttUu?ntrBkM$M-%!q|)=)_Y)mo((>q^l>hC-8VW!K-)k72mk>f
z00e*l5C8%|00;m9AOHk_01yBIKmZ5;0U!VbfB+Bx0zd!=00AKIbp=K!;yw%P>JwUP
zN8Z$BYH3yUJBnz@L=EacMGN|;r>&TdDbac%hz*WE#2d70bXsDzshu_)m6AW|wjzkT
zcBy>bZol}OTTD6WMU0ljJ&icy#4Kr$Hr@9)dvDXkMN4wZxwi50$>mD+G8Nh><%Z8z
z-r_hsyIZ&rI57ThUe6tC!j|bV27G0o?8J#?hL{KkW#FoUb?d|b>Kwkeb>zx!KDuCK
v`J3J9`63;-00e*l5C8%|00;m9AOHk_01yBI{~LklvK5${_ikI*<wn@Q%S-lK

literal 0
HcmV?d00001

diff --git a/tests/i_zero_super/script b/tests/i_zero_super/script
new file mode 100644
index 0000000..5952c6f
--- /dev/null
+++ b/tests/i_zero_super/script
@@ -0,0 +1,36 @@
+if test -x $E2IMAGE_EXE; then
+
+IMAGE=$test_dir/image.gz
+FSCK_OPT=-fy
+OUT=$test_name.log
+EXP=$test_dir/expect
+
+cp /dev/null $OUT
+
+gzip -d < $IMAGE > $TMPFILE
+
+$E2IMAGE_EXE -r -b 8193 $TMPFILE $TMPFILE.raw
+$FSCK -n -b 8193 $TMPFILE.raw > $OUT.new 2>&1
+sed -f $cmd_dir/filter.sed -e "s;$TMPFILE.raw;test.img;" $OUT.new >> $OUT
+
+rm -f $TMPFILE
+rm -f $TMPFILE.raw
+rm -f $OUT.new
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
+unset IMAGE FSCK_OPT OUT EXP
+
+else #if test -x $E2IMAGE_EXE; then
+	echo "$test_name: $test_description: skipped"
+fi
-- 
1.7.1

Re: [PATCH 3/4] tests: add test for e2image -b option

[Andreas Dilger]

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

On Jun 26, 2017, at 10:58 AM, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote:
> 
> From: Artem Blagodarenko <artem.blagodarenko@seagate.com>
> 
> The test makes raw image from partition with broken super block
> and executes e2fsck.

It's great to have a test case for this.

If possible, it is preferred to create new test cases by writing a script that formats a new filesystem and induces corruption via debugfs or dd or similar.  That makes it easier to see what is actually corrupted in the filesystem.

Cheers, Andreas

> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>
> ---
> tests/i_zero_super/expect   |   22 ++++++++++++++++++++++
> tests/i_zero_super/image.gz |  Bin 0 -> 13262 bytes
> tests/i_zero_super/script   |   36 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 58 insertions(+), 0 deletions(-)
> 
> diff --git a/tests/i_zero_super/expect b/tests/i_zero_super/expect
> new file mode 100644
> index 0000000..863d692
> --- /dev/null
> +++ b/tests/i_zero_super/expect
> @@ -0,0 +1,22 @@
> +test.img was not cleanly unmounted, check forced.

Shouldn't this report something about using the backup superblock?

> +Pass 1: Checking inodes, blocks, and sizes
> +Pass 2: Checking directory structure
> +Pass 3: Checking directory connectivity
> +Pass 4: Checking reference counts
> +Pass 5: Checking group summary information
> +Free blocks count wrong for group #0 (7987, counted=7982).
> +Fix? no
> +
> +Free blocks count wrong (11602, counted=11597).
> +Fix? no
> +
> +Free inodes count wrong for group #0 (1493, counted=1488).
> +Fix? no
> +
> +Free inodes count wrong (2997, counted=2992).
> +Fix? no
> +
> +
> +test.img: ********** WARNING: Filesystem still has errors **********
> +
> +test.img: 11/3008 files (0.0% non-contiguous), 398/12000 blocks
> diff --git a/tests/i_zero_super/image.gz b/tests/i_zero_super/image.gz
> new file mode 100644
> index 0000000000000000000000000000000000000000..eea9140194f4dd9ec9dc8bb7b9e2215b950587d2
> GIT binary patch
> literal 13262
> zcmeI&{WsHl902f@Tih#ASFE<IL%5VllFHLfbsepcr#yxVS=!QEX3V<Xij3TH)g@2m
> zDS0T_=4nglT7;E{xfz>0G_ld9Vc5R+bpL^V>zwZSynpzd^M0St`F!5z^~3w8H;SqT
> zqYs|hy;?=Ydxb*`Y=(_Uj;z^->s>9@r^5EDn{U+Cn|rW>b}kfCoqXW-K~s(F;j74#
> zhqt?LGco2DyY?LC#w8Uk#19^?CI2?lfagqx1)ifG8HH6hag-M>b#n;a0ZPl9H5uQ*
> zh;IX^QFd*|QbjWJu&O+~X7%S}AV@xtYOvP)CS>1lrK);rafz~F03E!AP48O_FHJ-S
> zg=;|Z6CYQNv-ZtIAsw=E5_ROA?i#k~veE+)ipFdv$#ixO-jpJlhBw`i@Ze2U$ugXh
> zEMdbbDH0)^B6nzm#&w;gg^SRCFE9f3*C-V-jA$$~oJK~np*>kNW!_c@&xu+YHyNXN
> zp%GA9=BovWD%_K}kqu4M8l_Vdo`ZrjD`Rn~ny_y)f?E3F0+*`oZ`-HKopl65$&V?2
> z6t#B;?@_*MZBA_RXBf6G+|DYp-XhkO$YO6^T9V`5bn#K^+MJ~w%?yMSFJ^6Sg0UPo
> zH18)>^zs;IYTrH~YA<7Mr$64hU|~C{vPc@*cM^i?V|<|Jn9+tquql)qba_d&&!IrM
> zN~Zac^0PwabLqMD!9ml85Ia=TGE1p+3h@zX%O9$xRb*SWAT&w23469Mvy?^fd1|g3
> z2OBk4fRetx=bz*1p{n()`A%z>On+9V^<KoFScBS4+?9aD>5jQ%qmv|KEd9g7%{cya
> z65*HM6(OZ+6L>k!K_u1|@&p_klxX(aQxoxcuK2-5NAEJvycMqbFk(JtZeOq(eaVqK
> zE$C<*4!NVbAnmvsr0$0sDX)ZlCMriySRgmmUjO^tlevd#3)Zt$LqUQ_eDSM@rLm)g
> z!XAD=Sx#gZABPJ)Up{;GKD|C}Y8VkwAwFuhGbzSGG-0EPN6htOi;<%tBC<{2D7M<c
> zHgZ1}&X(q@H%g}2b?~jUzVP!H8P+O>Hvi^SK!zjPB6^PPltypW6U@ls8<!hHlm+Zo
> zhV@ue6ayjtKI3?=--?Te84ndnH!`e)6ZcO$@NXO<7^J3z)N8$XWIo%2ak^=WHQZ6I
> z@Q$yP_0qj<cs{(29;$C_Nk@Z?7kA3j_q<0a%gT$(DtpQ;?ZCRc%)YpelMq?#?(f$b
> z_UamTFN+X&iEQDYQcs_9B*mswP5Tz!>I$Qi8V+~~sMW&Avh1qk{jsdUms4q%_I~j9
> zAP1wBH!x%p{*lPf%!OjFcUM;S-X3L~(Ph>1KAIhBU*6R=9GDzLN>@M4QqZpW;86in
> zU6u2kydgWZ5!3%Mj+USKnvbny#tWU>-s*8m*m3AEb(yr<){ZABVq)XGI#2cz42~T&
> zGiMA2h%d5h8S#Z$(EIzA$Y}eQPcMd3koG%AZ~YY0O4M39@<xLdrBh?hKWLtF{hBCP
> zKSKVfKI(kvuk_4HnqI|OAvLr|+>qm%pxYt5T=-)((!SH_-N)wOT5EJu-k$_?OS4By
> z`-cNru4=m6(i&s&<}6`1-*Pu6zm4e05s8Xx>Q+>QMg+NP$urt|PT#~`t~3AhF_nSY
> zvBHEWF`>oD!xz}9ttUu?ntrBkM$M-%!q|)=)_Y)mo((>q^l>hC-8VW!K-)k72mk>f
> z00e*l5C8%|00;m9AOHk_01yBIKmZ5;0U!VbfB+Bx0zd!=00AKIbp=K!;yw%P>JwUP
> zN8Z$BYH3yUJBnz@L=EacMGN|;r>&TdDbac%hz*WE#2d70bXsDzshu_)m6AW|wjzkT
> zcBy>bZol}OTTD6WMU0ljJ&icy#4Kr$Hr@9)dvDXkMN4wZxwi50$>mD+G8Nh><%Z8z
> z-r_hsyIZ&rI57ThUe6tC!j|bV27G0o?8J#?hL{KkW#FoUb?d|b>Kwkeb>zx!KDuCK
> v`J3J9`63;-00e*l5C8%|00;m9AOHk_01yBI{~LklvK5${_ikI*<wn@Q%S-lK
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/i_zero_super/script b/tests/i_zero_super/script
> new file mode 100644
> index 0000000..5952c6f
> --- /dev/null
> +++ b/tests/i_zero_super/script
> @@ -0,0 +1,36 @@
> +if test -x $E2IMAGE_EXE; then
> +
> +IMAGE=$test_dir/image.gz
> +FSCK_OPT=-fy
> +OUT=$test_name.log
> +EXP=$test_dir/expect
> +
> +cp /dev/null $OUT

Truncating a file can be done directly by the shell, like: "> $OUT".

> +
> +gzip -d < $IMAGE > $TMPFILE
> +
> +$E2IMAGE_EXE -r -b 8193 $TMPFILE $TMPFILE.raw
> +$FSCK -n -b 8193 $TMPFILE.raw > $OUT.new 2>&1
> +sed -f $cmd_dir/filter.sed -e "s;$TMPFILE.raw;test.img;" $OUT.new >> $OUT

This could avoid a temp output file and filter the result directly, like:

$FSCK -n -b 8193 $TMPFILE.raw |
	sed -f $cmd_dir/filter.sed -e 's;$TMPFILE.raw;test.img;' > $OUT

> +
> +rm -f $TMPFILE
> +rm -f $TMPFILE.raw
> +rm -f $OUT.new

These can be a single "rm" command.

> +cmp -s $OUT $EXP
> +status=$?
> +
> +if [ "$status" = 0 ] ; then
> +	echo "$test_name: $test_description: ok"
> +	touch $test_name.ok
> +else
> +	echo "$test_name: $test_description: failed"
> +	diff $EXP $OUT > $test_name.failed
> +	rm -f $test_name.tmp
> +fi
> +
> +unset IMAGE FSCK_OPT OUT EXP
> +
> +else #if test -x $E2IMAGE_EXE; then

Not sure what the comment here is?  This should be the "if ! test -x" branch?

> +	echo "$test_name: $test_description: skipped"
> +fi
> --
> 1.7.1
> 


Cheers, Andreas






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

[PATCH 4/4] man: add -b option to e2image

[Artem Blagodarenko]

From: Artem Blagodarenko <artem.blagodarenko@seagate.com>

Add manual page for e2image -b option.

Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>
---
 misc/e2image.8.in |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/misc/e2image.8.in b/misc/e2image.8.in
index f28a76e..61c23da 100644
--- a/misc/e2image.8.in
+++ b/misc/e2image.8.in
@@ -11,6 +11,10 @@ e2image \- Save critical ext2/ext3/ext4 filesystem metadata to a file
 .B \-r|Q
 ]
 [
+.B \-b
+.I superblock
+]
+[
 .B \-fr
 ]
 .I device
@@ -167,6 +171,12 @@ the
 option will prevent analysis of problems related to hash-tree indexed
 directories.
 .PP
+Option
+.B \-b
+.I superblock
+can be used to get image from partition with broken primary superblock.
+The partition is copied as is including broken primary superblock. 
+.PP
 Note that this will work even if you substitute "/dev/hda1" for another raw
 disk image, or QCOW2 image previously created by
 .BR e2image .
@@ -217,6 +227,11 @@ This can be useful to write a qcow2 image containing all data to a
 sparse image file where it can be loop mounted, or to a disk partition.
 Note that this may not work with qcow2 images not generated by e2image.
 .PP
+Option
+.B \-b
+.I superblock
+can be used same way as for raw images.
+.PP
 .SH INCLUDING DATA
 Normally
 .B e2image
-- 
1.7.1

Re: [PATCH 4/4] man: add -b option to e2image

[Andreas Dilger]

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

On Jun 26, 2017, at 10:58 AM, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote:
> 
> From: Artem Blagodarenko <artem.blagodarenko@seagate.com>
> 
> Add manual page for e2image -b option.
> 
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@seagate.com>

This should probably be included with the 2/4 patch that adds the "-b" option.

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

> ---
> misc/e2image.8.in |   15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/misc/e2image.8.in b/misc/e2image.8.in
> index f28a76e..61c23da 100644
> --- a/misc/e2image.8.in
> +++ b/misc/e2image.8.in
> @@ -11,6 +11,10 @@ e2image \- Save critical ext2/ext3/ext4 filesystem metadata to a file
> .B \-r|Q
> ]
> [
> +.B \-b
> +.I superblock
> +]
> +[
> .B \-fr
> ]
> .I device
> @@ -167,6 +171,12 @@ the
> option will prevent analysis of problems related to hash-tree indexed
> directories.
> .PP
> +Option
> +.B \-b
> +.I superblock
> +can be used to get image from partition with broken primary superblock.
> +The partition is copied as is including broken primary superblock.
> +.PP
> Note that this will work even if you substitute "/dev/hda1" for another raw
> disk image, or QCOW2 image previously created by
> .BR e2image .
> @@ -217,6 +227,11 @@ This can be useful to write a qcow2 image containing all data to a
> sparse image file where it can be loop mounted, or to a disk partition.
> Note that this may not work with qcow2 images not generated by e2image.
> .PP
> +Option
> +.B \-b
> +.I superblock
> +can be used same way as for raw images.
> +.PP
> .SH INCLUDING DATA
> Normally
> .B e2image
> --
> 1.7.1
> 


Cheers, Andreas






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