linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] e2fsck: don't flush the FS unless it's actually dirty
@ 2014-08-12 17:17 Darrick J. Wong
  2014-08-12 18:39 ` Theodore Ts'o
  0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2014-08-12 17:17 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Ext4 Developers List, Dan Jacobson

ext2fs_flush2() unconditionally writes the block group descriptors to
disk even if the underlying FS isn't marked dirty.  This causes the
following error message on a fsck -n run:

# e2fsck -fn /tmp/a
e2fsck 1.43-WIP (09-Jul-2014)
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
Error writing block 2 (Attempt to write block to filesystem resulted in short write).  Ignore error? no

Error writing block 2 (Attempt to write block to filesystem resulted in short write).  Ignore error? no

Error writing file system info: Attempt to write block to filesystem resulted in short write

Since ext2fs_close2() only calls flush if the dirty flag is set,
modify e2fsck to exhibit the same behavior so that we don't spit out
write errors for a read only check.

v2: Fix test to use $CRCSUM instead of md5sum.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 e2fsck/unix.c                  |    8 ++++---
 tests/f_readonly_fsck/expect   |   11 ++++++++++
 tests/f_readonly_fsck/image.gz |  Bin
 tests/f_readonly_fsck/name     |    1 +
 tests/f_readonly_fsck/script   |   46 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 3 deletions(-)
 create mode 100644 tests/f_readonly_fsck/expect
 create mode 100644 tests/f_readonly_fsck/image.gz
 create mode 100644 tests/f_readonly_fsck/name
 create mode 100644 tests/f_readonly_fsck/script

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 78bf3f4..6b0ca96 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1762,9 +1762,11 @@ no_journal:
 	}
 
 	e2fsck_write_bitmaps(ctx);
-	pctx.errcode = ext2fs_flush(ctx->fs);
-	if (pctx.errcode)
-		fix_problem(ctx, PR_6_FLUSH_FILESYSTEM, &pctx);
+	if (fs->flags & EXT2_FLAG_DIRTY) {
+		pctx.errcode = ext2fs_flush(ctx->fs);
+		if (pctx.errcode)
+			fix_problem(ctx, PR_6_FLUSH_FILESYSTEM, &pctx);
+	}
 	pctx.errcode = io_channel_flush(ctx->fs->io);
 	if (pctx.errcode)
 		fix_problem(ctx, PR_6_IO_FLUSH, &pctx);
diff --git a/tests/f_readonly_fsck/expect b/tests/f_readonly_fsck/expect
new file mode 100644
index 0000000..994dfa4
--- /dev/null
+++ b/tests/f_readonly_fsck/expect
@@ -0,0 +1,11 @@
+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 (4294968254, counted=958).
+Fix? no
+
+test_filesys: 11/256 files (9.1% non-contiguous), 18446744069414585410/2048 blocks
+Exit status is 0
+crc did not change.  2466215161
diff --git a/tests/f_readonly_fsck/image.gz b/tests/f_readonly_fsck/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..59f9cd91144b4b1978a59551f49e17f8740763e3
GIT binary patch
literal 2538
zcmb2|=3uD${UVr&`R!fXY!OEZ_7Cah{=)j5o>vaJaV!&T5nvKq)EX5LS$W?<CBlPg
zpUAQ<d55EJE^D@SO#LR5bJXdN(8dcKo~uNj-+6oV^y!6-0+VCv?0>w!_g2>W%^REG
zb1n?+Vbd;28_x8T@9vm-ZK=ImRA%g)$k-LGS6**Dx~6jFpL_d4o#RjCoO(LNzw*f9
zl79vtzux&ReSP2l+0r|{emVKseD`OA=IC`_*8TpI_3rei^Xb>^_x~$<_O`oPe3Q;@
zp}&>Ccdqhw4Y~H$KHs+L-mcFXrwUFby;>f1{9AicaeRvGzpELK{f=Hpem~d4a<<XM
zHUFoYT)WL&th%i3r6L0=xKaDHR_XTa<MY=ZR=gH*dzt;F8l6+6dB2p;A3t`=v_`Be
ztFHXlzvj=AFY_<Yo2|X@_LtmM|DP^eQ~zMX1~J(c^$vfTU&#mjZ}`Q2#eTs*#;^Qf
zA@)OmmUipkHg-sTn=CeW`?_~q`pcJzGd@UI`Tfu1@&(y585vQWk??;eFnrcNt>5<h
zahAz+nOnE&F5c<4ShT%zjb5tsG|g-stCCCE$?a+K%MVv9-BkJ~)o|0{>ic){ejAF+
zy`|s&@751}v!4HMxjkFXHES-`JgnL6>T)*w?zW%D{utlO{CD#4m-uJ%|4j+r#gAey
zI%k2lsAlipwf#$Ed|!4JU4Nadd3DM@jqkhVJS^p&);)F(d}q#wu4j}#8UmvsFd71*
ZAut*OqaiTRA#nGJx!?1P-xwGa7yyTBAU6O2

literal 0
HcmV?d00001

diff --git a/tests/f_readonly_fsck/name b/tests/f_readonly_fsck/name
new file mode 100644
index 0000000..97ab428
--- /dev/null
+++ b/tests/f_readonly_fsck/name
@@ -0,0 +1 @@
+ensure that a readonly check doesn't modify the fs
diff --git a/tests/f_readonly_fsck/script b/tests/f_readonly_fsck/script
new file mode 100644
index 0000000..d46c5a8
--- /dev/null
+++ b/tests/f_readonly_fsck/script
@@ -0,0 +1,46 @@
+FSCK_OPT=-fn
+OUT=$test_name.log
+if [ -f $test_dir/expect.gz ]; then
+	EXP=$test_name.tmp
+	gunzip < $test_dir/expect.gz > $EXP1
+else
+	EXP=$test_dir/expect
+fi
+
+cp /dev/null $OUT
+
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+
+gzip -d < $test_dir/image.gz > $TMPFILE
+
+old="$($CRCSUM < $TMPFILE)"
+
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE > $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" $OUT.new >> $OUT
+rm -f $OUT.new
+
+new="$($CRCSUM < $TMPFILE)"
+
+if [ "${old}" != "${new}" ]; then
+	echo "ERROR: crc mismatch!  ${old} ${new}" >> $OUT
+else
+	echo "crc did not change.  ${old}" >> $OUT
+fi
+
+rm -f $TMPFILE
+
+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
+	rm -f $test_name.tmp
+fi
+
+unset IMAGE FSCK_OPT OUT EXP old new

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] e2fsck: don't flush the FS unless it's actually dirty
  2014-08-12 17:17 [PATCH v2] e2fsck: don't flush the FS unless it's actually dirty Darrick J. Wong
@ 2014-08-12 18:39 ` Theodore Ts'o
  2014-08-12 18:44   ` [PATCH] tests: convert use of md5sum to crcsum Theodore Ts'o
  0 siblings, 1 reply; 3+ messages in thread
From: Theodore Ts'o @ 2014-08-12 18:39 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Ext4 Developers List, Dan Jacobson

On Tue, Aug 12, 2014 at 10:17:07AM -0700, Darrick J. Wong wrote:
> ext2fs_flush2() unconditionally writes the block group descriptors to
> disk even if the underlying FS isn't marked dirty.  This causes the
> following error message on a fsck -n run:...
> 
> v2: Fix test to use $CRCSUM instead of md5sum.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

As it turns out, I was wrong; there were other users of md5sum in the
tests that I had forgotten about.  Now that we have crcsum, it makes
sense to convert them over to use md5sum as well, so I'll do that.

Thanks again,

						- Ted

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] tests: convert use of md5sum to crcsum
  2014-08-12 18:39 ` Theodore Ts'o
