Skip to contentSkip to conversation

cleanup various mmp cruft & bugs

PATCH Applied Eric Sandeen 2 participants 9 messages Nov 10, 2011 – Nov 12, 2011

lore
Share & tools
lore
#0 lore raw

cover cleanup various mmp cruft & bugs

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

#1 lore raw

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

Document the dump_mmp and set_mmp_value commands in the debugfs manpage.

---

debugfs/debugfs.8.in +13 -0
index fe07f34..d0d1938 100644
@@ -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
Applied
#2 lore raw

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

Quoted 4 lines · Eric Sandeen

Document the dump_mmp and set_mmp_value commands in the debugfs manpage.

Applied, thanks.

- Ted

#3 lore raw

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

In reply to 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

---

debugfs/debugfs.c +6 -1
index eba3b69..1fb8f44 100644
@@ -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)
debugfs/set_fields.c +9 -0
index d461275..0041160 100644
@@ -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;
Applied
#4 lore raw

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

Quoted 7 lines · 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

Applied, thanks.

- Ted

#5 lore raw

PATCH 3/4 e2fsprogs: Fix write size in ext2fs_mmp_write

In reply to 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)

---

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

lib/ext2fs/mmp.c +1 -1
index 91f4fb2..b27d9a4 100644
@@ -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);
Applied
#6 lore raw

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

Quoted 12 lines · 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)
Quoted 8 lines

==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)

Applied, thanks.

- Ted

#7 lore raw

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

In reply to Eric Sandeen

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.

---

(attaching patch to deal with ctrl characters in patch)

tests/f_mmp/script +1 -1
index 548734a..4aca447 100644
@@ -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
tests/f_mmp_garbage/expect.1 +1 -1
index 4ee5cfb..a8add10 100644
@@ -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
tests/f_mmp_garbage/expect.2 +1 -1
index 3bf3869..6630002 100644
@@ -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
tests/f_mmp_garbage/script +1 -1
index 3c80032..8b5a22a 100644
@@ -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
tests/m_mmp/expect.1 +39 -39

117 lines not on this page · raw message

tests/m_mmp/script +1 -1
index 1547463..b3e206a 100644
@@ -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
tests/t_mmp_1on/script +1 -1
index ee9884f..3b0a376 100644
@@ -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
tests/t_mmp_2off/script +1 -1
index ec9f71e..56c6bed 100644
@@ -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
Applied
#8 lore raw

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

Quoted 10 lines · Eric Sandeen

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.

Applied, thanks.

- Ted