[PATCH 0/4] cleanup various mmp cruft & bugs

[PATCH 0/4] cleanup various mmp cruft & bugs

[Eric Sandeen]

Found several issues while working out mmp issues on s390, this patch series addresses them.

[PATCH 1/4] e2fsprogs: Document mmp commands in debugfs manpage

[Eric Sandeen]

Document the dump_mmp and set_mmp_value commands in
the debugfs manpage.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/debugfs/debugfs.8.in b/debugfs/debugfs.8.in
index fe07f34..d0d1938 100644
--- a/debugfs/debugfs.8.in
+++ b/debugfs/debugfs.8.in
@@ -210,6 +210,9 @@ option is given set the owner, group and permissions information on
 to match 
 .IR filespec .
 .TP
+.I dump_mmp
+Display the multiple-mount protection (mmp) field values.
+.TP
 .I dx_hash [-h hash_alg] [-s hash_seed] filename
 Calculate the directory hash of
 .IR filename .
@@ -485,6 +488,16 @@ The list of valid inode fields which can be set via this command
 can be displayed by using the command:
 .B set_inode_field -l
 .TP
+.I set_mmp_value field value
+Modify the multiple-mount protection (mmp) data so that the 
+mmp field
+.I field
+has value
+.I value.
+The list of valid mmp fields which can be set via this command
+can be displayed by using the command:
+.B set_mmp_value -l
+.TP
 .I set_super_value field value
 Set the superblock field
 .I field

[PATCH 2/4] e2fsprogs: Tidy up mmp handling in debugfs

[Eric Sandeen]

Several small fixes:

