linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] e2fsprogs: fix data lost with mke2fs -S
@ 2011-12-22  8:30 Akira Fujita
  2012-02-17  4:20 ` Ted Ts'o
  0 siblings, 1 reply; 5+ messages in thread
From: Akira Fujita @ 2011-12-22  8:30 UTC (permalink / raw)
  To: Theodore Tso, ext4 development

If we run the mke2fs with the -S option and the uninit_bg feature
simultaneously, the mke2fs marks blockgroups as uninitialized.
The e2fsck which run immediately after the mke2fs
removes all of the files.

To avoid this, the patch prohibits user from
setting the -S option and the uninit_bg feature simultaneously.

Usage example of the mke2fsk -S is as follows:

1. mke2fs -t ext4 -S -O ^uninit_bg DEV
2. tune2fs -O uninit_bg DEV 
3. e2fsck DEV

#2 is not necessary only if the uninit_bg feature is not set
to ext4 originally. 

Signed-off-by: Akira Fujita <a-fujita@rs.jp.nec.com>
---
 misc/mke2fs.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 0ef2531..19f3684 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1676,6 +1676,16 @@ profile_error:
        if (tmp)
                free(tmp);

+       if (super_only == 1 &&
+           EXT2_HAS_RO_COMPAT_FEATURE((&fs_param),
+                                       EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
+               fprintf(stderr, _("The -S option and the uninit_bg feature "
+                                 "are not compatible.\n"
+                                 "They can not be set both "
+                                 "simultaneously.\n"));
+               exit(1);
+       }
+
        /*
         * We now need to do a sanity check of fs_blocks_count for
         * 32-bit vs 64-bit block number support.

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

* Re: [PATCH 1/2] e2fsprogs: fix data lost with mke2fs -S
  2011-12-22  8:30 [PATCH 1/2] e2fsprogs: fix data lost with mke2fs -S Akira Fujita
@ 2012-02-17  4:20 ` Ted Ts'o
  2012-02-21  7:09   ` Akira Fujita
  0 siblings, 1 reply; 5+ messages in thread
From: Ted Ts'o @ 2012-02-17  4:20 UTC (permalink / raw)
  To: Akira Fujita; +Cc: ext4 development

On Thu, Dec 22, 2011 at 05:30:22PM +0900, Akira Fujita wrote:
> If we run the mke2fs with the -S option and the uninit_bg feature
> simultaneously, the mke2fs marks blockgroups as uninitialized.
> The e2fsck which run immediately after the mke2fs
> removes all of the files.
> 
> To avoid this, the patch prohibits user from
> setting the -S option and the uninit_bg feature simultaneously.

This is not the best fix.  The best fix is to clear the itable_unused
fields in the block group descriptors if mke2fs -S is set.  See below
for what I have in my tree.

					- Ted

commit 9b6a158524fe82202bef6d0d8a101b47e6c02b64
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Thu Feb 16 23:16:34 2012 -0500

    mke2fs: allow file systems w/ uninit_bg to be recovered with mke2fs -S
    
    The command mke2fs -S is used as a last ditch recovery command to
    write new superblock and block group descriptors, but _not_ to destroy
    the inode table in hopes of recovering from a badly corrupted file
    system.  If the uninit_bg feature is enabled, we need to make sure to
    clear the unused inodes count field in the block group descriptors or
    else e2fsck -fy will leave the file system completely empty.
    
    Thanks to Akira Fujita for reporting this problem.
    
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 08789c4..c70c1b4 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -2434,6 +2434,17 @@ int main (int argc, char *argv[])
 	if (super_only) {
 		fs->super->s_state |= EXT2_ERROR_FS;
 		fs->flags &= ~(EXT2_FLAG_IB_DIRTY|EXT2_FLAG_BB_DIRTY);
+		/* 
+		 * The command "mke2fs -S" is used to recover
+		 * corrupted file systems, so do not mark any of the
+		 * inodes as unused; we want e2fsck to consider all
+		 * inodes as potentially containing recoverable data.
+		 */
+		if (fs->super->s_feature_ro_compat &
+		    EXT4_FEATURE_RO_COMPAT_GDT_CSUM) {
+			for (i = 1; i < fs->group_desc_count; i++)
+				ext2fs_bg_itable_unused_set(fs, i, 0);
+		}
 	} else {
 		/* rsv must be a power of two (64kB is MD RAID sb alignment) */
 		blk64_t rsv = 65536 / fs->blocksize;

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

* Re: [PATCH 1/2] e2fsprogs: fix data lost with mke2fs -S
  2012-02-17  4:20 ` Ted Ts'o
