* [PATCH] tune2fs: confirm slow/dangerous operations
@ 2015-12-03 20:40 Darrick J. Wong
2015-12-04 22:12 ` Andreas Dilger
0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2015-12-03 20:40 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, Andreas Dilger
Give admins a short amount of time to confirm that they want to
proceed with a dangerous operation. Refuse to perform the op
unless the filesystem is freshly checked.
Cc: Andreas Dilger <adilger@dilger.ca>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
misc/tune2fs.c | 41 ++++++++++----
tests/t_dangerous/expect | 97 +++++++++++++++++++++++++++++++++
tests/t_dangerous/name | 1
tests/t_dangerous/script | 134 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 260 insertions(+), 13 deletions(-)
create mode 100644 tests/t_dangerous/expect
create mode 100644 tests/t_dangerous/name
create mode 100644 tests/t_dangerous/script
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index af7d73c..aaa1597 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -405,14 +405,25 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
return 0;
}
-static int check_fsck_needed(ext2_filsys fs)
+static void check_fsck_needed(ext2_filsys fs, const char *prompt)
{
- if (fs->super->s_state & EXT2_VALID_FS)
- return 0;
- printf("\n%s\n", _(please_fsck));
- if (mount_flags & EXT2_MF_READONLY)
- printf("%s", _("(and reboot afterwards!)\n"));
- return 1;
+ /* Refuse to modify anything but a freshly checked valid filesystem. */
+ if (!(fs->super->s_state & EXT2_VALID_FS) ||
+ (fs->super->s_state & EXT2_ERROR_FS) ||
+ (fs->super->s_lastcheck < fs->super->s_mtime)) {
+ printf("\n%s\n", _(please_fsck));
+ if (mount_flags & EXT2_MF_READONLY)
+ printf("%s", _("(and reboot afterwards!)\n"));
+ exit(1);
+ }
+
+ /* Give the admin a few seconds to bail out of a dangerous op. */
+ if (!getenv("TUNE2FS_FORCE_PROMPT") && (!isatty(0) || !isatty(1)))
+ return;
+
+ puts(prompt);
+ proceed_question(5);
+ return;
}
static void request_dir_fsck_afterwards(ext2_filsys fs)
@@ -1145,8 +1156,8 @@ mmp_error:
if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
- if (check_fsck_needed(fs))
- exit(1);
+ check_fsck_needed(fs,
+ _("Enabling checksums could take some time."));
if (mount_flags & EXT2_MF_MOUNTED) {
fputs(_("Cannot enable metadata_csum on a mounted "
"filesystem!\n"), stderr);
@@ -1186,8 +1197,8 @@ mmp_error:
EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
__u32 test_features[3];
- if (check_fsck_needed(fs))
- exit(1);
+ check_fsck_needed(fs,
+ _("Disabling checksums could take some time."));
if (mount_flags & EXT2_MF_MOUNTED) {
fputs(_("Cannot disable metadata_csum on a mounted "
"filesystem!\n"), stderr);
@@ -2784,6 +2795,8 @@ retry_open:
rc = 1;
goto closefs;
}
+ check_fsck_needed(fs,
+ _("Resizing inodes could take some time."));
/*
* If inode resize is requested use the
* Undo I/O manager
@@ -2999,8 +3012,10 @@ retry_open:
try_confirm_csum_seed_support();
exit(1);
}
- if (check_fsck_needed(fs))
- exit(1);
+ if (!ext2fs_has_feature_csum_seed(fs->super))
+ check_fsck_needed(fs,
+ _("Setting UUID on a checksummed "
+ "filesystem could take some time."));
/*
* Determine if the block group checksums are
diff --git a/tests/t_dangerous/expect b/tests/t_dangerous/expect
new file mode 100644
index 0000000..353bd57
--- /dev/null
+++ b/tests/t_dangerous/expect
@@ -0,0 +1,97 @@
+tune2fs dangerous prompts test
+Creating filesystem with 524288 1k blocks and 65536 inodes
+Superblock backups stored on blocks:
+ 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
+
+Allocating group tables: \b\b\b\b\bdone
+Writing inode tables: \b\b\b\b\bdone
+Creating journal (16384 blocks): done
+Creating 445 huge file(s) with 1024 blocks each: done
+Writing superblocks and filesystem accounting information: \b\b\b\b\bdone
+
+tune2fs -O metadata_csum test.img
+
+Please run e2fsck on the filesystem.
+
+Exit status is 1
+tune2fs -O metadata_csum test.img
+Exit status is 0
+Creating filesystem with 524288 1k blocks and 65536 inodes
+Superblock backups stored on blocks:
+ 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
+
+Allocating group tables: \b\b\b\b\bdone
+Writing inode tables: \b\b\b\b\bdone
+Creating journal (16384 blocks): done
+Creating 445 huge file(s) with 1024 blocks each: done
+Writing superblocks and filesystem accounting information: \b\b\b\b\bdone
+
+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
+
+Exit status is 0
+tune2fs -O metadata_csum test.img
+Enabling checksums could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n)
+Exit status is 1
+tune2fs -I 512 test.img
+Resizing inodes could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n)
+Exit status is 1
+tune2fs -U random test.img
+Setting UUID on a checksummed filesystem could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n)
+Exit status is 1
+
+Change in FS metadata:
+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
+
+Exit status is 0
+tune2fs -O metadata_csum test.img
+Enabling checksums could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n)
+Please run e2fsck -D on the filesystem.
+
+Exit status is 0
+test_filesys was not cleanly unmounted, check forced.
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 3A: Optimizing directories
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+
+
+
+tune2fs -I 512 test.img
+Resizing inodes could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n) Setting inode size 512
+Exit status is 0
+tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img
+Setting UUID on a checksummed filesystem could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n) Exit status is 0
+Backing up journal inode block information.
+
+
+Change in FS metadata:
+@@ -1,3 +1,3 @@
+-Filesystem UUID: 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf
+-Filesystem features: has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
+-Inode size: 256
++Filesystem UUID: f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0
++Filesystem features: has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
++Inode size: 512
+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
+
+Exit status is 0
diff --git a/tests/t_dangerous/name b/tests/t_dangerous/name
new file mode 100644
index 0000000..c9a1030
--- /dev/null
+++ b/tests/t_dangerous/name
@@ -0,0 +1 @@
+dangerous tune2fs operation prompts
diff --git a/tests/t_dangerous/script b/tests/t_dangerous/script
new file mode 100644
index 0000000..ee6e90c
--- /dev/null
+++ b/tests/t_dangerous/script
@@ -0,0 +1,134 @@
+FSCK_OPT=-fn
+OUT=$test_name.log
+EXP=$test_dir/expect
+CONF=$TMPFILE.conf
+
+cat > $CONF << ENDL
+[fs_types]
+ ext4h = {
+ features = has_journal,extent,huge_file,^flex_bg,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,resize_inode,64bit
+ blocksize = 1024
+ inode_size = 256
+ make_hugefiles = true
+ hugefiles_dir = /
+ hugefiles_slack = 32M
+ hugefiles_name = aaaaa
+ hugefiles_digits = 4
+ hugefiles_size = 1M
+ zero_hugefiles = false
+ }
+ENDL
+
+echo "tune2fs dangerous prompts test" > $OUT
+
+MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
+
+# trigger a filesystem check
+$DEBUGFS -w -R 'ssv mtime 300000000' $TMPFILE > /dev/null 2>&1
+$DEBUGFS -w -R 'ssv lastcheck 20000' $TMPFILE > /dev/null 2>&1
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# check fs
+$FSCK -f -y -N test_filesys $TMPFILE > /dev/null 2>&1
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
+
+# dump and check
+$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.before
+$FSCK $FSCK_OPT -f -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# change inode size
+echo "tune2fs -I 512 test.img" >> $OUT
+echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# change uuid
+echo "tune2fs -U random test.img" >> $OUT
+echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U random $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# check
+$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
+
+# dump and check
+$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.after
+echo "Change in FS metadata:" >> $OUT
+diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.before
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
+
+# change inode size
+echo "tune2fs -I 512 test.img" >> $OUT
+echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# change uuid
+echo "tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img" >> $OUT
+echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# check
+$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
+
+# dump and check
+$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.after
+echo "Change in FS metadata:" >> $OUT
+diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+rm $TMPFILE $OUT.before $OUT.after $CONF
+
+#
+# Do the verification
+#
+
+sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
+mv $OUT.new $OUT
+
+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 $DIFF_OPTS $EXP $OUT > $test_name.failed
+fi
+
+unset IMAGE FSCK_OPT OUT EXP CONF
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tune2fs: confirm slow/dangerous operations
2015-12-03 20:40 [PATCH] tune2fs: confirm slow/dangerous operations Darrick J. Wong
@ 2015-12-04 22:12 ` Andreas Dilger
2015-12-04 23:07 ` Darrick J. Wong
0 siblings, 1 reply; 7+ messages in thread
From: Andreas Dilger @ 2015-12-04 22:12 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Theodore Ts'o, linux-ext4
[-- Attachment #1: Type: text/plain, Size: 12837 bytes --]
On Dec 3, 2015, at 1:40 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> Give admins a short amount of time to confirm that they want to
> proceed with a dangerous operation. Refuse to perform the op
> unless the filesystem is freshly checked.
>
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> misc/tune2fs.c | 41 ++++++++++----
> tests/t_dangerous/expect | 97 +++++++++++++++++++++++++++++++++
> tests/t_dangerous/name | 1
> tests/t_dangerous/script | 134 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 260 insertions(+), 13 deletions(-)
> create mode 100644 tests/t_dangerous/expect
> create mode 100644 tests/t_dangerous/name
> create mode 100644 tests/t_dangerous/script
>
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index af7d73c..aaa1597 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -405,14 +405,25 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
> return 0;
> }
>
> -static int check_fsck_needed(ext2_filsys fs)
> +static void check_fsck_needed(ext2_filsys fs, const char *prompt)
> {
> - if (fs->super->s_state & EXT2_VALID_FS)
> - return 0;
> - printf("\n%s\n", _(please_fsck));
> - if (mount_flags & EXT2_MF_READONLY)
> - printf("%s", _("(and reboot afterwards!)\n"));
> - return 1;
> + /* Refuse to modify anything but a freshly checked valid filesystem. */
> + if (!(fs->super->s_state & EXT2_VALID_FS) ||
> + (fs->super->s_state & EXT2_ERROR_FS) ||
> + (fs->super->s_lastcheck < fs->super->s_mtime)) {
> + printf("\n%s\n", _(please_fsck));
> + if (mount_flags & EXT2_MF_READONLY)
> + printf("%s", _("(and reboot afterwards!)\n"));
> + exit(1);
> + }
Should this explicitly check for NEEDS_RECOVERY, or force journal replay
directly? It would be a sad thing if the filesystem was modified and then
journal replay clobbered it.
> +
> + /* Give the admin a few seconds to bail out of a dangerous op. */
> + if (!getenv("TUNE2FS_FORCE_PROMPT") && (!isatty(0) || !isatty(1)))
> + return;
> +
> + puts(prompt);
> + proceed_question(5);
> + return;
> }
>
> static void request_dir_fsck_afterwards(ext2_filsys fs)
> @@ -1145,8 +1156,8 @@ mmp_error:
>
> if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> - if (check_fsck_needed(fs))
> - exit(1);
> + check_fsck_needed(fs,
> + _("Enabling checksums could take some time."));
> if (mount_flags & EXT2_MF_MOUNTED) {
> fputs(_("Cannot enable metadata_csum on a mounted "
> "filesystem!\n"), stderr);
> @@ -1186,8 +1197,8 @@ mmp_error:
> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> __u32 test_features[3];
>
> - if (check_fsck_needed(fs))
> - exit(1);
> + check_fsck_needed(fs,
> + _("Disabling checksums could take some time."));
> if (mount_flags & EXT2_MF_MOUNTED) {
> fputs(_("Cannot disable metadata_csum on a mounted "
> "filesystem!\n"), stderr);
> @@ -2784,6 +2795,8 @@ retry_open:
> rc = 1;
> goto closefs;
> }
> + check_fsck_needed(fs,
> + _("Resizing inodes could take some time."));
> /*
> * If inode resize is requested use the
> * Undo I/O manager
> @@ -2999,8 +3012,10 @@ retry_open:
> try_confirm_csum_seed_support();
> exit(1);
> }
> - if (check_fsck_needed(fs))
> - exit(1);
> + if (!ext2fs_has_feature_csum_seed(fs->super))
> + check_fsck_needed(fs,
> + _("Setting UUID on a checksummed "
> + "filesystem could take some time."));
>
> /*
> * Determine if the block group checksums are
> diff --git a/tests/t_dangerous/expect b/tests/t_dangerous/expect
> new file mode 100644
> index 0000000..353bd57
> --- /dev/null
> +++ b/tests/t_dangerous/expect
> @@ -0,0 +1,97 @@
> +tune2fs dangerous prompts test
> +Creating filesystem with 524288 1k blocks and 65536 inodes
> +Superblock backups stored on blocks:
> + 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
> +
> +Allocating group tables: \b\b\b\b\bdone
> +Writing inode tables: \b\b\b\b\bdone
> +Creating journal (16384 blocks): done
> +Creating 445 huge file(s) with 1024 blocks each: done
> +Writing superblocks and filesystem accounting information: \b\b\b\b\bdone
> +
> +tune2fs -O metadata_csum test.img
> +
> +Please run e2fsck on the filesystem.
> +
> +Exit status is 1
> +tune2fs -O metadata_csum test.img
> +Exit status is 0
> +Creating filesystem with 524288 1k blocks and 65536 inodes
> +Superblock backups stored on blocks:
> + 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
> +
> +Allocating group tables: \b\b\b\b\bdone
> +Writing inode tables: \b\b\b\b\bdone
> +Creating journal (16384 blocks): done
> +Creating 445 huge file(s) with 1024 blocks each: done
> +Writing superblocks and filesystem accounting information: \b\b\b\b\bdone
> +
> +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
> +
> +Exit status is 0
> +tune2fs -O metadata_csum test.img
> +Enabling checksums could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n)
> +Exit status is 1
> +tune2fs -I 512 test.img
> +Resizing inodes could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n)
> +Exit status is 1
> +tune2fs -U random test.img
> +Setting UUID on a checksummed filesystem could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n)
> +Exit status is 1
Hmm, but shouldn't the "proceed anyway" prompt actually continue
if there is no input so that scripts don't break? Why do these
questions even get printed in the first place when running a script?
Cheers, Andreas
> +Change in FS metadata:
> +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
> +
> +Exit status is 0
> +tune2fs -O metadata_csum test.img
> +Enabling checksums could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n)
> +Please run e2fsck -D on the filesystem.
> +
> +Exit status is 0
> +test_filesys was not cleanly unmounted, check forced.
> +Pass 1: Checking inodes, blocks, and sizes
> +Pass 2: Checking directory structure
> +Pass 3: Checking directory connectivity
> +Pass 3A: Optimizing directories
> +Pass 4: Checking reference counts
> +Pass 5: Checking group summary information
> +
> +
> +
> +tune2fs -I 512 test.img
> +Resizing inodes could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n) Setting inode size 512
> +Exit status is 0
> +tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img
> +Setting UUID on a checksummed filesystem could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n) Exit status is 0
> +Backing up journal inode block information.
> +
> +
> +Change in FS metadata:
> +@@ -1,3 +1,3 @@
> +-Filesystem UUID: 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf
> +-Filesystem features: has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> +-Inode size: 256
> ++Filesystem UUID: f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0
> ++Filesystem features: has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> ++Inode size: 512
> +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
> +
> +Exit status is 0
> diff --git a/tests/t_dangerous/name b/tests/t_dangerous/name
> new file mode 100644
> index 0000000..c9a1030
> --- /dev/null
> +++ b/tests/t_dangerous/name
> @@ -0,0 +1 @@
> +dangerous tune2fs operation prompts
> diff --git a/tests/t_dangerous/script b/tests/t_dangerous/script
> new file mode 100644
> index 0000000..ee6e90c
> --- /dev/null
> +++ b/tests/t_dangerous/script
> @@ -0,0 +1,134 @@
> +FSCK_OPT=-fn
> +OUT=$test_name.log
> +EXP=$test_dir/expect
> +CONF=$TMPFILE.conf
> +
> +cat > $CONF << ENDL
> +[fs_types]
> + ext4h = {
> + features = has_journal,extent,huge_file,^flex_bg,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,resize_inode,64bit
> + blocksize = 1024
> + inode_size = 256
> + make_hugefiles = true
> + hugefiles_dir = /
> + hugefiles_slack = 32M
> + hugefiles_name = aaaaa
> + hugefiles_digits = 4
> + hugefiles_size = 1M
> + zero_hugefiles = false
> + }
> +ENDL
> +
> +echo "tune2fs dangerous prompts test" > $OUT
> +
> +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> +
> +# trigger a filesystem check
> +$DEBUGFS -w -R 'ssv mtime 300000000' $TMPFILE > /dev/null 2>&1
> +$DEBUGFS -w -R 'ssv lastcheck 20000' $TMPFILE > /dev/null 2>&1
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# check fs
> +$FSCK -f -y -N test_filesys $TMPFILE > /dev/null 2>&1
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> +
> +# dump and check
> +$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.before
> +$FSCK $FSCK_OPT -f -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# change inode size
> +echo "tune2fs -I 512 test.img" >> $OUT
> +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# change uuid
> +echo "tune2fs -U random test.img" >> $OUT
> +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U random $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# check
> +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> +
> +# dump and check
> +$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.after
> +echo "Change in FS metadata:" >> $OUT
> +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.before
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> +
> +# change inode size
> +echo "tune2fs -I 512 test.img" >> $OUT
> +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# change uuid
> +echo "tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img" >> $OUT
> +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# check
> +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> +
> +# dump and check
> +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.after
> +echo "Change in FS metadata:" >> $OUT
> +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +rm $TMPFILE $OUT.before $OUT.after $CONF
> +
> +#
> +# Do the verification
> +#
> +
> +sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
> +mv $OUT.new $OUT
> +
> +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 $DIFF_OPTS $EXP $OUT > $test_name.failed
> +fi
> +
> +unset IMAGE FSCK_OPT OUT EXP CONF
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tune2fs: confirm slow/dangerous operations
2015-12-04 22:12 ` Andreas Dilger
@ 2015-12-04 23:07 ` Darrick J. Wong
2015-12-04 23:36 ` Theodore Ts'o
2015-12-04 23:54 ` Andreas Dilger
0 siblings, 2 replies; 7+ messages in thread
From: Darrick J. Wong @ 2015-12-04 23:07 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Theodore Ts'o, linux-ext4
On Fri, Dec 04, 2015 at 03:12:53PM -0700, Andreas Dilger wrote:
> On Dec 3, 2015, at 1:40 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > Give admins a short amount of time to confirm that they want to
> > proceed with a dangerous operation. Refuse to perform the op
> > unless the filesystem is freshly checked.
> >
> > Cc: Andreas Dilger <adilger@dilger.ca>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > misc/tune2fs.c | 41 ++++++++++----
> > tests/t_dangerous/expect | 97 +++++++++++++++++++++++++++++++++
> > tests/t_dangerous/name | 1
> > tests/t_dangerous/script | 134 ++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 260 insertions(+), 13 deletions(-)
> > create mode 100644 tests/t_dangerous/expect
> > create mode 100644 tests/t_dangerous/name
> > create mode 100644 tests/t_dangerous/script
> >
> > diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> > index af7d73c..aaa1597 100644
> > --- a/misc/tune2fs.c
> > +++ b/misc/tune2fs.c
> > @@ -405,14 +405,25 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
> > return 0;
> > }
> >
> > -static int check_fsck_needed(ext2_filsys fs)
> > +static void check_fsck_needed(ext2_filsys fs, const char *prompt)
> > {
> > - if (fs->super->s_state & EXT2_VALID_FS)
> > - return 0;
> > - printf("\n%s\n", _(please_fsck));
> > - if (mount_flags & EXT2_MF_READONLY)
> > - printf("%s", _("(and reboot afterwards!)\n"));
> > - return 1;
> > + /* Refuse to modify anything but a freshly checked valid filesystem. */
> > + if (!(fs->super->s_state & EXT2_VALID_FS) ||
> > + (fs->super->s_state & EXT2_ERROR_FS) ||
> > + (fs->super->s_lastcheck < fs->super->s_mtime)) {
> > + printf("\n%s\n", _(please_fsck));
> > + if (mount_flags & EXT2_MF_READONLY)
> > + printf("%s", _("(and reboot afterwards!)\n"));
> > + exit(1);
> > + }
>
> Should this explicitly check for NEEDS_RECOVERY, or force journal replay
> directly? It would be a sad thing if the filesystem was modified and then
> journal replay clobbered it.
I was under the impression that the patch "tune2fs: warn if the filesystem
journal is dirty" was sufficient to discourage journal-clobbering?
AFAICT, that patch runs for any invocation of tune2fs, whereas
check_fsck_needed only applies to "dangerous" operations, i.e. the ones that
want to rewrite big chunks of FS.
Ted took it, but I don't think he's pushed his tree to kernel.org recently.
>
> > +
> > + /* Give the admin a few seconds to bail out of a dangerous op. */
> > + if (!getenv("TUNE2FS_FORCE_PROMPT") && (!isatty(0) || !isatty(1)))
> > + return;
> > +
> > + puts(prompt);
> > + proceed_question(5);
> > + return;
> > }
> >
> > static void request_dir_fsck_afterwards(ext2_filsys fs)
> > @@ -1145,8 +1156,8 @@ mmp_error:
> >
> > if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
> > EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> > - if (check_fsck_needed(fs))
> > - exit(1);
> > + check_fsck_needed(fs,
> > + _("Enabling checksums could take some time."));
> > if (mount_flags & EXT2_MF_MOUNTED) {
> > fputs(_("Cannot enable metadata_csum on a mounted "
> > "filesystem!\n"), stderr);
> > @@ -1186,8 +1197,8 @@ mmp_error:
> > EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> > __u32 test_features[3];
> >
> > - if (check_fsck_needed(fs))
> > - exit(1);
> > + check_fsck_needed(fs,
> > + _("Disabling checksums could take some time."));
> > if (mount_flags & EXT2_MF_MOUNTED) {
> > fputs(_("Cannot disable metadata_csum on a mounted "
> > "filesystem!\n"), stderr);
> > @@ -2784,6 +2795,8 @@ retry_open:
> > rc = 1;
> > goto closefs;
> > }
> > + check_fsck_needed(fs,
> > + _("Resizing inodes could take some time."));
> > /*
> > * If inode resize is requested use the
> > * Undo I/O manager
> > @@ -2999,8 +3012,10 @@ retry_open:
> > try_confirm_csum_seed_support();
> > exit(1);
> > }
> > - if (check_fsck_needed(fs))
> > - exit(1);
> > + if (!ext2fs_has_feature_csum_seed(fs->super))
> > + check_fsck_needed(fs,
> > + _("Setting UUID on a checksummed "
> > + "filesystem could take some time."));
> >
> > /*
> > * Determine if the block group checksums are
> > diff --git a/tests/t_dangerous/expect b/tests/t_dangerous/expect
> > new file mode 100644
> > index 0000000..353bd57
> > --- /dev/null
> > +++ b/tests/t_dangerous/expect
> > @@ -0,0 +1,97 @@
> > +tune2fs dangerous prompts test
> > +Creating filesystem with 524288 1k blocks and 65536 inodes
> > +Superblock backups stored on blocks:
> > + 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
> > +
> > +Allocating group tables: \b\b\b\b\bdone
> > +Writing inode tables: \b\b\b\b\bdone
> > +Creating journal (16384 blocks): done
> > +Creating 445 huge file(s) with 1024 blocks each: done
> > +Writing superblocks and filesystem accounting information: \b\b\b\b\bdone
> > +
> > +tune2fs -O metadata_csum test.img
> > +
> > +Please run e2fsck on the filesystem.
> > +
> > +Exit status is 1
> > +tune2fs -O metadata_csum test.img
> > +Exit status is 0
> > +Creating filesystem with 524288 1k blocks and 65536 inodes
> > +Superblock backups stored on blocks:
> > + 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
> > +
> > +Allocating group tables: \b\b\b\b\bdone
> > +Writing inode tables: \b\b\b\b\bdone
> > +Creating journal (16384 blocks): done
> > +Creating 445 huge file(s) with 1024 blocks each: done
> > +Writing superblocks and filesystem accounting information: \b\b\b\b\bdone
> > +
> > +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
> > +
> > +Exit status is 0
> > +tune2fs -O metadata_csum test.img
> > +Enabling checksums could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n)
> > +Exit status is 1
> > +tune2fs -I 512 test.img
> > +Resizing inodes could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n)
> > +Exit status is 1
> > +tune2fs -U random test.img
> > +Setting UUID on a checksummed filesystem could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n)
> > +Exit status is 1
>
> Hmm, but shouldn't the "proceed anyway" prompt actually continue
> if there is no input so that scripts don't break?
There /is/ input; the test script echoes 'n' or 'y' to simulate the TTY:
echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> Why do these questions even get printed in the first place when running a
> script?
I put in an environment variable that forces tune2fs to display the prompt
for testing purposes.
--D
>
> Cheers, Andreas
>
> > +Change in FS metadata:
> > +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
> > +
> > +Exit status is 0
> > +tune2fs -O metadata_csum test.img
> > +Enabling checksums could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n)
> > +Please run e2fsck -D on the filesystem.
> > +
> > +Exit status is 0
> > +test_filesys was not cleanly unmounted, check forced.
> > +Pass 1: Checking inodes, blocks, and sizes
> > +Pass 2: Checking directory structure
> > +Pass 3: Checking directory connectivity
> > +Pass 3A: Optimizing directories
> > +Pass 4: Checking reference counts
> > +Pass 5: Checking group summary information
> > +
> > +
> > +
> > +tune2fs -I 512 test.img
> > +Resizing inodes could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n) Setting inode size 512
> > +Exit status is 0
> > +tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img
> > +Setting UUID on a checksummed filesystem could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n) Exit status is 0
> > +Backing up journal inode block information.
> > +
> > +
> > +Change in FS metadata:
> > +@@ -1,3 +1,3 @@
> > +-Filesystem UUID: 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf
> > +-Filesystem features: has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> > +-Inode size: 256
> > ++Filesystem UUID: f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0
> > ++Filesystem features: has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> > ++Inode size: 512
> > +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
> > +
> > +Exit status is 0
> > diff --git a/tests/t_dangerous/name b/tests/t_dangerous/name
> > new file mode 100644
> > index 0000000..c9a1030
> > --- /dev/null
> > +++ b/tests/t_dangerous/name
> > @@ -0,0 +1 @@
> > +dangerous tune2fs operation prompts
> > diff --git a/tests/t_dangerous/script b/tests/t_dangerous/script
> > new file mode 100644
> > index 0000000..ee6e90c
> > --- /dev/null
> > +++ b/tests/t_dangerous/script
> > @@ -0,0 +1,134 @@
> > +FSCK_OPT=-fn
> > +OUT=$test_name.log
> > +EXP=$test_dir/expect
> > +CONF=$TMPFILE.conf
> > +
> > +cat > $CONF << ENDL
> > +[fs_types]
> > + ext4h = {
> > + features = has_journal,extent,huge_file,^flex_bg,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,resize_inode,64bit
> > + blocksize = 1024
> > + inode_size = 256
> > + make_hugefiles = true
> > + hugefiles_dir = /
> > + hugefiles_slack = 32M
> > + hugefiles_name = aaaaa
> > + hugefiles_digits = 4
> > + hugefiles_size = 1M
> > + zero_hugefiles = false
> > + }
> > +ENDL
> > +
> > +echo "tune2fs dangerous prompts test" > $OUT
> > +
> > +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> > +
> > +# trigger a filesystem check
> > +$DEBUGFS -w -R 'ssv mtime 300000000' $TMPFILE > /dev/null 2>&1
> > +$DEBUGFS -w -R 'ssv lastcheck 20000' $TMPFILE > /dev/null 2>&1
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# check fs
> > +$FSCK -f -y -N test_filesys $TMPFILE > /dev/null 2>&1
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> > +
> > +# dump and check
> > +$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.before
> > +$FSCK $FSCK_OPT -f -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# change inode size
> > +echo "tune2fs -I 512 test.img" >> $OUT
> > +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# change uuid
> > +echo "tune2fs -U random test.img" >> $OUT
> > +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U random $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# check
> > +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> > +
> > +# dump and check
> > +$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.after
> > +echo "Change in FS metadata:" >> $OUT
> > +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.before
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> > +
> > +# change inode size
> > +echo "tune2fs -I 512 test.img" >> $OUT
> > +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# change uuid
> > +echo "tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img" >> $OUT
> > +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# check
> > +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> > +
> > +# dump and check
> > +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.after
> > +echo "Change in FS metadata:" >> $OUT
> > +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +rm $TMPFILE $OUT.before $OUT.after $CONF
> > +
> > +#
> > +# Do the verification
> > +#
> > +
> > +sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
> > +mv $OUT.new $OUT
> > +
> > +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 $DIFF_OPTS $EXP $OUT > $test_name.failed
> > +fi
> > +
> > +unset IMAGE FSCK_OPT OUT EXP CONF
>
>
> Cheers, Andreas
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tune2fs: confirm slow/dangerous operations
2015-12-04 23:07 ` Darrick J. Wong
@ 2015-12-04 23:36 ` Theodore Ts'o
2015-12-05 1:21 ` Darrick J. Wong
2015-12-04 23:54 ` Andreas Dilger
1 sibling, 1 reply; 7+ messages in thread
From: Theodore Ts'o @ 2015-12-04 23:36 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Andreas Dilger, linux-ext4
On Fri, Dec 04, 2015 at 03:07:45PM -0800, Darrick J. Wong wrote:
>
> Ted took it, but I don't think he's pushed his tree to kernel.org recently.
I pushed to kernel.org a few days ago, and that patch is there:
http://git.kernel.org/cgit/fs/ext2/e2fsprogs.git/commit/?h=next&id=c5b3ae7fb5d58dd12a1e02c2443bad32c8a76150
- Ted
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tune2fs: confirm slow/dangerous operations
2015-12-04 23:07 ` Darrick J. Wong
2015-12-04 23:36 ` Theodore Ts'o
@ 2015-12-04 23:54 ` Andreas Dilger
2015-12-05 1:12 ` Darrick J. Wong
1 sibling, 1 reply; 7+ messages in thread
From: Andreas Dilger @ 2015-12-04 23:54 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Theodore Ts'o, linux-ext4
[-- Attachment #1: Type: text/plain, Size: 3411 bytes --]
On Dec 4, 2015, at 4:07 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Fri, Dec 04, 2015 at 03:12:53PM -0700, Andreas Dilger wrote:
>> On Dec 3, 2015, at 1:40 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>>>
>>> Give admins a short amount of time to confirm that they want to
>>> proceed with a dangerous operation. Refuse to perform the op
>>> unless the filesystem is freshly checked.
>>>
>>> Cc: Andreas Dilger <adilger@dilger.ca>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>> ---
>>> misc/tune2fs.c | 41 ++++++++++----
>>> tests/t_dangerous/expect | 97 +++++++++++++++++++++++++++++++++
>>> tests/t_dangerous/name | 1
>>> tests/t_dangerous/script | 134 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 260 insertions(+), 13 deletions(-)
>>> create mode 100644 tests/t_dangerous/expect
>>> create mode 100644 tests/t_dangerous/name
>>> create mode 100644 tests/t_dangerous/script
>>>
>>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>>> index af7d73c..aaa1597 100644
>>> --- a/misc/tune2fs.c
>>> +++ b/misc/tune2fs.c
>>> @@ -405,14 +405,25 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
>>> return 0;
>>> }
>>>
>>> -static int check_fsck_needed(ext2_filsys fs)
>>> +static void check_fsck_needed(ext2_filsys fs, const char *prompt)
>>> {
>>> - if (fs->super->s_state & EXT2_VALID_FS)
>>> - return 0;
>>> - printf("\n%s\n", _(please_fsck));
>>> - if (mount_flags & EXT2_MF_READONLY)
>>> - printf("%s", _("(and reboot afterwards!)\n"));
>>> - return 1;
>>> + /* Refuse to modify anything but a freshly checked valid filesystem. */
>>> + if (!(fs->super->s_state & EXT2_VALID_FS) ||
>>> + (fs->super->s_state & EXT2_ERROR_FS) ||
>>> + (fs->super->s_lastcheck < fs->super->s_mtime)) {
>>> + printf("\n%s\n", _(please_fsck));
>>> + if (mount_flags & EXT2_MF_READONLY)
>>> + printf("%s", _("(and reboot afterwards!)\n"));
>>> + exit(1);
>>> + }
>>
>> Should this explicitly check for NEEDS_RECOVERY, or force journal replay
>> directly? It would be a sad thing if the filesystem was modified and then
>> journal replay clobbered it.
>
> I was under the impression that the patch "tune2fs: warn if the filesystem
> journal is dirty" was sufficient to discourage journal-clobbering?
>
> AFAICT, that patch runs for any invocation of tune2fs, whereas
> check_fsck_needed only applies to "dangerous" operations, i.e. the ones that
> want to rewrite big chunks of FS.
That check is fine as a warning for "simple" changes (e.g. setting a flag in
the superblock), since if it gets clobbered when there _may_ be a superblock
in the journal it isn't any worse than if it was never set at all.
For "dangerous" options like these, if the filesystem is modified and then
the journal is replayed (e.g resize inodes, replay journal with old inode
table blocks on top of the modified inodes) the results would be disastrous.
So for dangerous options it makes sense to either refuse to change the filesystem
if the journal is dirty, or replay the journal before changing the filesystem
if the proceed question is accepted.
It wouldn't be terrible to have a proceed question if the journal is dirty
to see if they want to replay the journal before any field is modified, and
just revert the warning patch.
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tune2fs: confirm slow/dangerous operations
2015-12-04 23:54 ` Andreas Dilger
@ 2015-12-05 1:12 ` Darrick J. Wong
0 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2015-12-05 1:12 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Theodore Ts'o, linux-ext4
On Fri, Dec 04, 2015 at 04:54:01PM -0700, Andreas Dilger wrote:
> On Dec 4, 2015, at 4:07 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > On Fri, Dec 04, 2015 at 03:12:53PM -0700, Andreas Dilger wrote:
> >> On Dec 3, 2015, at 1:40 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >>>
> >>> Give admins a short amount of time to confirm that they want to
> >>> proceed with a dangerous operation. Refuse to perform the op
> >>> unless the filesystem is freshly checked.
> >>>
> >>> Cc: Andreas Dilger <adilger@dilger.ca>
> >>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> >>> ---
> >>> misc/tune2fs.c | 41 ++++++++++----
> >>> tests/t_dangerous/expect | 97 +++++++++++++++++++++++++++++++++
> >>> tests/t_dangerous/name | 1
> >>> tests/t_dangerous/script | 134 ++++++++++++++++++++++++++++++++++++++++++++++
> >>> 4 files changed, 260 insertions(+), 13 deletions(-)
> >>> create mode 100644 tests/t_dangerous/expect
> >>> create mode 100644 tests/t_dangerous/name
> >>> create mode 100644 tests/t_dangerous/script
> >>>
> >>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> >>> index af7d73c..aaa1597 100644
> >>> --- a/misc/tune2fs.c
> >>> +++ b/misc/tune2fs.c
> >>> @@ -405,14 +405,25 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
> >>> return 0;
> >>> }
> >>>
> >>> -static int check_fsck_needed(ext2_filsys fs)
> >>> +static void check_fsck_needed(ext2_filsys fs, const char *prompt)
> >>> {
> >>> - if (fs->super->s_state & EXT2_VALID_FS)
> >>> - return 0;
> >>> - printf("\n%s\n", _(please_fsck));
> >>> - if (mount_flags & EXT2_MF_READONLY)
> >>> - printf("%s", _("(and reboot afterwards!)\n"));
> >>> - return 1;
> >>> + /* Refuse to modify anything but a freshly checked valid filesystem. */
> >>> + if (!(fs->super->s_state & EXT2_VALID_FS) ||
> >>> + (fs->super->s_state & EXT2_ERROR_FS) ||
> >>> + (fs->super->s_lastcheck < fs->super->s_mtime)) {
> >>> + printf("\n%s\n", _(please_fsck));
> >>> + if (mount_flags & EXT2_MF_READONLY)
> >>> + printf("%s", _("(and reboot afterwards!)\n"));
> >>> + exit(1);
> >>> + }
> >>
> >> Should this explicitly check for NEEDS_RECOVERY, or force journal replay
> >> directly? It would be a sad thing if the filesystem was modified and then
> >> journal replay clobbered it.
> >
> > I was under the impression that the patch "tune2fs: warn if the filesystem
> > journal is dirty" was sufficient to discourage journal-clobbering?
> >
> > AFAICT, that patch runs for any invocation of tune2fs, whereas
> > check_fsck_needed only applies to "dangerous" operations, i.e. the ones that
> > want to rewrite big chunks of FS.
>
> That check is fine as a warning for "simple" changes (e.g. setting a flag in
> the superblock), since if it gets clobbered when there _may_ be a superblock
> in the journal it isn't any worse than if it was never set at all.
>
> For "dangerous" options like these, if the filesystem is modified and then
> the journal is replayed (e.g resize inodes, replay journal with old inode
> table blocks on top of the modified inodes) the results would be disastrous.
>
> So for dangerous options it makes sense to either refuse to change the filesystem
> if the journal is dirty, or replay the journal before changing the filesystem
> if the proceed question is accepted.
I think it makes most sense always to replay the journal if it needs recovery.
No need to tell the admin they have to do something if we can do it for them.
--D
>
> It wouldn't be terrible to have a proceed question if the journal is dirty
> to see if they want to replay the journal before any field is modified, and
> just revert the warning patch.
>
> Cheers, Andreas
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tune2fs: confirm slow/dangerous operations
2015-12-04 23:36 ` Theodore Ts'o
@ 2015-12-05 1:21 ` Darrick J. Wong
0 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2015-12-05 1:21 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Andreas Dilger, linux-ext4
On Fri, Dec 04, 2015 at 06:36:16PM -0500, Theodore Ts'o wrote:
> On Fri, Dec 04, 2015 at 03:07:45PM -0800, Darrick J. Wong wrote:
> >
> > Ted took it, but I don't think he's pushed his tree to kernel.org recently.
>
> I pushed to kernel.org a few days ago, and that patch is there:
>
> http://git.kernel.org/cgit/fs/ext2/e2fsprogs.git/commit/?h=next&id=c5b3ae7fb5d58dd12a1e02c2443bad32c8a76150
Ah, fun! I'll rebase and send a rollup of the pending patches.
(checksum_seed, filefrag, and the two tune2fs things we've been talking about.)
--D
>
> - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-05 1:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-03 20:40 [PATCH] tune2fs: confirm slow/dangerous operations Darrick J. Wong
2015-12-04 22:12 ` Andreas Dilger
2015-12-04 23:07 ` Darrick J. Wong
2015-12-04 23:36 ` Theodore Ts'o
2015-12-05 1:21 ` Darrick J. Wong
2015-12-04 23:54 ` Andreas Dilger
2015-12-05 1:12 ` Darrick J. Wong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.