Patchwork [03/12] ext4: prevent a fs without journal from being resized

login
register
mail settings
Submitter Yongqiang Yang
Date July 18, 2011, 2:52 a.m.
Message ID <1310957555-15617-4-git-send-email-xiaoqiangnk@gmail.com>
Download mbox | patch
Permalink /patch/105146/
State Superseded
Headers show

Comments

Yongqiang Yang - July 18, 2011, 2:52 a.m.
This patch prevents a fs without journal from being resized, because
it is easy to detroy the fs.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/resize.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
Tao Ma - July 18, 2011, 3:17 a.m.
On 07/18/2011 10:52 AM, Yongqiang Yang wrote:
> This patch prevents a fs without journal from being resized, because
> it is easy to detroy the fs.
Why you want to do this?  You see any corruption?
At least in our product system, no-journal mode is heavily used and we
really don't want to disable this feature.

Thanks
Tao
> 
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> ---
>  fs/ext4/resize.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 53d9795..33ab40d 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -33,6 +33,16 @@ int ext4_resize_begin(struct super_block *sb)
>  		return -EPERM;
>  	}
>  
> +	/*
> +	 * We are not allowed to do online-resizing on a filesystem without
> +	 * journal, otherwise, it is easy to destroy the filesystem.
> +	 */
> +	if (!EXT4_SB(sb)->s_journal) {
> +		ext4_warning(sb, "There is no journal for the filesystem, "
> +			     "so online resizing is not allowed\n");
> +		return -EPERM;
> +	}
> +
>  	if (test_and_set_bit_lock(EXT4_RESIZING, &EXT4_SB(sb)->s_resize_flags))
>  		ret = -EBUSY;
>  

--
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
Yongqiang Yang - July 18, 2011, 3:28 a.m.
On Mon, Jul 18, 2011 at 11:17 AM, Tao Ma <tm@tao.ma> wrote:
> On 07/18/2011 10:52 AM, Yongqiang Yang wrote:
>> This patch prevents a fs without journal from being resized, because
>> it is easy to detroy the fs.
> Why you want to do this?  You see any corruption?
> At least in our product system, no-journal mode is heavily used and we
> really don't want to disable this feature.
I did not see any corruption.  If there is no journal in a fs, then if
an error happens during online resizing, the filesystem will be
destroyed easily, I thought.  Just my thought:-)  It needs much more
feedbacks.

Thanks,
Yongqiang.
>
> Thanks
> Tao
>>
>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
>> ---
>>  fs/ext4/resize.c |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
>> index 53d9795..33ab40d 100644
>> --- a/fs/ext4/resize.c
>> +++ b/fs/ext4/resize.c
>> @@ -33,6 +33,16 @@ int ext4_resize_begin(struct super_block *sb)
>>               return -EPERM;
>>       }
>>
>> +     /*
>> +      * We are not allowed to do online-resizing on a filesystem without
>> +      * journal, otherwise, it is easy to destroy the filesystem.
>> +      */
>> +     if (!EXT4_SB(sb)->s_journal) {
>> +             ext4_warning(sb, "There is no journal for the filesystem, "
>> +                          "so online resizing is not allowed\n");
>> +             return -EPERM;
>> +     }
>> +
>>       if (test_and_set_bit_lock(EXT4_RESIZING, &EXT4_SB(sb)->s_resize_flags))
>>               ret = -EBUSY;
>>
>
>
Yongqiang Yang - July 18, 2011, 3:44 a.m.
On Mon, Jul 18, 2011 at 11:28 AM, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote:
> On Mon, Jul 18, 2011 at 11:17 AM, Tao Ma <tm@tao.ma> wrote:
>> On 07/18/2011 10:52 AM, Yongqiang Yang wrote:
>>> This patch prevents a fs without journal from being resized, because
>>> it is easy to detroy the fs.
>> Why you want to do this?  You see any corruption?
>> At least in our product system, no-journal mode is heavily used and we
>> really don't want to disable this feature.
Let's assume a situation without journal.  If the online resizing is
done successfully, it dirties super block and returns, then the added
groups could be used and some data are written to the added groups,
now comes an error before the super block are flushed. Could e2fsck
can find data in the added groups?  If not, this may bring something
strange to users.