@ 2014-08-12 18:44   ` Theodore Ts'o
  0 siblings, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2014-08-12 18:44 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: darrick.wong, Theodore Ts'o

The following tests were using md5sum: i_e2image, u_mke2fs, and
u_tune2fs.  Convert them to use crcsum for better portability (not all
environments have md5sum; some might have sha1sum instead :-)

For our purposes crcsum is quite sufficient.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/i_e2image/i_e2image.crc | 15 +++++++++++++++
 tests/i_e2image/i_e2image.md5 | 15 ---------------
 tests/i_e2image/script        | 24 ++++++++++++------------
 tests/u_mke2fs/script         | 14 +++++++-------
 tests/u_tune2fs/script        | 14 +++++++-------
 5 files changed, 41 insertions(+), 41 deletions(-)
 create mode 100644 tests/i_e2image/i_e2image.crc
 delete mode 100644 tests/i_e2image/i_e2image.md5

diff --git a/tests/i_e2image/i_e2image.crc b/tests/i_e2image/i_e2image.crc
new file mode 100644
index 0000000..02a8464
--- /dev/null
+++ b/tests/i_e2image/i_e2image.crc
@@ -0,0 +1,15 @@
+i_e2image/image1024.orig
+2161078647	image1024.orig
+467277198	_image.raw
+2164212449	_image.qcow2
+467277198	_image.qcow2.raw
+i_e2image/image2048.orig
+672740642	image2048.orig
+3688408350	_image.raw
+3821412753	_image.qcow2
+3688408350	_image.qcow2.raw
+i_e2image/image4096.orig
+4077552412	image4096.orig
+4159471388	_image.raw
+636354894	_image.qcow2
+4159471388	_image.qcow2.raw
diff --git a/tests/i_e2image/i_e2image.md5 b/tests/i_e2image/i_e2image.md5
deleted file mode 100644
index f96e721..0000000
--- a/tests/i_e2image/i_e2image.md5
+++ /dev/null
@@ -1,15 +0,0 @@
-i_e2image/image1024.orig
-d34914e0da07bdae80ab02288118fae2  image1024.orig
-bbef4e50d7237546c7d9c521d3df5b68  _image.raw
-1d4eb39452bed097dcd2c5bcd57180e6  _image.qcow2
-bbef4e50d7237546c7d9c521d3df5b68  _image.qcow2.raw
-i_e2image/image2048.orig
-aa9f702de181188f2a6d2c5158686c09  image2048.orig
-e6f8410d0690ef551bee0c2c0c642d8c  _image.raw
-dbbd9aa97c6c946b9122586bbd2a325a  _image.qcow2
-e6f8410d0690ef551bee0c2c0c642d8c  _image.qcow2.raw
-i_e2image/image4096.orig
-1d3e7f15b2ce9ca07aa23c32951c5176  image4096.orig
-734119dd8f240a33704139f8cdd8127c  _image.raw
-85fdbf5a8451b24b36ab82a02196deb9  _image.qcow2
-734119dd8f240a33704139f8cdd8127c  _image.qcow2.raw
diff --git a/tests/i_e2image/script b/tests/i_e2image/script
index adf59a4..c5b0666 100644
--- a/tests/i_e2image/script
+++ b/tests/i_e2image/script
@@ -7,42 +7,42 @@ RAW_IMG=_image.raw
 QCOW2_IMG=_image.qcow2
 QCOW2_TO_RAW=_image.qcow2.raw
 OUT=$test_name.log
-MD5=$SRCDIR/$test_name/$test_name.md5
-MD5_TMP=$test_name.md5.tmp
+CRC=$SRCDIR/$test_name/$test_name.crc
+CRC_TMP=$test_name.crc.tmp
 
-rm -f $test_name/_image.* $MD5_TMP $OUT >/dev/null 2>&1
+rm -f $test_name/_image.* $CRC_TMP $OUT >/dev/null 2>&1
 
 (
 for i in $ORIG_IMAGES; do
 	ORIG_IMG=$test_name/$i
-	echo $ORIG_IMG >> $MD5_TMP
+	echo $ORIG_IMG >> $CRC_TMP
 
 	bunzip2 < $SRCDIR/$ORIG_IMG.bz2 > $i
-	md5sum $i >> $MD5_TMP
+	echo "$($CRCSUM $i)	$i" >> $CRC_TMP
 
 	rm -f $RAW_IMG
 	echo "e2image -r $ORIG_IMG $RAW_IMG"
 	$E2IMAGE      -r $i $RAW_IMG
-	md5sum $RAW_IMG >> $MD5_TMP
+	echo "$($CRCSUM $RAW_IMG)	$RAW_IMG"  >> $CRC_TMP
 
 	echo "e2image -Q $ORIG_IMG $QCOW2_IMG"
 	$E2IMAGE      -Q $i $QCOW2_IMG
-	md5sum $QCOW2_IMG >> $MD5_TMP
+	echo "$($CRCSUM $QCOW2_IMG)	$QCOW2_IMG"  >> $CRC_TMP
 
 	rm -f $QCOW2_TO_RAW
 	echo "e2image -r $QCOW2_IMG $QCOW2_TO_RAW"
 	$E2IMAGE      -r $i $QCOW2_TO_RAW
-	md5sum $QCOW2_TO_RAW >> $MD5_TMP
+	echo "$($CRCSUM $QCOW2_TO_RAW)	$QCOW2_TO_RAW" >> $CRC_TMP
 
 	rm -f $i
 done
 ) >> $OUT 2>&1
 
-echo "md5sums:" >> $OUT
-cat $MD5_TMP >> $OUT
+echo "checksum:" >> $OUT
+cat $CRC_TMP >> $OUT
 echo "" >> $OUT
 
-diff $MD5 $MD5_TMP >> $OUT 2>&1
+diff $CRC $CRC_TMP >> $OUT 2>&1
 
 if [ $? -eq 0 ]; then
 	echo "$test_name: $test_description: ok"
@@ -52,7 +52,7 @@ else
 	echo "$test_name: $test_description: failed"
 fi
 
-rm -f _image.* $MD5_TMP >/dev/null 2>&1
+rm -f _image.* $CRC_TMP >/dev/null 2>&1
 
 else #if test -x $E2IMAGE_EXE; then
 	echo "$test_name: $test_description: skipped"
diff --git a/tests/u_mke2fs/script b/tests/u_mke2fs/script
index fcf5eae..d249ddd 100644
--- a/tests/u_mke2fs/script
+++ b/tests/u_mke2fs/script
@@ -11,19 +11,19 @@ dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
 
 echo mke2fs -q -F -o Linux -b 1024 test.img  > $OUT
 $MKE2FS -F -o Linux -I 128 -b 1024 $TMPFILE  >> $OUT 2>&1
-md5=`md5sum $TMPFILE | cut -d " " -f 1`
-echo md5sum before mke2fs $md5 >> $OUT
+crc=`$CRCSUM $TMPFILE`
+echo $CRCSUM before mke2fs $crc >> $OUT
 
 echo using mke2fs to test e2undo >> $OUT
 $MKE2FS -q -F -o Linux -I 256 -O uninit_bg -E lazy_itable_init=1 -b 1024 $TMPFILE  >> $OUT 2>&1
-new_md5=`md5sum $TMPFILE | cut -d " " -f 1`
-echo md5sum after mke2fs $new_md5 >> $OUT
+new_crc=`$CRCSUM $TMPFILE`
+echo $CRCSUM after mke2fs $new_crc >> $OUT
 
 $E2UNDO_EXE  $TDB_FILE $TMPFILE  >> $OUT 2>&1
-new_md5=`md5sum $TMPFILE | cut -d " " -f 1`
-echo md5sum after e2undo $new_md5 >> $OUT
+new_crc=`$CRCSUM $TMPFILE`
+echo $CRCSUM after e2undo $new_crc >> $OUT
 
-if [ $md5 = $new_md5 ]; then
+if [ $crc = $new_crc ]; then
 	echo "$test_name: $test_description: ok"
 	touch $test_name.ok
 else
diff --git a/tests/u_tune2fs/script b/tests/u_tune2fs/script
index 4cc1e0c..a443f5a 100644
--- a/tests/u_tune2fs/script
+++ b/tests/u_tune2fs/script
@@ -11,19 +11,19 @@ dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
 
 echo mke2fs -q -F -o Linux -b 1024 $TMPFILE  > $OUT
 $MKE2FS -q -F -o Linux -I 128 -b 1024 $TMPFILE  >> $OUT 2>&1
-md5=`md5sum $TMPFILE | cut -d " " -f 1`
-echo md5sum before tune2fs $md5 >> $OUT
+crc=`$CRCSUM $TMPFILE`
+echo $CRCSUM before tune2fs $crc >> $OUT
 
 echo using tune2fs to test e2undo >> $OUT
 $TUNE2FS -I 256 $TMPFILE  >> $OUT 2>&1
-new_md5=`md5sum $TMPFILE | cut -d " " -f 1`
-echo md5sum after tune2fs $new_md5 >> $OUT
+new_crc=`$CRCSUM $TMPFILE`
+echo $CRCSUM after tune2fs $new_crc >> $OUT
 
 $E2UNDO_EXE  $TDB_FILE $TMPFILE  >> $OUT 2>&1
-new_md5=`md5sum $TMPFILE | cut -d " " -f 1`
-echo md5sum after e2undo $new_md5 >> $OUT
+new_crc=`$CRCSUM $TMPFILE`
+echo $CRCSUM after e2undo $new_crc >> $OUT
 
-if [ $md5 = $new_md5 ]; then
+if [ $crc = $new_crc ]; then
 	echo "$test_name: $test_description: ok"
 	touch $test_name.ok
 else
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-08-12 18:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-12 17:17 [PATCH v2] e2fsck: don't flush the FS unless it's actually dirty Darrick J. Wong
2014-08-12 18:39 ` Theodore Ts'o
2014-08-12 18:44   ` [PATCH] tests: convert use of md5sum to crcsum Theodore Ts'o

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).