Patchwork mke2fs, tune2fs, resize2fs: add warning messages for bigalloc and quota

login
register
mail settings
Submitter Theodore Ts'o
Date Jan. 22, 2013, 12:09 a.m.
Message ID <1358813397-26528-1-git-send-email-tytso@mit.edu>
Download mbox | patch
Permalink /patch/214297/
State Accepted
Headers show

Comments

Theodore Ts'o - Jan. 22, 2013, 12:09 a.m.
The bigalloc and quota features have some known issues, so issue
warnings in case users try to use them.

More information can be found here:
	https://ext4.wiki.kernel.org/index.php/Bigalloc
	https://ext4.wiki.kernel.org/index.php/Quota

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 misc/mke2fs.c  | 12 ++++++++++++
 misc/tune2fs.c |  4 ++++
 resize/main.c  | 14 ++++++++++++++
 3 files changed, 30 insertions(+)
Eric Sandeen - Jan. 22, 2013, 12:19 a.m.
On Jan 21, 2013, at 6:16 PM, "Theodore Ts'o" <tytso@mit.edu> wrote:

> The bigalloc and quota features have some known issues, so issue
> warnings in case users try to use them.
> 
Looks good, thanks Ted.

Eric

> More information can be found here:
>    https://ext4.wiki.kernel.org/index.php/Bigalloc
>    https://ext4.wiki.kernel.org/index.php/Quota
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
> misc/mke2fs.c  | 12 ++++++++++++
> misc/tune2fs.c |  4 ++++
> resize/main.c  | 14 ++++++++++++++
> 3 files changed, 30 insertions(+)
> 
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index ace9eae..fe5ce7d 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -1942,6 +1942,18 @@ profile_error:
>        exit(1);
>    }
> 
> +    if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_BIGALLOC)
> +        fprintf(stderr, _("\nWarning: the bigalloc feature is still "
> +                  "under development\n"
> +                  "See https://ext4.wiki.kernel.org/"
> +                  "index.php/Bigalloc for more information\n\n"));
> +
> +    if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_QUOTA)
> +        fprintf(stderr, _("\nWarning: the quota feature is still "
> +                  "under development\n"
> +                  "See https://ext4.wiki.kernel.org/"
> +                  "index.php/Quota for more information\n\n"));
> +
>    /* Since sparse_super is the default, we would only have a problem
>     * here if it was explicitly disabled.
>     */
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 2c8b9e8..849f3a0 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -736,6 +736,10 @@ void handle_quota_options(ext2_filsys fs)
>    quota_release_context(&qctx);
> 
>    if ((usrquota == QOPT_ENABLE) || (grpquota == QOPT_ENABLE)) {
> +        fprintf(stderr, _("\nWarning: the quota feature is still "
> +                  "under development\n"
> +                  "See https://ext4.wiki.kernel.org/"
> +                  "index.php/Quota for more information\n\n"));
>        fs->super->s_feature_ro_compat |= EXT4_FEATURE_RO_COMPAT_QUOTA;
>        ext2fs_mark_super_dirty(fs);
>    } else if (!fs->super->s_usr_quota_inum &&
> diff --git a/resize/main.c b/resize/main.c
> index 711e375..4cbfe69 100644
> --- a/resize/main.c
> +++ b/resize/main.c
> @@ -147,6 +147,18 @@ static void determine_fs_stride(ext2_filsys fs)
> #endif
> }
> 
> +static bigalloc_check(ext2_filsys fs, int force)
> +{
> +    if (!force && EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
> +                EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
> +        fprintf(stderr, _("\nResizing bigalloc file systems has "
> +                  "not been fully tested.  Proceed\n"
> +                  "at your own risk!  Use the force option "
> +                  "if you want to go ahead anyway.\n\n"));
> +        exit(1);
> +    }
> +}
> +
> int main (int argc, char ** argv)
> {
>    errcode_t    retval;
> @@ -428,6 +440,7 @@ int main (int argc, char ** argv)
>        exit(0);
>    }
>    if (mount_flags & EXT2_MF_MOUNTED) {
> +        bigalloc_check(fs, force);
>        retval = online_resize_fs(fs, mtpt, &new_size, flags);
>    } else {
>        if (!force && ((fs->super->s_lastcheck < fs->super->s_mtime) ||
> @@ -438,6 +451,7 @@ int main (int argc, char ** argv)
>                device_name);
>            exit(1);
>        }
> +        bigalloc_check(fs, force);
>        printf(_("Resizing the filesystem on "
>             "%s to %llu (%dk) blocks.\n"),
>               device_name, new_size, fs->blocksize / 1024);
> -- 
> 1.7.12.rc0.22.gcdd159b
> 
> --
> 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
--
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
Andreas Dilger - Jan. 22, 2013, 5:24 p.m.
On 2013-01-21, at 17:19, Eric Sandeen <esandeen@redhat.com> wrote:

> On Jan 21, 2013, at 6:16 PM, "Theodore Ts'o" <tytso@mit.edu> wrote:
> 
>> The bigalloc and quota features have some known issues, so issue
>> warnings in case users try to use them.
>> 
> Looks good, thanks Ted.

My preference would be to have separate "--force-bigalloc" and "--force-quota" options instead of lumping everything under a single "--force/-f" flag. That can become habitual for one kind of usage (e.g. mke2fs on a regular file) and then it also forces other dangerous behaviour (such as these) as an unknown side effect. 

Cheers, Andreas 

>> More information can be found here:
>>   https://ext4.wiki.kernel.org/index.php/Bigalloc
>>   https://ext4.wiki.kernel.org/index.php/Quota
>> 
>> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>> ---
>> misc/mke2fs.c  | 12 ++++++++++++
>> misc/tune2fs.c |  4 ++++
>> resize/main.c  | 14 ++++++++++++++
>> 3 files changed, 30 insertions(+)
>> 
>> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
>> index ace9eae..fe5ce7d 100644
>> --- a/misc/mke2fs.c
>> +++ b/misc/mke2fs.c
>> @@ -1942,6 +1942,18 @@ profile_error:
>>       exit(1);
>>   }
>> 
>> +    if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_BIGALLOC)
>> +        fprintf(stderr, _("\nWarning: the bigalloc feature is still "
>> +                  "under development\n"
>> +                  "See https://ext4.wiki.kernel.org/"
>> +                  "index.php/Bigalloc for more information\n\n"));
>> +
>> +    if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_QUOTA)
>> +        fprintf(stderr, _("\nWarning: the quota feature is still "
>> +                  "under development\n"
>> +                  "See https://ext4.wiki.kernel.org/"
>> +                  "index.php/Quota for more information\n\n"));
>> +
>>   /* Since sparse_super is the default, we would only have a problem
>>    * here if it was explicitly disabled.
>>    */
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 2c8b9e8..849f3a0 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -736,6 +736,10 @@ void handle_quota_options(ext2_filsys fs)
>>   quota_release_context(&qctx);
>> 
>>   if ((usrquota == QOPT_ENABLE) || (grpquota == QOPT_ENABLE)) {
>> +        fprintf(stderr, _("\nWarning: the quota feature is still "
>> +                  "under development\n"
>> +                  "See https://ext4.wiki.kernel.org/"
>> +                  "index.php/Quota for more information\n\n"));
>>       fs->super->s_feature_ro_compat |= EXT4_FEATURE_RO_COMPAT_QUOTA;
>>       ext2fs_mark_super_dirty(fs);
>>   } else if (!fs->super->s_usr_quota_inum &&
>> diff --git a/resize/main.c b/resize/main.c
>> index 711e375..4cbfe69 100644
>> --- a/resize/main.c
>> +++ b/resize/main.c
>> @@ -147,6 +147,18 @@ static void determine_fs_stride(ext2_filsys fs)
>> #endif
>> }
>> 
>> +static bigalloc_check(ext2_filsys fs, int force)
>> +{
>> +    if (!force && EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
>> +                EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
>> +        fprintf(stderr, _("\nResizing bigalloc file systems has "
>> +                  "not been fully tested.  Proceed\n"
>> +                  "at your own risk!  Use the force option "
>> +                  "if you want to go ahead anyway.\n\n"));
>> +        exit(1);
>> +    }
>> +}
>> +
>> int main (int argc, char ** argv)
>> {
>>   errcode_t    retval;
>> @@ -428,6 +440,7 @@ int main (int argc, char ** argv)
>>       exit(0);
>>   }
>>   if (mount_flags & EXT2_MF_MOUNTED) {
>> +        bigalloc_check(fs, force);
>>       retval = online_resize_fs(fs, mtpt, &new_size, flags);
>>   } else {
>>       if (!force && ((fs->super->s_lastcheck < fs->super->s_mtime) ||
>> @@ -438,6 +451,7 @@ int main (int argc, char ** argv)
>>               device_name);
>>           exit(1);
>>       }
>> +        bigalloc_check(fs, force);
>>       printf(_("Resizing the filesystem on "
>>            "%s to %llu (%dk) blocks.\n"),
>>              device_name, new_size, fs->blocksize / 1024);
>> -- 
>> 1.7.12.rc0.22.gcdd159b
>> 
>> --
>> 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
> --
> 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
--
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
Eric Sandeen - Jan. 22, 2013, 5:46 p.m.
On 1/22/13 11:24 AM, Andreas Dilger wrote:
> My preference would be to have separate "--force-bigalloc" and
> "--force-quota" options instead of lumping everything under a single
> "--force/-f" flag. That can become habitual for one kind of usage
> (e.g. mke2fs on a regular file) and then it also forces other
> dangerous behaviour (such as these) as an unknown side effect.

Actually, that's a good point, I had the same thought and meant
to mention it but forgot. :(

I guess the release is out there now, though.

-Eric
--
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
Theodore Ts'o - Jan. 22, 2013, 6:45 p.m.
On Tue, Jan 22, 2013 at 10:24:47AM -0700, Andreas Dilger wrote:
> 
> My preference would be to have separate "--force-bigalloc" and
> "--force-quota" options instead of lumping everything under a single
> "--force/-f" flag. That can become habitual for one kind of usage
> (e.g. mke2fs on a regular file) and then it also forces other
> dangerous behaviour (such as these) as an unknown side effect.

This would require us to figure out a backwards compatible way of
dealing with using getopt_long() for systems that don't have
getopt_long().  One of the things which I've been thinking about doing
in the 1.43 series is to rename libquota.a to libsupport.a, and then
moving e2fsck/profile.c and some of misc/util.c into libsupport.a.
Then we could optionally include the getopt_long() sources if we are
building on a system that doesn't have it.  This assumes that
getopt_long() is portable enough, and I don't know what kind of
dependencies it might drag in....

In the long term, I agree with you.

                                        - Ted

--
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/misc/mke2fs.c b/misc/mke2fs.c
index ace9eae..fe5ce7d 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1942,6 +1942,18 @@  profile_error:
 		exit(1);
 	}
 
+	if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_BIGALLOC)
+		fprintf(stderr, _("\nWarning: the bigalloc feature is still "
+				  "under development\n"
+				  "See https://ext4.wiki.kernel.org/"
+				  "index.php/Bigalloc for more information\n\n"));
+
+	if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_QUOTA)
+		fprintf(stderr, _("\nWarning: the quota feature is still "
+				  "under development\n"
+				  "See https://ext4.wiki.kernel.org/"
+				  "index.php/Quota for more information\n\n"));
+
 	/* Since sparse_super is the default, we would only have a problem
 	 * here if it was explicitly disabled.
 	 */
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 2c8b9e8..849f3a0 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -736,6 +736,10 @@  void handle_quota_options(ext2_filsys fs)
 	quota_release_context(&qctx);
 
 	if ((usrquota == QOPT_ENABLE) || (grpquota == QOPT_ENABLE)) {
+		fprintf(stderr, _("\nWarning: the quota feature is still "
+				  "under development\n"
+				  "See https://ext4.wiki.kernel.org/"
+				  "index.php/Quota for more information\n\n"));
 		fs->super->s_feature_ro_compat |= EXT4_FEATURE_RO_COMPAT_QUOTA;
 		ext2fs_mark_super_dirty(fs);
 	} else if (!fs->super->s_usr_quota_inum &&
diff --git a/resize/main.c b/resize/main.c
index 711e375..4cbfe69 100644
--- a/resize/main.c
+++ b/resize/main.c
@@ -147,6 +147,18 @@  static void determine_fs_stride(ext2_filsys fs)
 #endif
 }
 
+static bigalloc_check(ext2_filsys fs, int force)
+{
+	if (!force && EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
+				EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
+		fprintf(stderr, _("\nResizing bigalloc file systems has "
+				  "not been fully tested.  Proceed\n"
+				  "at your own risk!  Use the force option "
+				  "if you want to go ahead anyway.\n\n"));
+		exit(1);
+	}
+}
+
 int main (int argc, char ** argv)
 {
 	errcode_t	retval;
@@ -428,6 +440,7 @@  int main (int argc, char ** argv)
 		exit(0);
 	}
 	if (mount_flags & EXT2_MF_MOUNTED) {
+		bigalloc_check(fs, force);
 		retval = online_resize_fs(fs, mtpt, &new_size, flags);
 	} else {
 		if (!force && ((fs->super->s_lastcheck < fs->super->s_mtime) ||
@@ -438,6 +451,7 @@  int main (int argc, char ** argv)
 				device_name);
 			exit(1);
 		}
+		bigalloc_check(fs, force);
 		printf(_("Resizing the filesystem on "
 			 "%s to %llu (%dk) blocks.\n"),
 		       device_name, new_size, fs->blocksize / 1024);