* Gracefully fail mmp commands if fs is not open
* Show magic number in dump_mmp command
* Fix header in output for set_mmp_value -l

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index eba3b69..1fb8f44 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -2165,11 +2165,15 @@ void do_punch(int argc, char *argv[])
 
 void do_dump_mmp(int argc, char *argv[])
 {
-	struct ext2_super_block *sb = current_fs->super;
+	struct ext2_super_block *sb;
 	struct mmp_struct *mmp_s;
 	time_t t;
 	errcode_t retval = 0;
 
+	if (check_fs_open(argv[0]))
+		return;
+
+	sb  = current_fs->super;
 	if (sb->s_mmp_block <= sb->s_first_data_block ||
 	    sb->s_mmp_block >= ext2fs_blocks_count(sb)) {
 		com_err(argv[0], EXT2_ET_MMP_BAD_BLOCK, "while dumping it.\n");
@@ -2203,6 +2207,7 @@ void do_dump_mmp(int argc, char *argv[])
 	fprintf(stdout, "time: %lld -- %s", mmp_s->mmp_time, ctime(&t));
 	fprintf(stdout, "node_name: %s\n", mmp_s->mmp_nodename);
 	fprintf(stdout, "device_name: %s\n", mmp_s->mmp_bdevname);
+	fprintf(stdout, "magic: 0x%x\n", mmp_s->mmp_magic);
 }
 
 static int source_file(const char *cmd_file, int sci_idx)
diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
index d461275..0041160 100644
--- a/debugfs/set_fields.c
+++ b/debugfs/set_fields.c
@@ -242,6 +242,9 @@ static struct field_set_info ext4_bg_fields[] = {
 	{ 0, 0, 0, 0 }
 };
 
+/* forward declaration */
+static struct field_set_info mmp_fields[];
+
 static int check_suffix(const char *field)
 {
 	int len = strlen(field);
@@ -561,6 +564,9 @@ static void print_possible_fields(struct field_set_info *fields)
 	} else if (fields == inode_fields) {
 		type = "Inode";
 		cmd = "set_inode";
+	} else if (fields == mmp_fields) {
+		type = "MMP";
+		cmd = "set_mmp_value";
 	} else {
 		type = "Block group descriptor";
 		cmd = "set_block_group";
@@ -768,6 +774,9 @@ void do_set_mmp_value(int argc, char *argv[])
 		return;
 	}
 
+	if (check_fs_open(argv[0]))
+		return;
+
 	if (current_fs->super->s_mmp_block == 0) {
 		com_err(argv[0], 0, "no MMP block allocated\n");
 		return;

[PATCH 3/4] e2fsprogs: Fix write size in ext2fs_mmp_write

[Eric Sandeen]

Without this change, we will write data past the end of the
mmp buf.  Valgrind catches this:

==6373== Syscall param write(buf) points to unaddressable byte(s)
==6373==    at 0x362260E470: __write_nocancel (in /lib64/libpthread-2.12.2.so)
==6373==    by 0x41CF83: raw_write_blk (unix_io.c:255)
==6373==    by 0x41D2BC: unix_write_blk64 (unix_io.c:757)
==6373==    by 0x41A05D: ext2fs_mmp_write (mmp.c:130)
==6373==    by 0x40B0C9: do_set_mmp_value (set_fields.c:806)
==6373==    by 0x421B61: really_execute_command (execute_cmd.c:108)
==6373==    by 0x421C54: ss_execute_line (execute_cmd.c:234)
==6373==    by 0x403743: main (debugfs.c:2339)
==6373==  Address 0x63f000 is not stack'd, malloc'd or (recently) free'd

and in my testing it led to silent failures while writing the mmp
block in debugfs:

write(3, "xV4\22PMM\342\325V\274N\0\0\0\0host.name."..., 4096) = -1 EFAULT (Bad address)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

p.s. I could do with a comment about what a negative "count" means...?

diff --git a/lib/ext2fs/mmp.c b/lib/ext2fs/mmp.c
index 91f4fb2..b27d9a4 100644
--- a/lib/ext2fs/mmp.c
+++ b/lib/ext2fs/mmp.c
@@ -127,7 +127,7 @@ errcode_t ext2fs_mmp_write(ext2_filsys fs, blk64_t mmp_blk, void *buf)
 
 	/* I was tempted to make this use O_DIRECT and the mmp_fd, but
 	 * this caused no end of grief, while leaving it as-is works. */
-	retval = io_channel_write_blk64(fs->io, mmp_blk, -fs->blocksize, buf);
+	retval = io_channel_write_blk64(fs->io, mmp_blk, -(int)sizeof(struct mmp_struct), buf);
 
 #ifdef WORDS_BIGENDIAN
 	ext2fs_swap_mmp(mmp_s);

[PATCH 4/4] e2fsprogs: fix mmp tests on hard 4k devices (resend)

[Eric Sandeen]

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

Several of the mmp tests were failing on an s390 guest because
direct IO cannot be done on sub-sector sizes, and they were
doing 1k IOs to files on a 4k logical/physical device.

A few tests could just be changed to 4k, but others
needed fixed up output as well.

With this I have all tests passing on s390 again.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

(attaching patch to deal with ctrl characters in patch)

[-- Attachment #2: mmp_test_fixes.patch --]
[-- Type: text/x-diff, Size: 7761 bytes --]

diff --git a/tests/f_mmp/script b/tests/f_mmp/script
index 548734a..4aca447 100644
--- a/tests/f_mmp/script
+++ b/tests/f_mmp/script
@@ -12,7 +12,7 @@ if [ $? == 0 ]; then
 fi
 
 echo "make the test image ..." > $test_name.log
-$MKE2FS -q -F -o Linux -b 1024 -O mmp -E mmp_update_interval=1 $TMPFILE 100 >> $test_name.log 2>&1
+$MKE2FS -q -F -o Linux -b 4096 -O mmp -E mmp_update_interval=1 $TMPFILE 100 >> $test_name.log 2>&1
 status=$?
 if [ "$status" != 0 ] ; then
 	echo "mke2fs -O mmp failed" > $test_name.failed
diff --git a/tests/f_mmp_garbage/expect.1 b/tests/f_mmp_garbage/expect.1
index 4ee5cfb..a8add10 100644
--- a/tests/f_mmp_garbage/expect.1
+++ b/tests/f_mmp_garbage/expect.1
@@ -5,5 +5,5 @@ Pass 2: Checking directory structure
 Pass 3: Checking directory connectivity
 Pass 4: Checking reference counts
 Pass 5: Checking group summary information
-test_filesys: 11/16 files (0.0% non-contiguous), 22/100 blocks
+test_filesys: 11/64 files (0.0% non-contiguous), 13/100 blocks
 Exit status is 0
diff --git a/tests/f_mmp_garbage/expect.2 b/tests/f_mmp_garbage/expect.2
index 3bf3869..6630002 100644
--- a/tests/f_mmp_garbage/expect.2
+++ b/tests/f_mmp_garbage/expect.2
@@ -3,5 +3,5 @@ Pass 2: Checking directory structure
 Pass 3: Checking directory connectivity
 Pass 4: Checking reference counts
 Pass 5: Checking group summary information
-test_filesys: 11/16 files (0.0% non-contiguous), 22/100 blocks
+test_filesys: 11/64 files (0.0% non-contiguous), 13/100 blocks
 Exit status is 0
diff --git a/tests/f_mmp_garbage/script b/tests/f_mmp_garbage/script
index 3c80032..8b5a22a 100644
--- a/tests/f_mmp_garbage/script
+++ b/tests/f_mmp_garbage/script
@@ -12,7 +12,7 @@ if [ $? == 0 ] ; then
 fi
 
 echo "make the test image ..." > $test_name.log
-$MKE2FS -q -F -o Linux -b 1024 -O mmp -E mmp_update_interval=1 $TMPFILE 100 >> $test_name.log 2>&1
+$MKE2FS -q -F -o Linux -b 4096 -O mmp -E mmp_update_interval=1 $TMPFILE 100 >> $test_name.log 2>&1
 status=$?
 if [ "$status" != 0 ] ; then
 	echo "mke2fs -O mmp failed" > $test_name.failed
diff --git a/tests/m_mmp/expect.1 b/tests/m_mmp/expect.1
index 3a00815..d3319b3 100644
--- a/tests/m_mmp/expect.1
+++ b/tests/m_mmp/expect.1
@@ -1,55 +1,55 @@
 Filesystem label=
 OS type: Linux
-Block size=2048 (log=1)
-Fragment size=2048 (log=1)
+Block size=4096 (log=2)
+Fragment size=4096 (log=2)
 Stride=0 blocks, Stripe width=0 blocks
-16384 inodes, 32768 blocks
-1638 blocks (5.00%) reserved for the super user
+65536 inodes, 65536 blocks
+3276 blocks (5.00%) reserved for the super user
 First data block=0
-Maximum filesystem blocks=33554432
+Maximum filesystem blocks=67108864
 2 block groups
-16384 blocks per group, 16384 fragments per group
-8192 inodes per group
+32768 blocks per group, 32768 fragments per group
+32768 inodes per group
 Superblock backups stored on blocks: 
-	16384
+	32768
 
 Allocating group tables: 0/2\b\b\b1/2\b\b\b   \b\b\bdone                            
 Writing inode tables: 0/2\b\b\b1/2\b\b\b   \b\b\bdone                            
 Multiple mount protection is enabled with update interval 5 seconds.
 Writing superblocks and filesystem accounting information: 0/2\b\b\b1/2\b\b\b   \b\b\bdone
 
-Filesystem features: ext_attr resize_inode dir_index filetype mmp sparse_super
+Filesystem features: ext_attr resize_inode dir_index filetype mmp sparse_super large_file
  
 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: 11/16384 files (0.0% non-contiguous), 1105/32768 blocks
+test_filesys: 11/65536 files (0.0% non-contiguous), 2093/65536 blocks
 Exit status is 0
 
 Filesystem volume name:   <none>
 Last mounted on:          <not available>
 Filesystem magic number:  0xEF53
 Filesystem revision #:    1 (dynamic)
-Filesystem features:      ext_attr resize_inode dir_index filetype mmp sparse_super
+Filesystem features:      ext_attr resize_inode dir_index filetype mmp sparse_super large_file
 Default mount options:    (none)
 Filesystem state:         clean
 Errors behavior:          Continue
 Filesystem OS type:       Linux
-Inode count:              16384
-Block count:              32768
-Reserved block count:     1638
-Free blocks:              31663
-Free inodes:              16373
+Inode count:              65536
+Block count:              65536
+Reserved block count:     3276
+Free blocks:              63443
+Free inodes:              65525
 First block:              0
-Block size:               2048
-Fragment size:            2048
-Reserved GDT blocks:      31
-Blocks per group:         16384
-Fragments per group:      16384
-Inodes per group:         8192
-Inode blocks per group:   512
+Block size:               4096
+Fragment size:            4096
+Reserved GDT blocks:      15
+Blocks per group:         32768
+Fragments per group:      32768
+Inodes per group:         32768
+Inode blocks per group:   1024
 Mount count:              0
 Check interval:           15552000 (6 months)
 Reserved blocks uid:      0
@@ -57,23 +57,23 @@ Reserved blocks gid:      0
 First inode:              11
 Inode size:	          128
 Default directory hash:   half_md4
-MMP block number:         557
+MMP block number:         1049
 MMP update interval:      5
 
 
-Group 0: (Blocks 0-16383)
+Group 0: (Blocks 0-32767)
   Primary superblock at 0, Group descriptors at 1-1
-  Reserved GDT blocks at 2-32
-  Block bitmap at 33 (+33), Inode bitmap at 34 (+34)
-  Inode table at 35-546 (+35)
-  15826 free blocks, 8181 free inodes, 2 directories
-  Free blocks: 558-16383
-  Free inodes: 12-8192
-Group 1: (Blocks 16384-32767)
-  Backup superblock at 16384, Group descriptors at 16385-16385
-  Reserved GDT blocks at 16386-16416
-  Block bitmap at 16417 (+33), Inode bitmap at 16418 (+34)
-  Inode table at 16419-16930 (+35)
-  15837 free blocks, 8192 free inodes, 0 directories
-  Free blocks: 16931-32767
-  Free inodes: 8193-16384
+  Reserved GDT blocks at 2-16
+  Block bitmap at 17 (+17), Inode bitmap at 18 (+18)
+  Inode table at 19-1042 (+19)
+  31718 free blocks, 32757 free inodes, 2 directories
+  Free blocks: 1050-32767
+  Free inodes: 12-32768
+Group 1: (Blocks 32768-65535)
+  Backup superblock at 32768, Group descriptors at 32769-32769
+  Reserved GDT blocks at 32770-32784
+  Block bitmap at 32785 (+17), Inode bitmap at 32786 (+18)
+  Inode table at 32787-33810 (+19)
+  31725 free blocks, 32768 free inodes, 0 directories
+  Free blocks: 33811-65535
+  Free inodes: 32769-65536
diff --git a/tests/m_mmp/script b/tests/m_mmp/script
index 1547463..b3e206a 100644
--- a/tests/m_mmp/script
+++ b/tests/m_mmp/script
@@ -10,6 +10,6 @@ if [ $? == 0 ]; then
 	echo "skipped for tmpfs (no O_DIRECT support)"
 	return 0
 fi
-MKE2FS_OPTS="-O mmp"
+MKE2FS_OPTS="-b 4096 -O mmp"
 . $cmd_dir/run_mke2fs
 unset MKE2FS_DEVICE_SECTSIZE
diff --git a/tests/t_mmp_1on/script b/tests/t_mmp_1on/script
index ee9884f..3b0a376 100644
--- a/tests/t_mmp_1on/script
+++ b/tests/t_mmp_1on/script
@@ -11,7 +11,7 @@ if [ $? == 0 ] ; then
 	return 0
 fi
 
-$MKE2FS -q -F -o Linux -b 1024 $TMPFILE 100 > $test_name.log 2>&1
+$MKE2FS -q -F -o Linux -b 4096 $TMPFILE 100 > $test_name.log 2>&1
 status=$?
 if [ "$status" != 0 ] ; then
 	echo "mke2fs failed" > $test_name.failed
diff --git a/tests/t_mmp_2off/script b/tests/t_mmp_2off/script
index ec9f71e..56c6bed 100644
--- a/tests/t_mmp_2off/script
+++ b/tests/t_mmp_2off/script
@@ -11,7 +11,7 @@ if [ $? == 0 ]; then
 	return 0
 fi
 
-$MKE2FS -q -F -o Linux -b 1024 -O mmp $TMPFILE 100 > $test_name.log 2>&1
+$MKE2FS -q -F -o Linux -b 4096 -O mmp $TMPFILE 100 > $test_name.log 2>&1
 status=$?
 if [ "$status" != 0 ] ; then
 	echo "mke2fs -O mmp failed" > $test_name.failed

Re: [PATCH 1/4] e2fsprogs: Document mmp commands in debugfs manpage

[Ted Ts'o]

On Thu, Nov 10, 2011 at 04:51:28PM -0600, Eric Sandeen wrote:
> Document the dump_mmp and set_mmp_value commands in
> the debugfs manpage.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Applied, thanks.

						- Ted

Re: [PATCH 2/4] e2fsprogs: Tidy up mmp handling in debugfs

[Ted Ts'o]

On Thu, Nov 10, 2011 at 04:53:22PM -0600, Eric Sandeen wrote:
> Several small fixes:
> 
> * Gracefully fail mmp commands if fs is not open
> * Show magic number in dump_mmp command
> * Fix header in output for set_mmp_value -l
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Applied, thanks.

						- Ted

Re: [PATCH 3/4] e2fsprogs: Fix write size in ext2fs_mmp_write

[Ted Ts'o]

On Thu, Nov 10, 2011 at 05:00:12PM -0600, Eric Sandeen wrote:
> Without this change, we will write data past the end of the
> mmp buf.  Valgrind catches this:
> 
> ==6373== Syscall param write(buf) points to unaddressable byte(s)
> ==6373==    at 0x362260E470: __write_nocancel (in /lib64/libpthread-2.12.2.so)
> ==6373==    by 0x41CF83: raw_write_blk (unix_io.c:255)
> ==6373==    by 0x41D2BC: unix_write_blk64 (unix_io.c:757)
> ==6373==    by 0x41A05D: ext2fs_mmp_write (mmp.c:130)
> ==6373==    by 0x40B0C9: do_set_mmp_value (set_fields.c:806)
> ==6373==    by 0x421B61: really_execute_command (execute_cmd.c:108)
> ==6373==    by 0x421C54: ss_execute_line (execute_cmd.c:234)
> ==6373==    by 0x403743: main (debugfs.c:2339)
> ==6373==  Address 0x63f000 is not stack'd, malloc'd or (recently) free'd
> 
> and in my testing it led to silent failures while writing the mmp
> block in debugfs:
> 
> write(3, "xV4\22PMM\342\325V\274N\0\0\0\0host.name."..., 4096) = -1 EFAULT (Bad address)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Applied, thanks.

						- Ted

Re: [PATCH 4/4] e2fsprogs: fix mmp tests on hard 4k devices (resend)

[Ted Ts'o]

On Thu, Nov 10, 2011 at 05:04:00PM -0600, Eric Sandeen wrote:
> Several of the mmp tests were failing on an s390 guest because
> direct IO cannot be done on sub-sector sizes, and they were
> doing 1k IOs to files on a 4k logical/physical device.
> 
> A few tests could just be changed to 4k, but others
> needed fixed up output as well.
> 
> With this I have all tests passing on s390 again.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Applied, thanks.

						- Ted