It seems that super block should be flushed by online resizing, not
just be dirtied. :-)
Yongqiang.
> I did not see any corruption.  If there is no journal in a fs, then if
> an error happens during online resizing, the filesystem will be
> destroyed easily, I thought.  Just my thought:-)  It needs much more
> feedbacks.
>
> Thanks,
> Yongqiang.
>>
>> Thanks
>> Tao
>>>
>>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
>>> ---
>>>  fs/ext4/resize.c |   10 ++++++++++
>>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
>>> index 53d9795..33ab40d 100644
>>> --- a/fs/ext4/resize.c
>>> +++ b/fs/ext4/resize.c
>>> @@ -33,6 +33,16 @@ int ext4_resize_begin(struct super_block *sb)
>>>               return -EPERM;
>>>       }
>>>
>>> +     /*
>>> +      * We are not allowed to do online-resizing on a filesystem without
>>> +      * journal, otherwise, it is easy to destroy the filesystem.
>>> +      */
>>> +     if (!EXT4_SB(sb)->s_journal) {
>>> +             ext4_warning(sb, "There is no journal for the filesystem, "
>>> +                          "so online resizing is not allowed\n");
>>> +             return -EPERM;
>>> +     }
>>> +
>>>       if (test_and_set_bit_lock(EXT4_RESIZING, &EXT4_SB(sb)->s_resize_flags))
>>>               ret = -EBUSY;
>>>
>>
>>
>
>
>
> --
> Best Wishes
> Yongqiang Yang
>
Andreas Dilger - July 18, 2011, 7 a.m.
On 2011-07-17, at 8:52 PM, Yongqiang Yang wrote:
> This patch prevents a fs without journal from being resized, because
> it is easy to detroy the fs.

This is somewhat surprising.  I can partly agree with it - the ext4
nojournal mode appeared after the online resizing, so it probably has
some holes in the nojournal recovery.

I suspect a well-placed sync could fix any problems, however.  Probably
just before the group/space was made available would be enough, either
once per group with the current code, or possibly once per resize with
your new code (depending on how it is implemented).  If one sync per
group is considered bad (because of impact to other IO) then it might
be enough to fdatasync only the parts of the device beyond the end of
the filesystem and the backup metadata.

> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> ---
> fs/ext4/resize.c |   10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 53d9795..33ab40d 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -33,6 +33,16 @@ int ext4_resize_begin(struct super_block *sb)
> 		return -EPERM;
> 	}
> 
> +	/*
> +	 * We are not allowed to do online-resizing on a filesystem without
> +	 * journal, otherwise, it is easy to destroy the filesystem.
> +	 */
> +	if (!EXT4_SB(sb)->s_journal) {
> +		ext4_warning(sb, "There is no journal for the filesystem, "
> +			     "so online resizing is not allowed\n");
> +		return -EPERM;
> +	}
> +
> 	if (test_and_set_bit_lock(EXT4_RESIZING, &EXT4_SB(sb)->s_resize_flags))
> 		ret = -EBUSY;
> 
> -- 
> 1.7.5.1
> 


Cheers, Andreas





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

Patch

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 53d9795..33ab40d 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -33,6 +33,16 @@  int ext4_resize_begin(struct super_block *sb)
 		return -EPERM;
 	}
 
+	/*
+	 * We are not allowed to do online-resizing on a filesystem without
+	 * journal, otherwise, it is easy to destroy the filesystem.
+	 */
+	if (!EXT4_SB(sb)->s_journal) {
+		ext4_warning(sb, "There is no journal for the filesystem, "
+			     "so online resizing is not allowed\n");
+		return -EPERM;
+	}
+
 	if (test_and_set_bit_lock(EXT4_RESIZING, &EXT4_SB(sb)->s_resize_flags))
 		ret = -EBUSY;