@ 2012-02-21  7:09   ` Akira Fujita
  2012-02-27  5:54     ` Ted Ts'o
  0 siblings, 1 reply; 5+ messages in thread
From: Akira Fujita @ 2012-02-21  7:09 UTC (permalink / raw)
  To: Ted Ts'o; +Cc: ext4 development

Hi Ted,

>      mke2fs: allow file systems w/ uninit_bg to be recovered with mke2fs -S
> 
>      The command mke2fs -S is used as a last ditch recovery command to
>      write new superblock and block group descriptors, but _not_ to destroy
>      the inode table in hopes of recovering from a badly corrupted file
>      system.  If the uninit_bg feature is enabled, we need to make sure to
>      clear the unused inodes count field in the block group descriptors or
>      else e2fsck -fy will leave the file system completely empty.
> 
>      Thanks to Akira Fujita for reporting this problem.

I tried to mkfs -S and e2fsck to salvage file
with e2fsprogs 1.42.1 (commit: 5ab348723247).  
But it seemed that the file was removed after e2fsck.
Could you check my method (below script)?

Am I missing something?

---
#!/bin/bash

DEV=""	#device
MP=""   #mount point

mke2fs -t ext4 $DEV
mount -t ext4 $DEV $MP

dd if=/dev/urandom of=$MP/FILE bs=4K count=100  #create file on ext4
ls -l $MP
umount $DEV

dd if=/dev/zero of=$DEV bs=4K count=1 #clear primary SB

mke2fs -t ext4 -S -b 4096 $DEV #write out SB
mount -t ext4 $DEV $MP #can't mount because of the gdp checksum difference

e2fsck $DEV #can salvage? 
mount -t ext4 $DEV $MP
ls -l $MP # It seems that "FILE" is removed
umount $DEV
---

Regards,
Akira Fujita

(2012/02/17 13:20), Ted Ts'o wrote:
> On Thu, Dec 22, 2011 at 05:30:22PM +0900, Akira Fujita wrote:
>> If we run the mke2fs with the -S option and the uninit_bg feature
>> simultaneously, the mke2fs marks blockgroups as uninitialized.
>> The e2fsck which run immediately after the mke2fs
>> removes all of the files.
>>
>> To avoid this, the patch prohibits user from
>> setting the -S option and the uninit_bg feature simultaneously.
> 
> This is not the best fix.  The best fix is to clear the itable_unused
> fields in the block group descriptors if mke2fs -S is set.  See below
> for what I have in my tree.
> 
> 					- Ted
> 
> commit 9b6a158524fe82202bef6d0d8a101b47e6c02b64
> Author: Theodore Ts'o<tytso@mit.edu>
> Date:   Thu Feb 16 23:16:34 2012 -0500
> 
>      mke2fs: allow file systems w/ uninit_bg to be recovered with mke2fs -S
> 
>      The command mke2fs -S is used as a last ditch recovery command to
>      write new superblock and block group descriptors, but _not_ to destroy
>      the inode table in hopes of recovering from a badly corrupted file
>      system.  If the uninit_bg feature is enabled, we need to make sure to
>      clear the unused inodes count field in the block group descriptors or
>      else e2fsck -fy will leave the file system completely empty.
> 
>      Thanks to Akira Fujita for reporting this problem.
> 
>      Signed-off-by: "Theodore Ts'o"<tytso@mit.edu>
> 
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 08789c4..c70c1b4 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -2434,6 +2434,17 @@ int main (int argc, char *argv[])
>   	if (super_only) {
>   		fs->super->s_state |= EXT2_ERROR_FS;
>   		fs->flags&= ~(EXT2_FLAG_IB_DIRTY|EXT2_FLAG_BB_DIRTY);
> +		/*
> +		 * The command "mke2fs -S" is used to recover
> +		 * corrupted file systems, so do not mark any of the
> +		 * inodes as unused; we want e2fsck to consider all
> +		 * inodes as potentially containing recoverable data.
> +		 */
> +		if (fs->super->s_feature_ro_compat&
> +		    EXT4_FEATURE_RO_COMPAT_GDT_CSUM) {
> +			for (i = 1; i<  fs->group_desc_count; i++)
> +				ext2fs_bg_itable_unused_set(fs, i, 0);
> +		}
>   	} else {
>   		/* rsv must be a power of two (64kB is MD RAID sb alignment) */
>   		blk64_t rsv = 65536 / fs->blocksize;
> --
> 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] 5+ messages in thread

