* [PATCH v6] e2image: add option to ignore fs errors
@ 2021-10-09 14:23 Artem Blagodarenko
2021-10-11 17:03 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: Artem Blagodarenko @ 2021-10-09 14:23 UTC (permalink / raw)
To: linux-ext4; +Cc: adilger.kernel, Alexey Lyashkov, Artem Blagodarenko
From: Alexey Lyashkov <alexey.lyashkov@hpe.com>
Add extended "-E ignore_errors" option to be more tolerant
to fs errors while scanning inode extents.
Signed-off-by: Alexey Lyashkov <alexey.lyashkov@hpe.com>
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@hpe.com>
Cray-bug-id: LUS-1922
Change-Id: Ib79300656726839b1d3b7ee1dd0793c60679d296
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
Changes since preveious version:
- The option is called ignore_error now
- Fixed typos
lib/support/Makefile.in | 4 +++
lib/support/mvstring.c | 25 +++++++++++++++
lib/support/mvstring.h | 1 +
misc/e2image.8.in | 15 ++++++++-
misc/e2image.c | 53 +++++++++++++++++++++++++++++---
misc/e2initrd_helper.c | 16 +---------
tests/i_error_tolerance/expect.1 | 23 ++++++++++++++
tests/i_error_tolerance/expect.2 | 7 +++++
tests/i_error_tolerance/script | 47 ++++++++++++++++++++++++++++
9 files changed, 171 insertions(+), 20 deletions(-)
create mode 100644 lib/support/mvstring.c
create mode 100644 lib/support/mvstring.h
create mode 100644 tests/i_error_tolerance/expect.1
create mode 100644 tests/i_error_tolerance/expect.2
create mode 100644 tests/i_error_tolerance/script
diff --git a/lib/support/Makefile.in b/lib/support/Makefile.in
index f3c7981e..c29b0a71 100644
--- a/lib/support/Makefile.in
+++ b/lib/support/Makefile.in
@@ -14,6 +14,7 @@ MKDIR_P = @MKDIR_P@
all::
OBJS= cstring.o \
+ mvstring.o \
mkquota.o \
plausible.o \
profile.o \
@@ -27,6 +28,7 @@ OBJS= cstring.o \
SRCS= $(srcdir)/argv_parse.c \
$(srcdir)/cstring.c \
+ $(srcdir)/mvstring.c \
$(srcdir)/mkquota.c \
$(srcdir)/parse_qtype.c \
$(srcdir)/plausible.c \
@@ -106,6 +108,8 @@ argv_parse.o: $(srcdir)/argv_parse.c $(top_builddir)/lib/config.h \
$(top_builddir)/lib/dirpaths.h $(srcdir)/argv_parse.h
cstring.o: $(srcdir)/cstring.c $(top_builddir)/lib/config.h \
$(top_builddir)/lib/dirpaths.h $(srcdir)/cstring.h
+mvstring.o: $(srcdir)/mvstring.c $(top_builddir)/lib/config.h \
+ $(srcdir)/mvstring.h
mkquota.o: $(srcdir)/mkquota.c $(top_builddir)/lib/config.h \
$(top_builddir)/lib/dirpaths.h $(top_srcdir)/lib/ext2fs/ext2_fs.h \
$(top_builddir)/lib/ext2fs/ext2_types.h $(top_srcdir)/lib/ext2fs/ext2fs.h \
diff --git a/lib/support/mvstring.c b/lib/support/mvstring.c
new file mode 100644
index 00000000..1ed2fd67
--- /dev/null
+++ b/lib/support/mvstring.c
@@ -0,0 +1,25 @@
+#include "config.h"
+#ifdef HAVE_STDLIB_H
+#include <stdlib.h>
+#endif
+#include <ctype.h>
+#include <string.h>
+#include "mvstring.h"
+
+
+/*
+ * fstab parsing code
+ */
+char *string_copy(const char *s)
+{
+ char *ret;
+
+ if (!s)
+ return 0;
+ ret = malloc(strlen(s)+1);
+ if (ret)
+ strcpy(ret, s);
+ return ret;
+}
+
+
diff --git a/lib/support/mvstring.h b/lib/support/mvstring.h
new file mode 100644
index 00000000..94590d56
--- /dev/null
+++ b/lib/support/mvstring.h
@@ -0,0 +1 @@
+extern char *string_copy(const char *s);
diff --git a/misc/e2image.8.in b/misc/e2image.8.in
index 90ea0c27..dfe53bc7 100644
--- a/misc/e2image.8.in
+++ b/misc/e2image.8.in
@@ -50,7 +50,10 @@ and
by using the
.B \-i
option to those programs. This can assist an expert in recovering
-catastrophically corrupted file systems.
+catastrophically corrupted file systems. If you going to grab an
+image from a corrupted FS
+.B \-E ignore_errors
+option to ignore fs errors, allows to grab fs image from a corrupted fs.
.PP
It is a very good idea to create image files for all file systems on a
system and save the partition layout (which can be generated using the
@@ -137,6 +140,16 @@ useful if the file system is being cloned to a flash-based storage device
(where reads are very fast and where it is desirable to avoid unnecessary
writes to reduce write wear on the device).
.TP
+.BI \-E " extended_options"
+Set e2image extended options. Extended options are comma separated, and
+may take an argument using the equals ('=') sign. The following options
+are supported:
+.RS 1.2i
+.TP
+.BI ignore_error
+Grab an image from a corrupted FS and ignore fs errors.
+.RE
+.TP
.B \-f
Override the read-only requirement for the source file system when saving
the image file using the
diff --git a/misc/e2image.c b/misc/e2image.c
index 2c1f3db3..45b8c2d5 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -53,6 +53,7 @@ extern int optind;
#include "support/nls-enable.h"
#include "support/plausible.h"
#include "support/quotaio.h"
+#include "support/mvstring.h"
#include "../version.h"
#define QCOW_OFLAG_COPIED (1ULL << 63)
@@ -79,6 +80,7 @@ static char move_mode;
static char show_progress;
static char *check_buf;
static int skipped_blocks;
+static int ignore_errors = 0;
static blk64_t align_offset(blk64_t offset, unsigned int n)
{
@@ -106,7 +108,7 @@ static int get_bits_from_size(size_t size)
static void usage(void)
{
fprintf(stderr, _("Usage: %s [ -r|-Q ] [ -f ] [ -b superblock ] [ -B blocksize ] "
- "device image-file\n"),
+ "[-E extended-options] device image-file\n"),
program_name);
fprintf(stderr, _(" %s -I device image-file\n"), program_name);
fprintf(stderr, _(" %s -ra [ -cfnp ] [ -o src_offset ] "
@@ -1379,7 +1381,8 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags,
com_err(program_name, retval,
_("while iterating over inode %u"),
ino);
- exit(1);
+ if (ignore_errors == 0)
+ exit(1);
}
} else {
if ((inode.i_flags & EXT4_EXTENTS_FL) ||
@@ -1392,7 +1395,8 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags,
if (retval) {
com_err(program_name, retval,
_("while iterating over inode %u"), ino);
- exit(1);
+ if (ignore_errors == 0)
+ exit(1);
}
}
}
@@ -1486,6 +1490,40 @@ static struct ext2_qcow2_hdr *check_qcow2_image(int *fd, char *name)
return qcow2_read_header(*fd);
}
+static void parse_extended_opts(const char *opts)
+{
+ char *buf, *token, *next, *p;
+ int ea_ver;
+ int extended_usage = 0;
+ unsigned long long reada_kb;
+
+ buf = string_copy(opts);
+ for (token = buf; token && *token; token = next) {
+ p = strchr(token, ',');
+ next = 0;
+ if (p) {
+ *p = 0;
+ next = p+1;
+ }
+ if (strcmp(token, "ignore_errors") == 0) {
+ ignore_errors = 1;
+ continue;
+ } else {
+ fprintf(stderr, _("Unknown extended option: %s\n"),
+ token);
+ extended_usage++;
+ }
+ }
+ free(buf);
+
+ if (extended_usage) {
+ fputs(_("\nExtended options are separated by commas. "
+ "Valid extended options are:\n\n"), stderr);
+ fputs("\tignore_errors\n", stderr);
+ exit(1);
+ }
+}
+
int main (int argc, char ** argv)
{
int c;
@@ -1506,6 +1544,7 @@ int main (int argc, char ** argv)
struct stat st;
blk64_t superblock = 0;
int blocksize = 0;
+ char *extended_opts = 0;
#ifdef ENABLE_NLS
setlocale(LC_MESSAGES, "");
@@ -1519,7 +1558,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, "b:B:nrsIQafo:O:pc")) != EOF)
+ while ((c = getopt(argc, argv, "b:B:E:nrsIQafo:O:pc")) != EOF)
switch (c) {
case 'b':
superblock = strtoull(optarg, NULL, 0);
@@ -1527,6 +1566,9 @@ int main (int argc, char ** argv)
case 'B':
blocksize = strtoul(optarg, NULL, 0);
break;
+ case 'E':
+ extended_opts = optarg;
+ break;
case 'I':
flags |= E2IMAGE_INSTALL_FLAG;
break;
@@ -1609,6 +1651,9 @@ int main (int argc, char ** argv)
exit(1);
}
+ if (extended_opts)
+ parse_extended_opts(extended_opts);
+
if (img_type && !ignore_rw_mount &&
(mount_flags & EXT2_MF_MOUNTED) &&
!(mount_flags & EXT2_MF_READONLY)) {
diff --git a/misc/e2initrd_helper.c b/misc/e2initrd_helper.c
index 436aab8c..ab5991a4 100644
--- a/misc/e2initrd_helper.c
+++ b/misc/e2initrd_helper.c
@@ -36,6 +36,7 @@ extern char *optarg;
#include "ext2fs/ext2fs.h"
#include "blkid/blkid.h"
#include "support/nls-enable.h"
+#include "support/mvstring.h"
#include "../version.h"
@@ -151,21 +152,6 @@ static int mem_file_eof(struct mem_file *file)
return (file->ptr >= file->size);
}
-/*
- * fstab parsing code
- */
-static char *string_copy(const char *s)
-{
- char *ret;
-
- if (!s)
- return 0;
- ret = malloc(strlen(s)+1);
- if (ret)
- strcpy(ret, s);
- return ret;
-}
-
static char *skip_over_blank(char *cp)
{
while (*cp && isspace(*cp))
diff --git a/tests/i_error_tolerance/expect.1 b/tests/i_error_tolerance/expect.1
new file mode 100644
index 00000000..e8d64954
--- /dev/null
+++ b/tests/i_error_tolerance/expect.1
@@ -0,0 +1,23 @@
+Pass 1: Checking inodes, blocks, and sizes
+Inode 12 has illegal block(s). Clear? yes
+
+Illegal indirect block (1000000) in inode 12. CLEARED.
+Inode 12, i_blocks is 34, should be 24. Fix? yes
+
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+Block bitmap differences: -(31--34) -41
+Fix? yes
+
+Free blocks count wrong for group #0 (158, counted=163).
+Fix? yes
+
+Free blocks count wrong (158, counted=163).
+Fix? yes
+
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+test_filesys: 12/24 files (8.3% non-contiguous), 37/200 blocks
+Exit status is 1
diff --git a/tests/i_error_tolerance/expect.2 b/tests/i_error_tolerance/expect.2
new file mode 100644
index 00000000..d9fcc327
--- /dev/null
+++ b/tests/i_error_tolerance/expect.2
@@ -0,0 +1,7 @@
+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
+test_filesys: 12/24 files (8.3% non-contiguous), 37/200 blocks
+Exit status is 0
diff --git a/tests/i_error_tolerance/script b/tests/i_error_tolerance/script
new file mode 100644
index 00000000..315569c7
--- /dev/null
+++ b/tests/i_error_tolerance/script
@@ -0,0 +1,47 @@
+if ! test -x $E2IMAGE_EXE; then
+ echo "$test_name: $test_description: skipped (no e2image)"
+ return 0
+fi
+if ! test -x $DEBUGFS_EXE; then
+ echo "$test_name: $test_description: skipped (no debugfs)"
+ return 0
+fi
+
+SKIP_GUNZIP="true"
+
+TEST_DATA="$test_name.tmp"
+dd if=/dev/urandom of=$TEST_DATA bs=1k count=16 > /dev/null 2>&1
+
+dd if=/dev/zero of=$TMPFILE bs=1k count=200 > /dev/null 2>&1
+$MKE2FS -Ft ext4 -O ^extents $TMPFILE > /dev/null 2>&1
+$DEBUGFS -w $TMPFILE << EOF > /dev/null 2>&1
+write $TEST_DATA testfile
+set_inode_field testfile block[IND] 1000000
+q
+EOF
+
+$E2IMAGE -r $TMPFILE $TMPFILE.back
+
+if [ $? = 0 ] ; then
+ echo "Image expected to be broken"
+ echo "$test_name: $test_description: fail"
+ touch $test_name.failed
+ return 0
+fi
+
+$E2IMAGE -r -E ignore_errors $TMPFILE $TMPFILE.back
+
+if [ $? = 1 ] ; then
+ echo "Can not get image even with ignore_errors"
+ echo "$test_name: $test_description: fail"
+ touch $test_name.failed
+ return 0
+fi
+
+mv $TMPFILE.back $TMPFILE
+
+. $cmd_dir/run_e2fsck
+
+rm -f $TEST_DATA
+
+unset E2FSCK_TIME TEST_DATA
--
2.18.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v6] e2image: add option to ignore fs errors
2021-10-09 14:23 [PATCH v6] e2image: add option to ignore fs errors Artem Blagodarenko
@ 2021-10-11 17:03 ` Darrick J. Wong
2021-10-12 14:53 ` Artem Blagodarenko
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2021-10-11 17:03 UTC (permalink / raw)
To: Artem Blagodarenko
Cc: linux-ext4, adilger.kernel, Alexey Lyashkov, Artem Blagodarenko
On Sat, Oct 09, 2021 at 10:23:00AM -0400, Artem Blagodarenko wrote:
> From: Alexey Lyashkov <alexey.lyashkov@hpe.com>
>
> Add extended "-E ignore_errors" option to be more tolerant
> to fs errors while scanning inode extents.
>
> Signed-off-by: Alexey Lyashkov <alexey.lyashkov@hpe.com>
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@hpe.com>
> Cray-bug-id: LUS-1922
> Change-Id: Ib79300656726839b1d3b7ee1dd0793c60679d296
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> ---
>
> Changes since preveious version:
> - The option is called ignore_error now
> - Fixed typos
>
> lib/support/Makefile.in | 4 +++
> lib/support/mvstring.c | 25 +++++++++++++++
> lib/support/mvstring.h | 1 +
> misc/e2image.8.in | 15 ++++++++-
> misc/e2image.c | 53 +++++++++++++++++++++++++++++---
> misc/e2initrd_helper.c | 16 +---------
> tests/i_error_tolerance/expect.1 | 23 ++++++++++++++
> tests/i_error_tolerance/expect.2 | 7 +++++
> tests/i_error_tolerance/script | 47 ++++++++++++++++++++++++++++
> 9 files changed, 171 insertions(+), 20 deletions(-)
> create mode 100644 lib/support/mvstring.c
> create mode 100644 lib/support/mvstring.h
> create mode 100644 tests/i_error_tolerance/expect.1
> create mode 100644 tests/i_error_tolerance/expect.2
> create mode 100644 tests/i_error_tolerance/script
>
> diff --git a/lib/support/Makefile.in b/lib/support/Makefile.in
> index f3c7981e..c29b0a71 100644
> --- a/lib/support/Makefile.in
> +++ b/lib/support/Makefile.in
> @@ -14,6 +14,7 @@ MKDIR_P = @MKDIR_P@
> all::
>
> OBJS= cstring.o \
> + mvstring.o \
> mkquota.o \
> plausible.o \
> profile.o \
> @@ -27,6 +28,7 @@ OBJS= cstring.o \
>
> SRCS= $(srcdir)/argv_parse.c \
> $(srcdir)/cstring.c \
> + $(srcdir)/mvstring.c \
> $(srcdir)/mkquota.c \
> $(srcdir)/parse_qtype.c \
> $(srcdir)/plausible.c \
> @@ -106,6 +108,8 @@ argv_parse.o: $(srcdir)/argv_parse.c $(top_builddir)/lib/config.h \
> $(top_builddir)/lib/dirpaths.h $(srcdir)/argv_parse.h
> cstring.o: $(srcdir)/cstring.c $(top_builddir)/lib/config.h \
> $(top_builddir)/lib/dirpaths.h $(srcdir)/cstring.h
> +mvstring.o: $(srcdir)/mvstring.c $(top_builddir)/lib/config.h \
> + $(srcdir)/mvstring.h
> mkquota.o: $(srcdir)/mkquota.c $(top_builddir)/lib/config.h \
> $(top_builddir)/lib/dirpaths.h $(top_srcdir)/lib/ext2fs/ext2_fs.h \
> $(top_builddir)/lib/ext2fs/ext2_types.h $(top_srcdir)/lib/ext2fs/ext2fs.h \
> diff --git a/lib/support/mvstring.c b/lib/support/mvstring.c
> new file mode 100644
> index 00000000..1ed2fd67
> --- /dev/null
> +++ b/lib/support/mvstring.c
> @@ -0,0 +1,25 @@
> +#include "config.h"
> +#ifdef HAVE_STDLIB_H
> +#include <stdlib.h>
> +#endif
> +#include <ctype.h>
> +#include <string.h>
> +#include "mvstring.h"
> +
> +
> +/*
> + * fstab parsing code
> + */
> +char *string_copy(const char *s)
> +{
> + char *ret;
> +
> + if (!s)
> + return 0;
> + ret = malloc(strlen(s)+1);
> + if (ret)
> + strcpy(ret, s);
> + return ret;
Why is it necessary to reimplement strdup?
> +}
> +
> +
> diff --git a/lib/support/mvstring.h b/lib/support/mvstring.h
> new file mode 100644
> index 00000000..94590d56
> --- /dev/null
> +++ b/lib/support/mvstring.h
> @@ -0,0 +1 @@
> +extern char *string_copy(const char *s);
> diff --git a/misc/e2image.8.in b/misc/e2image.8.in
> index 90ea0c27..dfe53bc7 100644
> --- a/misc/e2image.8.in
> +++ b/misc/e2image.8.in
> @@ -50,7 +50,10 @@ and
> by using the
> .B \-i
> option to those programs. This can assist an expert in recovering
> -catastrophically corrupted file systems.
> +catastrophically corrupted file systems. If you going to grab an
> +image from a corrupted FS
> +.B \-E ignore_errors
> +option to ignore fs errors, allows to grab fs image from a corrupted fs.
Don't restate things in manual pages.
"If you know the filesystem is corrupt, see the -E ignore_errors option
below for information about how to tell e2image to deal with that."
> .PP
> It is a very good idea to create image files for all file systems on a
> system and save the partition layout (which can be generated using the
> @@ -137,6 +140,16 @@ useful if the file system is being cloned to a flash-based storage device
> (where reads are very fast and where it is desirable to avoid unnecessary
> writes to reduce write wear on the device).
> .TP
> +.BI \-E " extended_options"
> +Set e2image extended options. Extended options are comma separated, and
> +may take an argument using the equals ('=') sign. The following options
> +are supported:
> +.RS 1.2i
> +.TP
> +.BI ignore_error
But you said it was -E ignore_errors above. Which is it?
> +Grab an image from a corrupted FS and ignore fs errors.
> +.RE
> +.TP
> .B \-f
> Override the read-only requirement for the source file system when saving
> the image file using the
> diff --git a/misc/e2image.c b/misc/e2image.c
> index 2c1f3db3..45b8c2d5 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -53,6 +53,7 @@ extern int optind;
> #include "support/nls-enable.h"
> #include "support/plausible.h"
> #include "support/quotaio.h"
> +#include "support/mvstring.h"
> #include "../version.h"
>
> #define QCOW_OFLAG_COPIED (1ULL << 63)
> @@ -79,6 +80,7 @@ static char move_mode;
> static char show_progress;
> static char *check_buf;
> static int skipped_blocks;
> +static int ignore_errors = 0;
>
> static blk64_t align_offset(blk64_t offset, unsigned int n)
> {
> @@ -106,7 +108,7 @@ static int get_bits_from_size(size_t size)
> static void usage(void)
> {
> fprintf(stderr, _("Usage: %s [ -r|-Q ] [ -f ] [ -b superblock ] [ -B blocksize ] "
> - "device image-file\n"),
> + "[-E extended-options] device image-file\n"),
> program_name);
> fprintf(stderr, _(" %s -I device image-file\n"), program_name);
> fprintf(stderr, _(" %s -ra [ -cfnp ] [ -o src_offset ] "
> @@ -1379,7 +1381,8 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags,
> com_err(program_name, retval,
> _("while iterating over inode %u"),
> ino);
> - exit(1);
> + if (ignore_errors == 0)
> + exit(1);
> }
> } else {
> if ((inode.i_flags & EXT4_EXTENTS_FL) ||
> @@ -1392,7 +1395,8 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags,
> if (retval) {
> com_err(program_name, retval,
> _("while iterating over inode %u"), ino);
> - exit(1);
> + if (ignore_errors == 0)
> + exit(1);
> }
> }
> }
> @@ -1486,6 +1490,40 @@ static struct ext2_qcow2_hdr *check_qcow2_image(int *fd, char *name)
> return qcow2_read_header(*fd);
> }
>
> +static void parse_extended_opts(const char *opts)
> +{
> + char *buf, *token, *next, *p;
> + int ea_ver;
> + int extended_usage = 0;
> + unsigned long long reada_kb;
> +
> + buf = string_copy(opts);
> + for (token = buf; token && *token; token = next) {
> + p = strchr(token, ',');
> + next = 0;
> + if (p) {
> + *p = 0;
> + next = p+1;
> + }
> + if (strcmp(token, "ignore_errors") == 0) {
getsubopt() ?
> + ignore_errors = 1;
> + continue;
> + } else {
> + fprintf(stderr, _("Unknown extended option: %s\n"),
> + token);
> + extended_usage++;
> + }
> + }
> + free(buf);
> +
> + if (extended_usage) {
> + fputs(_("\nExtended options are separated by commas. "
> + "Valid extended options are:\n\n"), stderr);
> + fputs("\tignore_errors\n", stderr);
> + exit(1);
> + }
> +}
> +
> int main (int argc, char ** argv)
> {
> int c;
> @@ -1506,6 +1544,7 @@ int main (int argc, char ** argv)
> struct stat st;
> blk64_t superblock = 0;
> int blocksize = 0;
> + char *extended_opts = 0;
>
> #ifdef ENABLE_NLS
> setlocale(LC_MESSAGES, "");
> @@ -1519,7 +1558,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, "b:B:nrsIQafo:O:pc")) != EOF)
> + while ((c = getopt(argc, argv, "b:B:E:nrsIQafo:O:pc")) != EOF)
> switch (c) {
> case 'b':
> superblock = strtoull(optarg, NULL, 0);
> @@ -1527,6 +1566,9 @@ int main (int argc, char ** argv)
> case 'B':
> blocksize = strtoul(optarg, NULL, 0);
> break;
> + case 'E':
> + extended_opts = optarg;
> + break;
> case 'I':
> flags |= E2IMAGE_INSTALL_FLAG;
> break;
> @@ -1609,6 +1651,9 @@ int main (int argc, char ** argv)
> exit(1);
> }
>
> + if (extended_opts)
> + parse_extended_opts(extended_opts);
> +
> if (img_type && !ignore_rw_mount &&
> (mount_flags & EXT2_MF_MOUNTED) &&
> !(mount_flags & EXT2_MF_READONLY)) {
> diff --git a/misc/e2initrd_helper.c b/misc/e2initrd_helper.c
> index 436aab8c..ab5991a4 100644
> --- a/misc/e2initrd_helper.c
> +++ b/misc/e2initrd_helper.c
> @@ -36,6 +36,7 @@ extern char *optarg;
> #include "ext2fs/ext2fs.h"
> #include "blkid/blkid.h"
> #include "support/nls-enable.h"
> +#include "support/mvstring.h"
>
> #include "../version.h"
>
> @@ -151,21 +152,6 @@ static int mem_file_eof(struct mem_file *file)
> return (file->ptr >= file->size);
> }
>
> -/*
> - * fstab parsing code
> - */
> -static char *string_copy(const char *s)
> -{
> - char *ret;
> -
> - if (!s)
> - return 0;
> - ret = malloc(strlen(s)+1);
> - if (ret)
> - strcpy(ret, s);
> - return ret;
> -}
> -
> static char *skip_over_blank(char *cp)
> {
> while (*cp && isspace(*cp))
> diff --git a/tests/i_error_tolerance/expect.1 b/tests/i_error_tolerance/expect.1
> new file mode 100644
> index 00000000..e8d64954
> --- /dev/null
> +++ b/tests/i_error_tolerance/expect.1
> @@ -0,0 +1,23 @@
> +Pass 1: Checking inodes, blocks, and sizes
> +Inode 12 has illegal block(s). Clear? yes
> +
> +Illegal indirect block (1000000) in inode 12. CLEARED.
> +Inode 12, i_blocks is 34, should be 24. Fix? yes
> +
> +Pass 2: Checking directory structure
> +Pass 3: Checking directory connectivity
> +Pass 4: Checking reference counts
> +Pass 5: Checking group summary information
> +Block bitmap differences: -(31--34) -41
> +Fix? yes
> +
> +Free blocks count wrong for group #0 (158, counted=163).
> +Fix? yes
> +
> +Free blocks count wrong (158, counted=163).
> +Fix? yes
> +
> +
> +test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
> +test_filesys: 12/24 files (8.3% non-contiguous), 37/200 blocks
> +Exit status is 1
> diff --git a/tests/i_error_tolerance/expect.2 b/tests/i_error_tolerance/expect.2
> new file mode 100644
> index 00000000..d9fcc327
> --- /dev/null
> +++ b/tests/i_error_tolerance/expect.2
> @@ -0,0 +1,7 @@
> +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
> +test_filesys: 12/24 files (8.3% non-contiguous), 37/200 blocks
> +Exit status is 0
> diff --git a/tests/i_error_tolerance/script b/tests/i_error_tolerance/script
> new file mode 100644
> index 00000000..315569c7
> --- /dev/null
> +++ b/tests/i_error_tolerance/script
> @@ -0,0 +1,47 @@
> +if ! test -x $E2IMAGE_EXE; then
> + echo "$test_name: $test_description: skipped (no e2image)"
> + return 0
> +fi
> +if ! test -x $DEBUGFS_EXE; then
> + echo "$test_name: $test_description: skipped (no debugfs)"
> + return 0
> +fi
> +
> +SKIP_GUNZIP="true"
> +
> +TEST_DATA="$test_name.tmp"
> +dd if=/dev/urandom of=$TEST_DATA bs=1k count=16 > /dev/null 2>&1
> +
> +dd if=/dev/zero of=$TMPFILE bs=1k count=200 > /dev/null 2>&1
> +$MKE2FS -Ft ext4 -O ^extents $TMPFILE > /dev/null 2>&1
> +$DEBUGFS -w $TMPFILE << EOF > /dev/null 2>&1
> +write $TEST_DATA testfile
> +set_inode_field testfile block[IND] 1000000
> +q
> +EOF
> +
> +$E2IMAGE -r $TMPFILE $TMPFILE.back
> +
> +if [ $? = 0 ] ; then
> + echo "Image expected to be broken"
> + echo "$test_name: $test_description: fail"
> + touch $test_name.failed
> + return 0
> +fi
> +
> +$E2IMAGE -r -E ignore_errors $TMPFILE $TMPFILE.back
> +
> +if [ $? = 1 ] ; then
> + echo "Can not get image even with ignore_errors"
> + echo "$test_name: $test_description: fail"
> + touch $test_name.failed
> + return 0
> +fi
> +
> +mv $TMPFILE.back $TMPFILE
> +
> +. $cmd_dir/run_e2fsck
> +
> +rm -f $TEST_DATA
> +
> +unset E2FSCK_TIME TEST_DATA
> --
> 2.18.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v6] e2image: add option to ignore fs errors
2021-10-11 17:03 ` Darrick J. Wong
@ 2021-10-12 14:53 ` Artem Blagodarenko
0 siblings, 0 replies; 3+ messages in thread
From: Artem Blagodarenko @ 2021-10-12 14:53 UTC (permalink / raw)
To: Darrick J. Wong
Cc: linux-ext4, adilger.kernel, Alexey Lyashkov, Artem Blagodarenko
Hello Derrick,
Thank you for the inspection.
> Why not use "strdup()" for this? It isn't really a problem with
> this patch, since it was in e2initrd_helper.c previously and just
> moved into the helper library, but seems strange. The strdup()
> function has existed for a very long time already, so there should
> not be any compatibility issues, but Ted added a patch using this
> function only a year ago, so maybe I'm missing something? It dates
> back to:
>
> 2001-01-05 Use string_copy() instead of strdup() for portability's sake
>
> It would probably make sense to remove the duplicate copies that
> still exist in e2fsck/util.c and misc/fsck.c, and add a comment
> why it is better than strdup()?
I see a commit
commit 8820c79f75c37a3bc85cea7f56e7277025e157ef
Author: Theodore Ts'o <tytso@mit.edu>
Date: Sat Jan 6 04:20:03 2001 +0000
That says
"Use string_copy() instead of strdup() for
portability's sake.”
Probably something is changed already, but I prefer use string_copy(),
that was implemented already. I just made it public.
> On 11 Oct 2021, at 20:03, Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Sat, Oct 09, 2021 at 10:23:00AM -0400, Artem Blagodarenko wrote:
>> From: Alexey Lyashkov <alexey.lyashkov@hpe.com>
>>
>> Add extended "-E ignore_errors" option to be more tolerant
>> to fs errors while scanning inode extents.
>>
>> Signed-off-by: Alexey Lyashkov <alexey.lyashkov@hpe.com>
>> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@hpe.com>
>> Cray-bug-id: LUS-1922
>> Change-Id: Ib79300656726839b1d3b7ee1dd0793c60679d296
>> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
>> ---
>>
>> Changes since preveious version:
>> - The option is called ignore_error now
>> - Fixed typos
>>
>> lib/support/Makefile.in | 4 +++
>> lib/support/mvstring.c | 25 +++++++++++++++
>> lib/support/mvstring.h | 1 +
>> misc/e2image.8.in | 15 ++++++++-
>> misc/e2image.c | 53 +++++++++++++++++++++++++++++---
>> misc/e2initrd_helper.c | 16 +---------
>> tests/i_error_tolerance/expect.1 | 23 ++++++++++++++
>> tests/i_error_tolerance/expect.2 | 7 +++++
>> tests/i_error_tolerance/script | 47 ++++++++++++++++++++++++++++
>> 9 files changed, 171 insertions(+), 20 deletions(-)
>> create mode 100644 lib/support/mvstring.c
>> create mode 100644 lib/support/mvstring.h
>> create mode 100644 tests/i_error_tolerance/expect.1
>> create mode 100644 tests/i_error_tolerance/expect.2
>> create mode 100644 tests/i_error_tolerance/script
>>
>> diff --git a/lib/support/Makefile.in b/lib/support/Makefile.in
>> index f3c7981e..c29b0a71 100644
>> --- a/lib/support/Makefile.in
>> +++ b/lib/support/Makefile.in
>> @@ -14,6 +14,7 @@ MKDIR_P = @MKDIR_P@
>> all::
>>
>> OBJS= cstring.o \
>> + mvstring.o \
>> mkquota.o \
>> plausible.o \
>> profile.o \
>> @@ -27,6 +28,7 @@ OBJS= cstring.o \
>>
>> SRCS= $(srcdir)/argv_parse.c \
>> $(srcdir)/cstring.c \
>> + $(srcdir)/mvstring.c \
>> $(srcdir)/mkquota.c \
>> $(srcdir)/parse_qtype.c \
>> $(srcdir)/plausible.c \
>> @@ -106,6 +108,8 @@ argv_parse.o: $(srcdir)/argv_parse.c $(top_builddir)/lib/config.h \
>> $(top_builddir)/lib/dirpaths.h $(srcdir)/argv_parse.h
>> cstring.o: $(srcdir)/cstring.c $(top_builddir)/lib/config.h \
>> $(top_builddir)/lib/dirpaths.h $(srcdir)/cstring.h
>> +mvstring.o: $(srcdir)/mvstring.c $(top_builddir)/lib/config.h \
>> + $(srcdir)/mvstring.h
>> mkquota.o: $(srcdir)/mkquota.c $(top_builddir)/lib/config.h \
>> $(top_builddir)/lib/dirpaths.h $(top_srcdir)/lib/ext2fs/ext2_fs.h \
>> $(top_builddir)/lib/ext2fs/ext2_types.h $(top_srcdir)/lib/ext2fs/ext2fs.h \
>> diff --git a/lib/support/mvstring.c b/lib/support/mvstring.c
>> new file mode 100644
>> index 00000000..1ed2fd67
>> --- /dev/null
>> +++ b/lib/support/mvstring.c
>> @@ -0,0 +1,25 @@
>> +#include "config.h"
>> +#ifdef HAVE_STDLIB_H
>> +#include <stdlib.h>
>> +#endif
>> +#include <ctype.h>
>> +#include <string.h>
>> +#include "mvstring.h"
>> +
>> +
>> +/*
>> + * fstab parsing code
>> + */
>> +char *string_copy(const char *s)
>> +{
>> + char *ret;
>> +
>> + if (!s)
>> + return 0;
>> + ret = malloc(strlen(s)+1);
>> + if (ret)
>> + strcpy(ret, s);
>> + return ret;
>
> Why is it necessary to reimplement strdup?
Please, see my comment above.
>
>> +}
>> +
>> +
>> diff --git a/lib/support/mvstring.h b/lib/support/mvstring.h
>> new file mode 100644
>> index 00000000..94590d56
>> --- /dev/null
>> +++ b/lib/support/mvstring.h
>> @@ -0,0 +1 @@
>> +extern char *string_copy(const char *s);
>> diff --git a/misc/e2image.8.in b/misc/e2image.8.in
>> index 90ea0c27..dfe53bc7 100644
>> --- a/misc/e2image.8.in
>> +++ b/misc/e2image.8.in
>> @@ -50,7 +50,10 @@ and
>> by using the
>> .B \-i
>> option to those programs. This can assist an expert in recovering
>> -catastrophically corrupted file systems.
>> +catastrophically corrupted file systems. If you going to grab an
>> +image from a corrupted FS
>> +.B \-E ignore_errors
>> +option to ignore fs errors, allows to grab fs image from a corrupted fs.
>
> Don't restate things in manual pages.
>
> "If you know the filesystem is corrupt, see the -E ignore_errors option
> below for information about how to tell e2image to deal with that.”
>
I will fix it in the next version, thanks.
>> .PP
>> It is a very good idea to create image files for all file systems on a
>> system and save the partition layout (which can be generated using the
>> @@ -137,6 +140,16 @@ useful if the file system is being cloned to a flash-based storage device
>> (where reads are very fast and where it is desirable to avoid unnecessary
>> writes to reduce write wear on the device).
>> .TP
>> +.BI \-E " extended_options"
>> +Set e2image extended options. Extended options are comma separated, and
>> +may take an argument using the equals ('=') sign. The following options
>> +are supported:
>> +.RS 1.2i
>> +.TP
>> +.BI ignore_error
>
> But you said it was -E ignore_errors above. Which is it?
I will fix it in the next version, thanks.
>
>> +Grab an image from a corrupted FS and ignore fs errors.
>> +.RE
>> +.TP
>> .B \-f
>> Override the read-only requirement for the source file system when saving
>> the image file using the
>> diff --git a/misc/e2image.c b/misc/e2image.c
>> index 2c1f3db3..45b8c2d5 100644
>> --- a/misc/e2image.c
>> +++ b/misc/e2image.c
>> @@ -53,6 +53,7 @@ extern int optind;
>> #include "support/nls-enable.h"
>> #include "support/plausible.h"
>> #include "support/quotaio.h"
>> +#include "support/mvstring.h"
>> #include "../version.h"
>>
>> #define QCOW_OFLAG_COPIED (1ULL << 63)
>> @@ -79,6 +80,7 @@ static char move_mode;
>> static char show_progress;
>> static char *check_buf;
>> static int skipped_blocks;
>> +static int ignore_errors = 0;
>>
>> static blk64_t align_offset(blk64_t offset, unsigned int n)
>> {
>> @@ -106,7 +108,7 @@ static int get_bits_from_size(size_t size)
>> static void usage(void)
>> {
>> fprintf(stderr, _("Usage: %s [ -r|-Q ] [ -f ] [ -b superblock ] [ -B blocksize ] "
>> - "device image-file\n"),
>> + "[-E extended-options] device image-file\n"),
>> program_name);
>> fprintf(stderr, _(" %s -I device image-file\n"), program_name);
>> fprintf(stderr, _(" %s -ra [ -cfnp ] [ -o src_offset ] "
>> @@ -1379,7 +1381,8 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags,
>> com_err(program_name, retval,
>> _("while iterating over inode %u"),
>> ino);
>> - exit(1);
>> + if (ignore_errors == 0)
>> + exit(1);
>> }
>> } else {
>> if ((inode.i_flags & EXT4_EXTENTS_FL) ||
>> @@ -1392,7 +1395,8 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags,
>> if (retval) {
>> com_err(program_name, retval,
>> _("while iterating over inode %u"), ino);
>> - exit(1);
>> + if (ignore_errors == 0)
>> + exit(1);
>> }
>> }
>> }
>> @@ -1486,6 +1490,40 @@ static struct ext2_qcow2_hdr *check_qcow2_image(int *fd, char *name)
>> return qcow2_read_header(*fd);
>> }
>>
>> +static void parse_extended_opts(const char *opts)
>> +{
>> + char *buf, *token, *next, *p;
>> + int ea_ver;
>> + int extended_usage = 0;
>> + unsigned long long reada_kb;
>> +
>> + buf = string_copy(opts);
>> + for (token = buf; token && *token; token = next) {
>> + p = strchr(token, ',');
>> + next = 0;
>> + if (p) {
>> + *p = 0;
>> + next = p+1;
>> + }
>> + if (strcmp(token, "ignore_errors") == 0) {
>
> getsubopt() ?
>
>> + ignore_errors = 1;
>> + continue;
>> + } else {
>> + fprintf(stderr, _("Unknown extended option: %s\n"),
>> + token);
>> + extended_usage++;
>> + }
>> + }
>> + free(buf);
>> +
>> + if (extended_usage) {
>> + fputs(_("\nExtended options are separated by commas. "
>> + "Valid extended options are:\n\n"), stderr);
>> + fputs("\tignore_errors\n", stderr);
>> + exit(1);
>> + }
>> +}
>> +
>> int main (int argc, char ** argv)
>> {
>> int c;
>> @@ -1506,6 +1544,7 @@ int main (int argc, char ** argv)
>> struct stat st;
>> blk64_t superblock = 0;
>> int blocksize = 0;
>> + char *extended_opts = 0;
>>
>> #ifdef ENABLE_NLS
>> setlocale(LC_MESSAGES, "");
>> @@ -1519,7 +1558,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, "b:B:nrsIQafo:O:pc")) != EOF)
>> + while ((c = getopt(argc, argv, "b:B:E:nrsIQafo:O:pc")) != EOF)
>> switch (c) {
>> case 'b':
>> superblock = strtoull(optarg, NULL, 0);
>> @@ -1527,6 +1566,9 @@ int main (int argc, char ** argv)
>> case 'B':
>> blocksize = strtoul(optarg, NULL, 0);
>> break;
>> + case 'E':
>> + extended_opts = optarg;
>> + break;
>> case 'I':
>> flags |= E2IMAGE_INSTALL_FLAG;
>> break;
>> @@ -1609,6 +1651,9 @@ int main (int argc, char ** argv)
>> exit(1);
>> }
>>
>> + if (extended_opts)
>> + parse_extended_opts(extended_opts);
>> +
>> if (img_type && !ignore_rw_mount &&
>> (mount_flags & EXT2_MF_MOUNTED) &&
>> !(mount_flags & EXT2_MF_READONLY)) {
>> diff --git a/misc/e2initrd_helper.c b/misc/e2initrd_helper.c
>> index 436aab8c..ab5991a4 100644
>> --- a/misc/e2initrd_helper.c
>> +++ b/misc/e2initrd_helper.c
>> @@ -36,6 +36,7 @@ extern char *optarg;
>> #include "ext2fs/ext2fs.h"
>> #include "blkid/blkid.h"
>> #include "support/nls-enable.h"
>> +#include "support/mvstring.h"
>>
>> #include "../version.h"
>>
>> @@ -151,21 +152,6 @@ static int mem_file_eof(struct mem_file *file)
>> return (file->ptr >= file->size);
>> }
>>
>> -/*
>> - * fstab parsing code
>> - */
>> -static char *string_copy(const char *s)
>> -{
>> - char *ret;
>> -
>> - if (!s)
>> - return 0;
>> - ret = malloc(strlen(s)+1);
>> - if (ret)
>> - strcpy(ret, s);
>> - return ret;
>> -}
>> -
>> static char *skip_over_blank(char *cp)
>> {
>> while (*cp && isspace(*cp))
>> diff --git a/tests/i_error_tolerance/expect.1 b/tests/i_error_tolerance/expect.1
>> new file mode 100644
>> index 00000000..e8d64954
>> --- /dev/null
>> +++ b/tests/i_error_tolerance/expect.1
>> @@ -0,0 +1,23 @@
>> +Pass 1: Checking inodes, blocks, and sizes
>> +Inode 12 has illegal block(s). Clear? yes
>> +
>> +Illegal indirect block (1000000) in inode 12. CLEARED.
>> +Inode 12, i_blocks is 34, should be 24. Fix? yes
>> +
>> +Pass 2: Checking directory structure
>> +Pass 3: Checking directory connectivity
>> +Pass 4: Checking reference counts
>> +Pass 5: Checking group summary information
>> +Block bitmap differences: -(31--34) -41
>> +Fix? yes
>> +
>> +Free blocks count wrong for group #0 (158, counted=163).
>> +Fix? yes
>> +
>> +Free blocks count wrong (158, counted=163).
>> +Fix? yes
>> +
>> +
>> +test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
>> +test_filesys: 12/24 files (8.3% non-contiguous), 37/200 blocks
>> +Exit status is 1
>> diff --git a/tests/i_error_tolerance/expect.2 b/tests/i_error_tolerance/expect.2
>> new file mode 100644
>> index 00000000..d9fcc327
>> --- /dev/null
>> +++ b/tests/i_error_tolerance/expect.2
>> @@ -0,0 +1,7 @@
>> +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
>> +test_filesys: 12/24 files (8.3% non-contiguous), 37/200 blocks
>> +Exit status is 0
>> diff --git a/tests/i_error_tolerance/script b/tests/i_error_tolerance/script
>> new file mode 100644
>> index 00000000..315569c7
>> --- /dev/null
>> +++ b/tests/i_error_tolerance/script
>> @@ -0,0 +1,47 @@
>> +if ! test -x $E2IMAGE_EXE; then
>> + echo "$test_name: $test_description: skipped (no e2image)"
>> + return 0
>> +fi
>> +if ! test -x $DEBUGFS_EXE; then
>> + echo "$test_name: $test_description: skipped (no debugfs)"
>> + return 0
>> +fi
>> +
>> +SKIP_GUNZIP="true"
>> +
>> +TEST_DATA="$test_name.tmp"
>> +dd if=/dev/urandom of=$TEST_DATA bs=1k count=16 > /dev/null 2>&1
>> +
>> +dd if=/dev/zero of=$TMPFILE bs=1k count=200 > /dev/null 2>&1
>> +$MKE2FS -Ft ext4 -O ^extents $TMPFILE > /dev/null 2>&1
>> +$DEBUGFS -w $TMPFILE << EOF > /dev/null 2>&1
>> +write $TEST_DATA testfile
>> +set_inode_field testfile block[IND] 1000000
>> +q
>> +EOF
>> +
>> +$E2IMAGE -r $TMPFILE $TMPFILE.back
>> +
>> +if [ $? = 0 ] ; then
>> + echo "Image expected to be broken"
>> + echo "$test_name: $test_description: fail"
>> + touch $test_name.failed
>> + return 0
>> +fi
>> +
>> +$E2IMAGE -r -E ignore_errors $TMPFILE $TMPFILE.back
>> +
>> +if [ $? = 1 ] ; then
>> + echo "Can not get image even with ignore_errors"
>> + echo "$test_name: $test_description: fail"
>> + touch $test_name.failed
>> + return 0
>> +fi
>> +
>> +mv $TMPFILE.back $TMPFILE
>> +
>> +. $cmd_dir/run_e2fsck
>> +
>> +rm -f $TEST_DATA
>> +
>> +unset E2FSCK_TIME TEST_DATA
>> --
>> 2.18.4
Best regards,
Artem Blagodarenko.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-12 14:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-09 14:23 [PATCH v6] e2image: add option to ignore fs errors Artem Blagodarenko
2021-10-11 17:03 ` Darrick J. Wong
2021-10-12 14:53 ` Artem Blagodarenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox