2014-11-12 09:52:40

by Xiaoguang Wang

[permalink] [raw]
Subject: [PATCH v2 1/3] e2fsprogs/tune2fs: fix memory leak in inode_scan_and_fix()

When we use ext2fs_open_inode_scan() to iterate inodes and finish
jobs, we also need a ext2fs_close_inode_scan(scan) operation, but in
inode_scan_and_fix(), we forgot to call it, fix this error.

Signed-off-by: Xiaoguang Wang <[email protected]>
---
misc/tune2fs.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 7fee870..065b483 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2147,6 +2147,7 @@ static int inode_scan_and_fix(ext2_filsys fs, ext2fs_block_bitmap bmap)

err_out:
ext2fs_free_mem(&block_buf);
+ ext2fs_close_inode_scan(scan);

return retval;
}
--
1.8.2.1



2014-11-12 09:52:43

by Xiaoguang Wang

[permalink] [raw]
Subject: [PATCH v2 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
obviously, we need to re-compute the group descriptor checksums, fix this. If
not doing this, mount operation will fail.

Meanwhile, the patch will trigger an existing memory write overflow, which will
casue segfault, please see the next patch.

Signed-off-by: Xiaoguang Wang <[email protected]>
---
misc/tune2fs.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 065b483..91dc7c1 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2908,8 +2908,7 @@ retry_open:
EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
rewrite_checksums = 1;
}
- if (rewrite_checksums)
- rewrite_metadata_checksums(fs);
+
if (I_flag) {
if (mount_flags & EXT2_MF_MOUNTED) {
fputs(_("The inode size may only be "
@@ -2935,6 +2934,7 @@ retry_open:
if (resize_inode(fs, new_inode_size) == 0) {
printf(_("Setting inode size %lu\n"),
new_inode_size);
+ rewrite_checksums = 1;
} else {
printf("%s", _("Failed to change inode size\n"));
rc = 1;
@@ -2942,6 +2942,9 @@ retry_open:
}
}

+ if (rewrite_checksums)
+ rewrite_metadata_checksums(fs);
+
if (l_flag)
list_super(sb);
if (stride_set) {
--
1.8.2.1


2014-11-12 09:53:09

by Xiaoguang Wang

[permalink] [raw]
Subject: [PATCH v2 3/3] e2fsprogs/tune2fs: fix memory write overflow

If we apply this patch 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size',
we will trigger a segfault, this is because of the inode cache issues.

Firstly we should notice that in expand_inode_table(), we have change the super block's s_inode_size
to new inode size(for example, 256).

Then we re-compute metadata checksums, see below code flow:
|-->rewrite_metadata_checksums
|----->rewrite_inodes
|-------->ext2fs_write_inode_full
In ext2fs_write_inode_full(), if an inode cache is hit, the below code will be executed:
/* Check to see if the inode cache needs to be updated */
if (fs->icache) {
for (i=0; i < fs->icache->cache_size; i++) {
if (fs->icache->cache[i].ino == ino) {
memcpy(fs->icache->cache[i].inode, inode,
(bufsize > length) ? length : bufsize);
break;
}
}
}

Before executing rewrite_inodes(), actually the inode in inode cache is allocated by
old inode size(for example, 128), but here the memcpy will obviously write overflow,
'(bufsize > length) ? length : bufsize' here will return 256(new inode size), so this is
wrong, we need to fix this.
I think we should call ext2fs_free_inode_cache() in expand_inode_table(), to drop the
inode cache, because inode size has changed, if necessary, we will re-create this inode cache.

Steps to reproduce this bug(apply 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size' first).
dd if=/dev/zero of=file.img bs=1M count=128
device_name=$(/sbin/losetup -f)
/sbin/losetup -f file.img
mkfs.ext4 -I 128 -O ^flex_bg $device_name
tune2fs -I 256 $device_name

Signed-off-by: Xiaoguang Wang <[email protected]>
---
misc/tune2fs.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 91dc7c1..34d2cd9 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2244,6 +2244,8 @@ static int expand_inode_table(ext2_filsys fs, unsigned long new_ino_size)

/* Update the meta data */
fs->inode_blocks_per_group = new_ino_blks_per_grp;
+ ext2fs_free_inode_cache(fs->icache);
+ fs->icache = 0;
fs->super->s_inode_size = new_ino_size;

err_out:
--
1.8.2.1


2014-11-12 22:40:08

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

On Wed, Nov 12, 2014 at 05:49:39PM +0800, Xiaoguang Wang wrote:
> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
> obviously, we need to re-compute the group descriptor checksums, fix this. If
> not doing this, mount operation will fail.
>
> Meanwhile, the patch will trigger an existing memory write overflow, which will
> casue segfault, please see the next patch.
>
> Signed-off-by: Xiaoguang Wang <[email protected]>
> ---
> misc/tune2fs.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 065b483..91dc7c1 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2908,8 +2908,7 @@ retry_open:
> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
> rewrite_checksums = 1;
> }
> - if (rewrite_checksums)
> - rewrite_metadata_checksums(fs);
> +
> if (I_flag) {
> if (mount_flags & EXT2_MF_MOUNTED) {
> fputs(_("The inode size may only be "
> @@ -2935,6 +2934,7 @@ retry_open:
> if (resize_inode(fs, new_inode_size) == 0) {
> printf(_("Setting inode size %lu\n"),
> new_inode_size);
> + rewrite_checksums = 1;
> } else {
> printf("%s", _("Failed to change inode size\n"));
> rc = 1;
> @@ -2942,6 +2942,9 @@ retry_open:
> }
> }
>
> + if (rewrite_checksums)
> + rewrite_metadata_checksums(fs);
> +

Aha! expand_inode_table() fails to recompute the checksums of the inode blocks
it's moving around, and happily your change takes care of recomputing the inode
checksums for a metadata_csum FS. The changelog for this patch doesn't mention
this, but it should.

There ought to be a regression test for this. Can you send one along, please?

I crafted my own test case for the metadata_csum failure while trying to figure
out what this patchset does, so I'll simply send it out. Unfortunately, the
test requires a fix for a bug in the hugefile code that I'll cram onto the
patchbomb.

Other than that, you can add for all three patches:
Reviewed-by: Darrick J. Wong <[email protected]>

--D

> if (l_flag)
> list_super(sb);
> if (stride_set) {
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-14 09:00:27

by Xiaoguang Wang

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

Hi,
On 11/13/2014 06:39 AM, Darrick J. Wong wrote:
> On Wed, Nov 12, 2014 at 05:49:39PM +0800, Xiaoguang Wang wrote:
>> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
>> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
>> obviously, we need to re-compute the group descriptor checksums, fix this. If
>> not doing this, mount operation will fail.
>>
>> Meanwhile, the patch will trigger an existing memory write overflow, which will
>> casue segfault, please see the next patch.
>>
>> Signed-off-by: Xiaoguang Wang <[email protected]>
>> ---
>> misc/tune2fs.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 065b483..91dc7c1 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -2908,8 +2908,7 @@ retry_open:
>> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
>> rewrite_checksums = 1;
>> }
>> - if (rewrite_checksums)
>> - rewrite_metadata_checksums(fs);
>> +
>> if (I_flag) {
>> if (mount_flags & EXT2_MF_MOUNTED) {
>> fputs(_("The inode size may only be "
>> @@ -2935,6 +2934,7 @@ retry_open:
>> if (resize_inode(fs, new_inode_size) == 0) {
>> printf(_("Setting inode size %lu\n"),
>> new_inode_size);
>> + rewrite_checksums = 1;
>> } else {
>> printf("%s", _("Failed to change inode size\n"));
>> rc = 1;
>> @@ -2942,6 +2942,9 @@ retry_open:
>> }
>> }
>>
>> + if (rewrite_checksums)
>> + rewrite_metadata_checksums(fs);
>> +
>
> Aha! expand_inode_table() fails to recompute the checksums of the inode blocks
> it's moving around, and happily your change takes care of recomputing the inode
> checksums for a metadata_csum FS. The changelog for this patch doesn't mention
> this, but it should.
Oh, I had not realized the inode checksum, thanks for pointing this.
So we agree to call a rewrite_metadata_checksums() here, in new version patch, i'll
update the changelog :)

>
> There ought to be a regression test for this. Can you send one along, please?
Sure, I spent some time about how to write a test, will send this test soon.

Regards,
Xiaoguang Wang
>
> I crafted my own test case for the metadata_csum failure while trying to figure
> out what this patchset does, so I'll simply send it out. Unfortunately, the
> test requires a fix for a bug in the hugefile code that I'll cram onto the
> patchbomb.
>
> Other than that, you can add for all three patches:
> Reviewed-by: Darrick J. Wong <[email protected]>
>
> --D
>
>> if (l_flag)
>> list_super(sb);
>> if (stride_set) {
>> --
>> 1.8.2.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> .
>


2014-11-14 09:18:37

by Xiaoguang Wang

[permalink] [raw]
Subject: [PATCH v3 1/3] e2fsprogs/tune2fs: fix memory leak in inode_scan_and_fix()

When we use ext2fs_open_inode_scan() to iterate inodes and finish
jobs, we also need a ext2fs_close_inode_scan(scan) operation, but in
inode_scan_and_fix(), we forgot to call it, fix this error.

Signed-off-by: Xiaoguang Wang <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
misc/tune2fs.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 7fee870..065b483 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2147,6 +2147,7 @@ static int inode_scan_and_fix(ext2_filsys fs, ext2fs_block_bitmap bmap)

err_out:
ext2fs_free_mem(&block_buf);
+ ext2fs_close_inode_scan(scan);

return retval;
}
--
1.8.2.1


2014-11-14 09:18:43

by Xiaoguang Wang

[permalink] [raw]
Subject: [PATCH v3 3/3] e2fsprogs/tune2fs: fix memory write overflow

If we apply this patch 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size',
we will trigger a segfault, this is because of the inode cache issues.

Firstly we should notice that in expand_inode_table(), we have change the super block's s_inode_size
to new inode size(for example, 256).

Then we re-compute metadata checksums, see below code flow:
|-->rewrite_metadata_checksums
|----->rewrite_inodes
|-------->ext2fs_write_inode_full
In ext2fs_write_inode_full(), if an inode cache is hit, the below code will be executed:
/* Check to see if the inode cache needs to be updated */
if (fs->icache) {
for (i=0; i < fs->icache->cache_size; i++) {
if (fs->icache->cache[i].ino == ino) {
memcpy(fs->icache->cache[i].inode, inode,
(bufsize > length) ? length : bufsize);
break;
}
}
}

Before executing rewrite_inodes(), actually the inode in inode cache is allocated by
old inode size(for example, 128), but here the memcpy will obviously write overflow,
'(bufsize > length) ? length : bufsize' here will return 256(new inode size), so this is
wrong, we need to fix this.
I think we should call ext2fs_free_inode_cache() in expand_inode_table(), to drop the
inode cache, because inode size has changed, if necessary, we will re-create this inode cache.

Steps to reproduce this bug(apply 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size' first).
dd if=/dev/zero of=file.img bs=1M count=128
device_name=$(/sbin/losetup -f)
/sbin/losetup -f file.img
mkfs.ext4 -I 128 -O ^flex_bg $device_name
tune2fs -I 256 $device_name

Signed-off-by: Xiaoguang Wang <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
misc/tune2fs.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 91dc7c1..34d2cd9 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2244,6 +2244,8 @@ static int expand_inode_table(ext2_filsys fs, unsigned long new_ino_size)

/* Update the meta data */
fs->inode_blocks_per_group = new_ino_blks_per_grp;
+ ext2fs_free_inode_cache(fs->icache);
+ fs->icache = 0;
fs->super->s_inode_size = new_ino_size;

err_out:
--
1.8.2.1


2014-11-14 09:18:40

by Xiaoguang Wang

[permalink] [raw]
Subject: [PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
obviously, we need to re-compute the group descriptor checksums, and the inode
's size has also changed, we also need to recompute the checksums of inodes for
metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
this will fix checksum issues.

Meanwhile, the patch will trigger an existing memory write overflow, which will
casue segfault, please see the next patch.

Signed-off-by: Xiaoguang Wang <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
misc/tune2fs.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 065b483..91dc7c1 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2908,8 +2908,7 @@ retry_open:
EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
rewrite_checksums = 1;
}
- if (rewrite_checksums)
- rewrite_metadata_checksums(fs);
+
if (I_flag) {
if (mount_flags & EXT2_MF_MOUNTED) {
fputs(_("The inode size may only be "
@@ -2935,6 +2934,7 @@ retry_open:
if (resize_inode(fs, new_inode_size) == 0) {
printf(_("Setting inode size %lu\n"),
new_inode_size);
+ rewrite_checksums = 1;
} else {
printf("%s", _("Failed to change inode size\n"));
rc = 1;
@@ -2942,6 +2942,9 @@ retry_open:
}
}

+ if (rewrite_checksums)
+ rewrite_metadata_checksums(fs);
+
if (l_flag)
list_super(sb);
if (stride_set) {
--
1.8.2.1


2014-11-14 18:58:26

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] e2fsprogs/tune2fs: fix memory write overflow

On Fri, Nov 14, 2014 at 05:15:37PM +0800, Xiaoguang Wang wrote:
> If we apply this patch 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size',
> we will trigger a segfault, this is because of the inode cache issues.
>
> Firstly we should notice that in expand_inode_table(), we have change the super block's s_inode_size
> to new inode size(for example, 256).
>
> Then we re-compute metadata checksums, see below code flow:
> |-->rewrite_metadata_checksums
> |----->rewrite_inodes
> |-------->ext2fs_write_inode_full
> In ext2fs_write_inode_full(), if an inode cache is hit, the below code will be executed:
> /* Check to see if the inode cache needs to be updated */
> if (fs->icache) {
> for (i=0; i < fs->icache->cache_size; i++) {
> if (fs->icache->cache[i].ino == ino) {
> memcpy(fs->icache->cache[i].inode, inode,
> (bufsize > length) ? length : bufsize);
> break;
> }
> }
> }
>
> Before executing rewrite_inodes(), actually the inode in inode cache is allocated by
> old inode size(for example, 128), but here the memcpy will obviously write overflow,
> '(bufsize > length) ? length : bufsize' here will return 256(new inode size), so this is
> wrong, we need to fix this.
> I think we should call ext2fs_free_inode_cache() in expand_inode_table(), to drop the
> inode cache, because inode size has changed, if necessary, we will re-create this inode cache.
>
> Steps to reproduce this bug(apply 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size' first).
> dd if=/dev/zero of=file.img bs=1M count=128
> device_name=$(/sbin/losetup -f)
> /sbin/losetup -f file.img
> mkfs.ext4 -I 128 -O ^flex_bg $device_name
> tune2fs -I 256 $device_name
>
> Signed-off-by: Xiaoguang Wang <[email protected]>
> Reviewed-by: Darrick J. Wong <[email protected]>
> ---
> misc/tune2fs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 91dc7c1..34d2cd9 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2244,6 +2244,8 @@ static int expand_inode_table(ext2_filsys fs, unsigned long new_ino_size)
>
> /* Update the meta data */
> fs->inode_blocks_per_group = new_ino_blks_per_grp;
> + ext2fs_free_inode_cache(fs->icache);
> + fs->icache = 0;

By the way (and this is a general question, not one specific to just this
patch), why do some parts of e2fsprogs assign 0 (instead of NULL) to pointers?

--D

> fs->super->s_inode_size = new_ino_size;
>
> err_out:
> --
> 1.8.2.1
>

2014-11-14 23:10:18

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

On Fri, Nov 14, 2014 at 05:15:36PM +0800, Xiaoguang Wang wrote:
> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
> obviously, we need to re-compute the group descriptor checksums, and the inode
> 's size has also changed, we also need to recompute the checksums of inodes for
> metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
> this will fix checksum issues.
>
> Meanwhile, the patch will trigger an existing memory write overflow, which will
> casue segfault, please see the next patch.
>
> Signed-off-by: Xiaoguang Wang <[email protected]>
> Reviewed-by: Darrick J. Wong <[email protected]>
> ---
> misc/tune2fs.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 065b483..91dc7c1 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2908,8 +2908,7 @@ retry_open:
> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
> rewrite_checksums = 1;
> }
> - if (rewrite_checksums)
> - rewrite_metadata_checksums(fs);
> +
> if (I_flag) {
> if (mount_flags & EXT2_MF_MOUNTED) {
> fputs(_("The inode size may only be "
> @@ -2935,6 +2934,7 @@ retry_open:
> if (resize_inode(fs, new_inode_size) == 0) {

Come to think of it, if we're enabling metadata_csum /and/ resizing the inode,
we have to set EXT2_FLAG_IGNORE_CSUM_ERRORS before calling resize_inode. If we
don't, the library calls that resize_inode() makes will try to verify the
checksums (which haven't been set yet) and the operation will fail.

Will send patch, but at this point I'm wondering if it doesn't make more sense
for me to incorporate them into my patchbomb rather than let this series get
lost in the blizzard...

--D

> printf(_("Setting inode size %lu\n"),
> new_inode_size);
> + rewrite_checksums = 1;
> } else {
> printf("%s", _("Failed to change inode size\n"));
> rc = 1;
> @@ -2942,6 +2942,9 @@ retry_open:
> }
> }
>
> + if (rewrite_checksums)
> + rewrite_metadata_checksums(fs);
> +
> if (l_flag)
> list_super(sb);
> if (stride_set) {
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-17 01:58:55

by Xiaoguang Wang

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

Hi,

On 11/15/2014 07:10 AM, Darrick J. Wong wrote:
> On Fri, Nov 14, 2014 at 05:15:36PM +0800, Xiaoguang Wang wrote:
>> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
>> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
>> obviously, we need to re-compute the group descriptor checksums, and the inode
>> 's size has also changed, we also need to recompute the checksums of inodes for
>> metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
>> this will fix checksum issues.
>>
>> Meanwhile, the patch will trigger an existing memory write overflow, which will
>> casue segfault, please see the next patch.
>>
>> Signed-off-by: Xiaoguang Wang <[email protected]>
>> Reviewed-by: Darrick J. Wong <[email protected]>
>> ---
>> misc/tune2fs.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 065b483..91dc7c1 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -2908,8 +2908,7 @@ retry_open:
>> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
>> rewrite_checksums = 1;
>> }
>> - if (rewrite_checksums)
>> - rewrite_metadata_checksums(fs);
>> +
>> if (I_flag) {
>> if (mount_flags & EXT2_MF_MOUNTED) {
>> fputs(_("The inode size may only be "
>> @@ -2935,6 +2934,7 @@ retry_open:
>> if (resize_inode(fs, new_inode_size) == 0) {
>
> Come to think of it, if we're enabling metadata_csum /and/ resizing the inode,
> we have to set EXT2_FLAG_IGNORE_CSUM_ERRORS before calling resize_inode. If we
> don't, the library calls that resize_inode() makes will try to verify the
> checksums (which haven't been set yet) and the operation will fail.

Aha, right, sorry!
>
> Will send patch, but at this point I'm wondering if it doesn't make more sense
> for me to incorporate them into my patchbomb rather than let this series get
> lost in the blizzard...

OK, it would be better for you to incorporate them into your patchbomb, thanks!
Also I attached one filefrag's bug fix, please incorporate it too :)

Regards,
Xiaoguang Wang
>
> --D
>
>> printf(_("Setting inode size %lu\n"),
>> new_inode_size);
>> + rewrite_checksums = 1;
>> } else {
>> printf("%s", _("Failed to change inode size\n"));
>> rc = 1;
>> @@ -2942,6 +2942,9 @@ retry_open:
>> }
>> }
>>
>> + if (rewrite_checksums)
>> + rewrite_metadata_checksums(fs);
>> +
>> if (l_flag)
>> list_super(sb);
>> if (stride_set) {
>> --
>> 1.8.2.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> .
>


Attachments:
0001-filefrag-fix-wrong-extent-count-calculation-when-usi.patch (4.15 kB)

2014-11-17 19:10:50

by Darrick J. Wong

[permalink] [raw]
Subject: [PATCH] tune2fs: disable csum verification before resizing inode

When we're turning on metadata checksumming /and/ resizing the inode
at the same time, disable checksum verification during the
resize_inode() call because the subroutines it calls will try to
verify the checksums (which have not yet been set), causing the
operation to fail unnecessarily.

Signed-off-by: Darrick J. Wong <[email protected]>
---
misc/tune2fs.c | 7 +++-
tests/t_iexpand_mcsum/expect | 57 ++++++++++++++++++++++++++++++
tests/t_iexpand_mcsum/name | 1 +
tests/t_iexpand_mcsum/script | 80 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 144 insertions(+), 1 deletion(-)
create mode 100644 tests/t_iexpand_mcsum/expect
create mode 100644 tests/t_iexpand_mcsum/name
create mode 100644 tests/t_iexpand_mcsum/script

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index b510c49..f01b05b 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2961,8 +2961,13 @@ retry_open:
* We want to update group descriptor also
* with the new free inode count
*/
+ if (rewrite_checksums)
+ fs->flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
- if (resize_inode(fs, new_inode_size) == 0) {
+ retval = resize_inode(fs, new_inode_size);
+ if (rewrite_checksums)
+ fs->flags &= ~EXT2_FLAG_IGNORE_CSUM_ERRORS;
+ if (retval == 0) {
printf(_("Setting inode size %lu\n"),
new_inode_size);
rewrite_checksums = 1;
diff --git a/tests/t_iexpand_mcsum/expect b/tests/t_iexpand_mcsum/expect
new file mode 100644
index 0000000..2bebc9f
--- /dev/null
+++ b/tests/t_iexpand_mcsum/expect
@@ -0,0 +1,57 @@
+tune2fs test
+Creating filesystem with 786432 1k blocks and 98304 inodes
+Superblock backups stored on blocks:
+ 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409, 663553
+
+Allocating group tables: done
+Writing inode tables: done
+Creating journal (16384 blocks): done
+Creating 6334 huge file(s) with 117 blocks each: done
+Writing superblocks and filesystem accounting information: done
+
+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 -I 256 -O metadata_csum test.img
+Setting inode size 256
+
+Please run e2fsck -D on the filesystem.
+
+Exit status is 0
+Backing up journal inode block information.
+
+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
+
+
+
+Change in FS metadata:
+@@ -5 +5 @@
+-Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
++Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
+@@ -13 +13 @@
+-Free blocks: 16280
++Free blocks: 3958
+@@ -22 +22 @@
+-Inode blocks per group: 128
++Inode blocks per group: 256
+@@ -28 +28 @@
+-Inode size: 128
++Inode size: 256
+@@ -31,0 +32 @@
++Checksum type: crc32c
+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_iexpand_mcsum/name b/tests/t_iexpand_mcsum/name
new file mode 100644
index 0000000..e767715
--- /dev/null
+++ b/tests/t_iexpand_mcsum/name
@@ -0,0 +1 @@
+expand inodes and turn on metadata_csum
diff --git a/tests/t_iexpand_mcsum/script b/tests/t_iexpand_mcsum/script
new file mode 100644
index 0000000..cb424ed
--- /dev/null
+++ b/tests/t_iexpand_mcsum/script
@@ -0,0 +1,80 @@
+if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
+
+FSCK_OPT=-fn
+OUT=$test_name.log
+EXP=$test_dir/expect
+CONF=$TMPFILE.conf
+
+#gzip -d < $EXP.gz > $EXP
+
+cat > $CONF << ENDL
+[fs_types]
+ ext4h = {
+ features = has_journal,extent,huge_file,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,^resize_inode,^meta_bg,^flex_bg,^metadata_csum,64bit
+ blocksize = 1024
+ inode_size = 256
+ make_hugefiles = true
+ hugefiles_dir = /
+ hugefiles_slack = 16000K
+ hugefiles_name = aaaaa
+ hugefiles_digits = 4
+ hugefiles_size = 117K
+ zero_hugefiles = false
+ }
+ENDL
+
+echo "tune2fs test" > $OUT
+
+MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -I 128 $TMPFILE 786432 >> $OUT 2>&1
+rm -rf $CONF
+
+# dump and check
+($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.before 2> /dev/null
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# convert it
+echo "tune2fs -I 256 -O metadata_csum test.img" >> $OUT
+dd if=/dev/zero of=$TMPFILE conv=notrunc bs=1 count=1 seek=3221225471 2> /dev/null
+$TUNE2FS -I 256 -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+$FSCK -N test_filesys -y -f -D $TMPFILE >> $OUT 2>&1
+
+# dump and check
+($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.after 2> /dev/null
+echo "Change in FS metadata:" >> $OUT
+diff -u0 $OUT.before $OUT.after | sed -e '/^---.*/d' -e '/^+++.*/d' >> $OUT
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+rm $TMPFILE
+
+#
+# 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
+
+rm $OUT.before $OUT.after
+
+unset IMAGE FSCK_OPT OUT EXP CONF
+
+else #if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
+ echo "$test_name: $test_description: skipped"
+fi
+

2014-12-03 02:07:13

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] e2fsprogs/tune2fs: fix memory leak in inode_scan_and_fix()

On Fri, Nov 14, 2014 at 05:15:35PM +0800, Xiaoguang Wang wrote:
> When we use ext2fs_open_inode_scan() to iterate inodes and finish
> jobs, we also need a ext2fs_close_inode_scan(scan) operation, but in
> inode_scan_and_fix(), we forgot to call it, fix this error.
>
> Signed-off-by: Xiaoguang Wang <[email protected]>
> Reviewed-by: Darrick J. Wong <[email protected]>

Applied, thanks.

- Ted

2014-12-03 03:30:32

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

On Fri, Nov 14, 2014 at 05:15:36PM +0800, Xiaoguang Wang wrote:
> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
> obviously, we need to re-compute the group descriptor checksums, and the inode
> 's size has also changed, we also need to recompute the checksums of inodes for
> metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
> this will fix checksum issues.
>
> Meanwhile, the patch will trigger an existing memory write overflow, which will
> casue segfault, please see the next patch.
>
> Signed-off-by: Xiaoguang Wang <[email protected]>
> Reviewed-by: Darrick J. Wong <[email protected]>

Applied, thanks.

- Ted

2014-12-03 03:33:47

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] tune2fs: disable csum verification before resizing inode

On Mon, Nov 17, 2014 at 11:10:39AM -0800, Darrick J. Wong wrote:
> When we're turning on metadata checksumming /and/ resizing the inode
> at the same time, disable checksum verification during the
> resize_inode() call because the subroutines it calls will try to
> verify the checksums (which have not yet been set), causing the
> operation to fail unnecessarily.
>
> Signed-off-by: Darrick J. Wong <[email protected]>

Applied, thanks.

- Ted

2014-12-03 03:46:56

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] tune2fs: disable csum verification before resizing inode

On Mon, Nov 17, 2014 at 11:10:39AM -0800, Darrick J. Wong wrote:
> When we're turning on metadata checksumming /and/ resizing the inode
> at the same time, disable checksum verification during the
> resize_inode() call because the subroutines it calls will try to
> verify the checksums (which have not yet been set), causing the
> operation to fail unnecessarily.
>
> Signed-off-by: Darrick J. Wong <[email protected]>

The t_iexpand_mcsum test is failing when I try applying this patch:

--- /usr/projects/e2fsprogs/e2fsprogs/tests/t_iexpand_mcsum/expect 2014-12-03 03:33:08.359025380 +0000
+++ t_iexpand_mcsum.log 2014-12-03 03:38:42.519031358 +0000
@@ -26,9 +26,388 @@

Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
+Directory inode 2, block #0, offset 1004: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #116, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #117, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #118, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #119, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #120, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #121, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #122, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #123, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #1, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #2, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #3, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #4, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #5, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #6, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #7, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #8, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #9, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #10, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #11, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #12, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #13, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #14, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #15, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #16, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #17, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #18, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #19, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #20, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #21, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #22, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #23, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #24, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #25, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #26, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #27, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #28, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #29, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #30, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #31, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #32, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #33, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #34, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #35, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #36, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #37, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #38, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #39, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #40, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #41, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #42, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #43, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #44, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #45, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #46, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #47, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #48, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #49, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #50, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #51, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #52, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #53, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #54, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #55, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #56, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #57, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #58, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #59, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #60, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #61, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #62, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #63, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #64, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #65, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #66, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #67, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #68, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #69, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #70, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #71, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #72, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #73, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #74, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #75, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #76, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #77, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #78, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #79, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #80, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #81, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #82, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #83, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #84, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #85, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #86, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #87, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #88, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #89, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #90, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #91, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #92, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #93, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #94, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #95, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #96, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #97, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #98, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #99, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #100, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #101, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #102, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #103, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #104, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #105, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #106, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #107, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #108, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #109, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #110, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #111, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #112, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #113, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #114, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #115, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
Pass 3: Checking directory connectivity
+Error while trying to find /lost+found: Directory block checksum does not match directory block
+/lost+found not found. Create? yes
+
+Error creating /lost+found directory (ext2fs_link): Directory block checksum does not match directory block
Pass 3A: Optimizing directories
Pass 4: Checking reference counts
+Unattached inode 6346
+Connect to /lost+found? yes
+
Pass 5: Checking group summary information


@@ -37,9 +416,11 @@
@@ -5 +5 @@
-Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
+Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
-@@ -13 +13 @@
--Free blocks: 16280
-+Free blocks: 3958
+@@ -13,2 +13,2 @@
+-Free blocks: 16282
+-Free inodes: 91959
++Free blocks: 3970
++Free inodes: 91958
@@ -22 +22 @@
-Inode blocks per group: 128
+Inode blocks per group: 256
@@ -51,7 +432,17 @@
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
+'..' in /lost+found/#6346 (6346) is / (2), should be /lost+found (11).
+Fix? no
+
Pass 4: Checking reference counts
+Inode 2 ref count is 3, should be 4. Fix? no
+
+Inode 6346 ref count is 3, should be 2. Fix? no
+
Pass 5: Checking group summary information

-Exit status is 0
+
+
+
+Exit status is 4


> ---
> misc/tune2fs.c | 7 +++-
> tests/t_iexpand_mcsum/expect | 57 ++++++++++++++++++++++++++++++
> tests/t_iexpand_mcsum/name | 1 +
> tests/t_iexpand_mcsum/script | 80 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 144 insertions(+), 1 deletion(-)
> create mode 100644 tests/t_iexpand_mcsum/expect
> create mode 100644 tests/t_iexpand_mcsum/name
> create mode 100644 tests/t_iexpand_mcsum/script
>
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index b510c49..f01b05b 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2961,8 +2961,13 @@ retry_open:
> * We want to update group descriptor also
> * with the new free inode count
> */
> + if (rewrite_checksums)
> + fs->flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
> fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
> - if (resize_inode(fs, new_inode_size) == 0) {
> + retval = resize_inode(fs, new_inode_size);
> + if (rewrite_checksums)
> + fs->flags &= ~EXT2_FLAG_IGNORE_CSUM_ERRORS;
> + if (retval == 0) {
> printf(_("Setting inode size %lu\n"),
> new_inode_size);
> rewrite_checksums = 1;
> diff --git a/tests/t_iexpand_mcsum/expect b/tests/t_iexpand_mcsum/expect
> new file mode 100644
> index 0000000..2bebc9f
> --- /dev/null
> +++ b/tests/t_iexpand_mcsum/expect
> @@ -0,0 +1,57 @@
> +tune2fs test
> +Creating filesystem with 786432 1k blocks and 98304 inodes
> +Superblock backups stored on blocks:
> + 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409, 663553
> +
> +Allocating group tables: done
> +Writing inode tables: done
> +Creating journal (16384 blocks): done
> +Creating 6334 huge file(s) with 117 blocks each: done
> +Writing superblocks and filesystem accounting information: done
> +
> +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 -I 256 -O metadata_csum test.img
> +Setting inode size 256
> +
> +Please run e2fsck -D on the filesystem.
> +
> +Exit status is 0
> +Backing up journal inode block information.
> +
> +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
> +
> +
> +
> +Change in FS metadata:
> +@@ -5 +5 @@
> +-Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> ++Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> +@@ -13 +13 @@
> +-Free blocks: 16280
> ++Free blocks: 3958
> +@@ -22 +22 @@
> +-Inode blocks per group: 128
> ++Inode blocks per group: 256
> +@@ -28 +28 @@
> +-Inode size: 128
> ++Inode size: 256
> +@@ -31,0 +32 @@
> ++Checksum type: crc32c
> +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_iexpand_mcsum/name b/tests/t_iexpand_mcsum/name
> new file mode 100644
> index 0000000..e767715
> --- /dev/null
> +++ b/tests/t_iexpand_mcsum/name
> @@ -0,0 +1 @@
> +expand inodes and turn on metadata_csum
> diff --git a/tests/t_iexpand_mcsum/script b/tests/t_iexpand_mcsum/script
> new file mode 100644
> index 0000000..cb424ed
> --- /dev/null
> +++ b/tests/t_iexpand_mcsum/script
> @@ -0,0 +1,80 @@
> +if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> +
> +FSCK_OPT=-fn
> +OUT=$test_name.log
> +EXP=$test_dir/expect
> +CONF=$TMPFILE.conf
> +
> +#gzip -d < $EXP.gz > $EXP
> +
> +cat > $CONF << ENDL
> +[fs_types]
> + ext4h = {
> + features = has_journal,extent,huge_file,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,^resize_inode,^meta_bg,^flex_bg,^metadata_csum,64bit
> + blocksize = 1024
> + inode_size = 256
> + make_hugefiles = true
> + hugefiles_dir = /
> + hugefiles_slack = 16000K
> + hugefiles_name = aaaaa
> + hugefiles_digits = 4
> + hugefiles_size = 117K
> + zero_hugefiles = false
> + }
> +ENDL
> +
> +echo "tune2fs test" > $OUT
> +
> +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -I 128 $TMPFILE 786432 >> $OUT 2>&1
> +rm -rf $CONF
> +
> +# dump and check
> +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.before 2> /dev/null
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# convert it
> +echo "tune2fs -I 256 -O metadata_csum test.img" >> $OUT
> +dd if=/dev/zero of=$TMPFILE conv=notrunc bs=1 count=1 seek=3221225471 2> /dev/null
> +$TUNE2FS -I 256 -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +$FSCK -N test_filesys -y -f -D $TMPFILE >> $OUT 2>&1
> +
> +# dump and check
> +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.after 2> /dev/null
> +echo "Change in FS metadata:" >> $OUT
> +diff -u0 $OUT.before $OUT.after | sed -e '/^---.*/d' -e '/^+++.*/d' >> $OUT
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +rm $TMPFILE
> +
> +#
> +# 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
> +
> +rm $OUT.before $OUT.after
> +
> +unset IMAGE FSCK_OPT OUT EXP CONF
> +
> +else #if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> + echo "$test_name: $test_description: skipped"
> +fi
> +

2014-12-03 05:40:02

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH] tune2fs: disable csum verification before resizing inode

On Tue, Dec 02, 2014 at 10:46:49PM -0500, Theodore Ts'o wrote:
> On Mon, Nov 17, 2014 at 11:10:39AM -0800, Darrick J. Wong wrote:
> > When we're turning on metadata checksumming /and/ resizing the inode
> > at the same time, disable checksum verification during the
> > resize_inode() call because the subroutines it calls will try to
> > verify the checksums (which have not yet been set), causing the
> > operation to fail unnecessarily.
> >
> > Signed-off-by: Darrick J. Wong <[email protected]>
>
> The t_iexpand_mcsum test is failing when I try applying this patch:
>
> --- /usr/projects/e2fsprogs/e2fsprogs/tests/t_iexpand_mcsum/expect 2014-12-03 03:33:08.359025380 +0000
> +++ t_iexpand_mcsum.log 2014-12-03 03:38:42.519031358 +0000
> @@ -26,9 +26,388 @@
>
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> +Directory inode 2, block #0, offset 1004: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #116, offset 1000: directory passes checks but fails checksum.
> +Fix? yes

Arrrgh. The fun of maintaining the test suite when the patches go into -next
in a different order from where they are in my tree... :)

The patches "e2fsck: only complain about no-checksum directory blocks once" and
"e2fsck: don't complain about root dir csum failures when getting lnf" fix fsck
not to print these unnecessary error messages. They both pre-date the creation
of this patch, which is why the test is failing. You can safely amend the
/expect file, but it'll be necessary to un-amend it if you take those two older
patches.

:)

--D

> +
> +Directory inode 2, block #117, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #118, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #119, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #120, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #121, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #122, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #123, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #1, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #2, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #3, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #4, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #5, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #6, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #7, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #8, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #9, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #10, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #11, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #12, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #13, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #14, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #15, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #16, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #17, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #18, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #19, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #20, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #21, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #22, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #23, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #24, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #25, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #26, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #27, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #28, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #29, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #30, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #31, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #32, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #33, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #34, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #35, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #36, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #37, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #38, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #39, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #40, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #41, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #42, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #43, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #44, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #45, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #46, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #47, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #48, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #49, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #50, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #51, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #52, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #53, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #54, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #55, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #56, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #57, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #58, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #59, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #60, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #61, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #62, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #63, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #64, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #65, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #66, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #67, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #68, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #69, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #70, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #71, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #72, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #73, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #74, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #75, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #76, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #77, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #78, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #79, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #80, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #81, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #82, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #83, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #84, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #85, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #86, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #87, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #88, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #89, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #90, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #91, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #92, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #93, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #94, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #95, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #96, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #97, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #98, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #99, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #100, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #101, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #102, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #103, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #104, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #105, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #106, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #107, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #108, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #109, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #110, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #111, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #112, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #113, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #114, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #115, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> Pass 3: Checking directory connectivity
> +Error while trying to find /lost+found: Directory block checksum does not match directory block
> +/lost+found not found. Create? yes
> +
> +Error creating /lost+found directory (ext2fs_link): Directory block checksum does not match directory block
> Pass 3A: Optimizing directories
> Pass 4: Checking reference counts
> +Unattached inode 6346
> +Connect to /lost+found? yes
> +
> Pass 5: Checking group summary information
>
>
> @@ -37,9 +416,11 @@
> @@ -5 +5 @@
> -Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> +Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> -@@ -13 +13 @@
> --Free blocks: 16280
> -+Free blocks: 3958
> +@@ -13,2 +13,2 @@
> +-Free blocks: 16282
> +-Free inodes: 91959
> ++Free blocks: 3970
> ++Free inodes: 91958
> @@ -22 +22 @@
> -Inode blocks per group: 128
> +Inode blocks per group: 256
> @@ -51,7 +432,17 @@
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> +'..' in /lost+found/#6346 (6346) is / (2), should be /lost+found (11).
> +Fix? no
> +
> Pass 4: Checking reference counts
> +Inode 2 ref count is 3, should be 4. Fix? no
> +
> +Inode 6346 ref count is 3, should be 2. Fix? no
> +
> Pass 5: Checking group summary information
>
> -Exit status is 0
> +
> +
> +
> +Exit status is 4
>
>
> > ---
> > misc/tune2fs.c | 7 +++-
> > tests/t_iexpand_mcsum/expect | 57 ++++++++++++++++++++++++++++++
> > tests/t_iexpand_mcsum/name | 1 +
> > tests/t_iexpand_mcsum/script | 80 ++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 144 insertions(+), 1 deletion(-)
> > create mode 100644 tests/t_iexpand_mcsum/expect
> > create mode 100644 tests/t_iexpand_mcsum/name
> > create mode 100644 tests/t_iexpand_mcsum/script
> >
> > diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> > index b510c49..f01b05b 100644
> > --- a/misc/tune2fs.c
> > +++ b/misc/tune2fs.c
> > @@ -2961,8 +2961,13 @@ retry_open:
> > * We want to update group descriptor also
> > * with the new free inode count
> > */
> > + if (rewrite_checksums)
> > + fs->flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
> > fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
> > - if (resize_inode(fs, new_inode_size) == 0) {
> > + retval = resize_inode(fs, new_inode_size);
> > + if (rewrite_checksums)
> > + fs->flags &= ~EXT2_FLAG_IGNORE_CSUM_ERRORS;
> > + if (retval == 0) {
> > printf(_("Setting inode size %lu\n"),
> > new_inode_size);
> > rewrite_checksums = 1;
> > diff --git a/tests/t_iexpand_mcsum/expect b/tests/t_iexpand_mcsum/expect
> > new file mode 100644
> > index 0000000..2bebc9f
> > --- /dev/null
> > +++ b/tests/t_iexpand_mcsum/expect
> > @@ -0,0 +1,57 @@
> > +tune2fs test
> > +Creating filesystem with 786432 1k blocks and 98304 inodes
> > +Superblock backups stored on blocks:
> > + 8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409, 663553
> > +
> > +Allocating group tables: done
> > +Writing inode tables: done
> > +Creating journal (16384 blocks): done
> > +Creating 6334 huge file(s) with 117 blocks each: done
> > +Writing superblocks and filesystem accounting information: done
> > +
> > +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 -I 256 -O metadata_csum test.img
> > +Setting inode size 256
> > +
> > +Please run e2fsck -D on the filesystem.
> > +
> > +Exit status is 0
> > +Backing up journal inode block information.
> > +
> > +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
> > +
> > +
> > +
> > +Change in FS metadata:
> > +@@ -5 +5 @@
> > +-Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> > ++Filesystem features: has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> > +@@ -13 +13 @@
> > +-Free blocks: 16280
> > ++Free blocks: 3958
> > +@@ -22 +22 @@
> > +-Inode blocks per group: 128
> > ++Inode blocks per group: 256
> > +@@ -28 +28 @@
> > +-Inode size: 128
> > ++Inode size: 256
> > +@@ -31,0 +32 @@
> > ++Checksum type: crc32c
> > +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_iexpand_mcsum/name b/tests/t_iexpand_mcsum/name
> > new file mode 100644
> > index 0000000..e767715
> > --- /dev/null
> > +++ b/tests/t_iexpand_mcsum/name
> > @@ -0,0 +1 @@
> > +expand inodes and turn on metadata_csum
> > diff --git a/tests/t_iexpand_mcsum/script b/tests/t_iexpand_mcsum/script
> > new file mode 100644
> > index 0000000..cb424ed
> > --- /dev/null
> > +++ b/tests/t_iexpand_mcsum/script
> > @@ -0,0 +1,80 @@
> > +if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> > +
> > +FSCK_OPT=-fn
> > +OUT=$test_name.log
> > +EXP=$test_dir/expect
> > +CONF=$TMPFILE.conf
> > +
> > +#gzip -d < $EXP.gz > $EXP
> > +
> > +cat > $CONF << ENDL
> > +[fs_types]
> > + ext4h = {
> > + features = has_journal,extent,huge_file,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,^resize_inode,^meta_bg,^flex_bg,^metadata_csum,64bit
> > + blocksize = 1024
> > + inode_size = 256
> > + make_hugefiles = true
> > + hugefiles_dir = /
> > + hugefiles_slack = 16000K
> > + hugefiles_name = aaaaa
> > + hugefiles_digits = 4
> > + hugefiles_size = 117K
> > + zero_hugefiles = false
> > + }
> > +ENDL
> > +
> > +echo "tune2fs test" > $OUT
> > +
> > +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -I 128 $TMPFILE 786432 >> $OUT 2>&1
> > +rm -rf $CONF
> > +
> > +# dump and check
> > +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.before 2> /dev/null
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# convert it
> > +echo "tune2fs -I 256 -O metadata_csum test.img" >> $OUT
> > +dd if=/dev/zero of=$TMPFILE conv=notrunc bs=1 count=1 seek=3221225471 2> /dev/null
> > +$TUNE2FS -I 256 -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +$FSCK -N test_filesys -y -f -D $TMPFILE >> $OUT 2>&1
> > +
> > +# dump and check
> > +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.after 2> /dev/null
> > +echo "Change in FS metadata:" >> $OUT
> > +diff -u0 $OUT.before $OUT.after | sed -e '/^---.*/d' -e '/^+++.*/d' >> $OUT
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +rm $TMPFILE
> > +
> > +#
> > +# 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
> > +
> > +rm $OUT.before $OUT.after
> > +
> > +unset IMAGE FSCK_OPT OUT EXP CONF
> > +
> > +else #if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> > + echo "$test_name: $test_description: skipped"
> > +fi
> > +
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-12-03 14:34:35

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] tune2fs: disable csum verification before resizing inode

On Tue, Dec 02, 2014 at 09:39:51PM -0800, Darrick J. Wong wrote:
>
> The patches "e2fsck: only complain about no-checksum directory blocks once" and
> "e2fsck: don't complain about root dir csum failures when getting lnf" fix fsck
> not to print these unnecessary error messages. They both pre-date the creation
> of this patch, which is why the test is failing. You can safely amend the
> /expect file, but it'll be necessary to un-amend it if you take those two older
> patches.

Ah, I figured it was something like that. OK, I'll find those patches
and get them into my tree; so I'll fix it on my end.

Thanks!!

- Ted