* Re: [PATCH 1/2] e2fsprogs: fix data lost with mke2fs -S
  2012-02-21  7:09   ` Akira Fujita
@ 2012-02-27  5:54     ` Ted Ts'o
  2012-02-27  6:26       ` Akira Fujita
  0 siblings, 1 reply; 5+ messages in thread
From: Ted Ts'o @ 2012-02-27  5:54 UTC (permalink / raw)
  To: Akira Fujita; +Cc: ext4 development

On Tue, Feb 21, 2012 at 04:09:28PM +0900, Akira Fujita wrote:
> 
> I tried to mkfs -S and e2fsck to salvage file
> with e2fsprogs 1.42.1 (commit: 5ab348723247).  
> But it seemed that the file was removed after e2fsck.

Oops, sorry.  I made a stupid mistake in my mke2fs patch.  This should
fix things so that your script works correctly.

					- Ted

commit 30ac1ce7df719e40b0c3c612696ada7c9ebbaed2
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Mon Feb 27 00:51:39 2012 -0500

    mke2fs: make sure bg 0's unused inode count field is zero'ed for mke2fs -S
    
    There was a bug/typo in commit ba9e0afc5 which caused the first block
    group (bg #0) to not have its unused inode count field to get set to
    zero in the case of mke2fs -S.  This caused inodes in the first block
    group to not be recoverable via mke2fs -S.  Oops.
    
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index c70c1b4..51435d2 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -2442,7 +2442,7 @@ int main (int argc, char *argv[])
 		 */
 		if (fs->super->s_feature_ro_compat &
 		    EXT4_FEATURE_RO_COMPAT_GDT_CSUM) {
-			for (i = 1; i < fs->group_desc_count; i++)
+			for (i = 0; i < fs->group_desc_count; i++)
 				ext2fs_bg_itable_unused_set(fs, i, 0);
 		}
 	} else {

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

* Re: [PATCH 1/2] e2fsprogs: fix data lost with mke2fs -S
  2012-02-27  5:54     ` Ted Ts'o
@ 2012-02-27  6:26       ` Akira Fujita
  0 siblings, 0 replies; 5+ messages in thread
From: Akira Fujita @ 2012-02-27  6:26 UTC (permalink / raw)
  To: Ted Ts'o; +Cc: ext4 development

(2012/02/27 14:54), Ted Ts'o wrote:
> On Tue, Feb 21, 2012 at 04:09:28PM +0900, Akira Fujita wrote:
>>
>> I tried to mkfs -S and e2fsck to salvage file
>> with e2fsprogs 1.42.1 (commit: 5ab348723247).
>> But it seemed that the file was removed after e2fsck.
> 
> Oops, sorry.  I made a stupid mistake in my mke2fs patch.  This should
> fix things so that your script works correctly.

Worked fine, thank you for you work. :)

Akira Fujita
 
> 					- Ted
> 
> commit 30ac1ce7df719e40b0c3c612696ada7c9ebbaed2
> Author: Theodore Ts'o<tytso@mit.edu>
> Date:   Mon Feb 27 00:51:39 2012 -0500
> 
>      mke2fs: make sure bg 0's unused inode count field is zero'ed for mke2fs -S
> 
>      There was a bug/typo in commit ba9e0afc5 which caused the first block
>      group (bg #0) to not have its unused inode count field to get set to
>      zero in the case of mke2fs -S.  This caused inodes in the first block
>      group to not be recoverable via mke2fs -S.  Oops.
> 
>      Signed-off-by: "Theodore Ts'o"<tytso@mit.edu>
> 
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index c70c1b4..51435d2 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -2442,7 +2442,7 @@ int main (int argc, char *argv[])
>   		 */
>   		if (fs->super->s_feature_ro_compat&
>   		EXT4_FEATURE_RO_COMPAT_GDT_CSUM) {
> -			for (i = 1; i<  fs->group_desc_count; i++)
> +			for (i = 0; i<  fs->group_desc_count; i++)
>   				ext2fs_bg_itable_unused_set(fs, i, 0);
>   		}
>   	} else {
> --
> 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] 5+ messages in thread

end of thread, other threads:[~2012-02-27  6:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-22  8:30 [PATCH 1/2] e2fsprogs: fix data lost with mke2fs -S Akira Fujita
2012-02-17  4:20 ` Ted Ts'o
2012-02-21  7:09   ` Akira Fujita
2012-02-27  5:54     ` Ted Ts'o
2012-02-27  6:26       ` Akira Fujita

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