* [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group
2013-01-15 0:37 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
@ 2013-01-15 0:37 ` Theodore Ts'o
2013-01-15 15:10 ` Eric Sandeen
2013-01-15 15:22 ` Zheng Liu
2013-01-15 0:37 ` [PATCH 3/5] libe2p: teach parse_num_blocks2() to return bytes if log_block_size < 0 Theodore Ts'o
` (3 subsequent siblings)
4 siblings, 2 replies; 32+ messages in thread
From: Theodore Ts'o @ 2013-01-15 0:37 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: gnehzuil.liu, Theodore Ts'o
If bigalloc is enabled, then -g will specify the clusters per block
group. (If bigalloc is not enabled, then a cluster == a block, so the
meaning of -g is not changed.)
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
misc/mke2fs.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 75d0e48..5cb49b3 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1962,6 +1962,15 @@ profile_error:
}
}
+ /*
+ * If the bigalloc feature is enabled, then the -g option will
+ * specify the number of clusters per group.
+ */
+ if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_BIGALLOC) {
+ fs_param.s_clusters_per_group = fs_param.s_blocks_per_group;
+ fs_param.s_blocks_per_group = 0;
+ }
+
if (inode_size == 0)
inode_size = get_int_from_profile(fs_types, "inode_size", 0);
if (!flex_bg_size && (fs_param.s_feature_incompat &
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group
2013-01-15 0:37 ` [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group Theodore Ts'o
@ 2013-01-15 15:10 ` Eric Sandeen
2013-01-15 19:05 ` Theodore Ts'o
2013-01-15 15:22 ` Zheng Liu
1 sibling, 1 reply; 32+ messages in thread
From: Eric Sandeen @ 2013-01-15 15:10 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List, gnehzuil.liu
On 1/14/13 6:37 PM, Theodore Ts'o wrote:
> If bigalloc is enabled, then -g will specify the clusters per block
> group. (If bigalloc is not enabled, then a cluster == a block, so the
> meaning of -g is not changed.)
This should be clearly documented in the man page as well, I think.
-Eric
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
> misc/mke2fs.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 75d0e48..5cb49b3 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -1962,6 +1962,15 @@ profile_error:
> }
> }
>
> + /*
> + * If the bigalloc feature is enabled, then the -g option will
> + * specify the number of clusters per group.
> + */
> + if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_BIGALLOC) {
> + fs_param.s_clusters_per_group = fs_param.s_blocks_per_group;
> + fs_param.s_blocks_per_group = 0;
> + }
> +
> if (inode_size == 0)
> inode_size = get_int_from_profile(fs_types, "inode_size", 0);
> if (!flex_bg_size && (fs_param.s_feature_incompat &
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group
2013-01-15 15:10 ` Eric Sandeen
@ 2013-01-15 19:05 ` Theodore Ts'o
0 siblings, 0 replies; 32+ messages in thread
From: Theodore Ts'o @ 2013-01-15 19:05 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Ext4 Developers List, gnehzuil.liu
On Tue, Jan 15, 2013 at 09:10:41AM -0600, Eric Sandeen wrote:
> On 1/14/13 6:37 PM, Theodore Ts'o wrote:
> > If bigalloc is enabled, then -g will specify the clusters per block
> > group. (If bigalloc is not enabled, then a cluster == a block, so the
> > meaning of -g is not changed.)
>
> This should be clearly documented in the man page as well, I think.
Yes, I'm going to include this change as a part of the Zheng's man
page clean up.
- Ted
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group
2013-01-15 0:37 ` [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group Theodore Ts'o
2013-01-15 15:10 ` Eric Sandeen
@ 2013-01-15 15:22 ` Zheng Liu
1 sibling, 0 replies; 32+ messages in thread
From: Zheng Liu @ 2013-01-15 15:22 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
On Mon, Jan 14, 2013 at 07:37:09PM -0500, Theodore Ts'o wrote:
> If bigalloc is enabled, then -g will specify the clusters per block
> group. (If bigalloc is not enabled, then a cluster == a block, so the
> meaning of -g is not changed.)
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>
Regards,
- Zheng
> ---
> misc/mke2fs.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 75d0e48..5cb49b3 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -1962,6 +1962,15 @@ profile_error:
> }
> }
>
> + /*
> + * If the bigalloc feature is enabled, then the -g option will
> + * specify the number of clusters per group.
> + */
> + if (fs_param.s_feature_ro_compat & EXT4_FEATURE_RO_COMPAT_BIGALLOC) {
> + fs_param.s_clusters_per_group = fs_param.s_blocks_per_group;
> + fs_param.s_blocks_per_group = 0;
> + }
> +
> if (inode_size == 0)
> inode_size = get_int_from_profile(fs_types, "inode_size", 0);
> if (!flex_bg_size && (fs_param.s_feature_incompat &
> --
> 1.7.12.rc0.22.gcdd159b
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/5] libe2p: teach parse_num_blocks2() to return bytes if log_block_size < 0
2013-01-15 0:37 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
2013-01-15 0:37 ` [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group Theodore Ts'o
@ 2013-01-15 0:37 ` Theodore Ts'o
2013-01-15 15:23 ` Zheng Liu
2013-01-15 0:37 ` [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M Theodore Ts'o
` (2 subsequent siblings)
4 siblings, 1 reply; 32+ messages in thread
From: Theodore Ts'o @ 2013-01-15 0:37 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: gnehzuil.liu, Theodore Ts'o
Previously the behavior of parse_num_block2 was undefined if
log_block_size was less than zero. It will now return a number in
units of bytes.
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
lib/e2p/parse_num.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/lib/e2p/parse_num.c b/lib/e2p/parse_num.c
index d9ad3e7..cb0dc5b 100644
--- a/lib/e2p/parse_num.c
+++ b/lib/e2p/parse_num.c
@@ -35,10 +35,16 @@ unsigned long long parse_num_blocks2(const char *arg, int log_block_size)
num <<= 10;
/* fallthrough */
case 'K': case 'k':
- num >>= log_block_size;
+ if (log_block_size < 0)
+ num <<= 10;
+ else
+ num >>= log_block_size;
break;
case 's':
- num >>= (1+log_block_size);
+ if (log_block_size < 0)
+ num << 1;
+ else
+ num >>= (1+log_block_size);
break;
case '\0':
break;
@@ -62,11 +68,21 @@ main(int argc, char **argv)
unsigned long num;
int log_block_size = 0;
- if (argc != 2) {
- fprintf(stderr, "Usage: %s arg\n", argv[0]);
+ if (argc != 2 && argc != 3) {
+ fprintf(stderr, "Usage: %s arg [log_block_size]\n", argv[0]);
exit(1);
}
+ if (argc == 3) {
+ char *p;
+
+ log_block_size = strtol(argv[2], &p, 0);
+ if (*p) {
+ fprintf(stderr, "Bad log_block_size: %s\n", argv[2]);
+ exit(1);
+ }
+ }
+
num = parse_num_blocks(argv[1], log_block_size);
printf("Parsed number: %lu\n", num);
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 3/5] libe2p: teach parse_num_blocks2() to return bytes if log_block_size < 0
2013-01-15 0:37 ` [PATCH 3/5] libe2p: teach parse_num_blocks2() to return bytes if log_block_size < 0 Theodore Ts'o
@ 2013-01-15 15:23 ` Zheng Liu
0 siblings, 0 replies; 32+ messages in thread
From: Zheng Liu @ 2013-01-15 15:23 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
On Mon, Jan 14, 2013 at 07:37:10PM -0500, Theodore Ts'o wrote:
> Previously the behavior of parse_num_block2 was undefined if
> log_block_size was less than zero. It will now return a number in
> units of bytes.
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>
Regards,
- Zheng
> ---
> lib/e2p/parse_num.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/lib/e2p/parse_num.c b/lib/e2p/parse_num.c
> index d9ad3e7..cb0dc5b 100644
> --- a/lib/e2p/parse_num.c
> +++ b/lib/e2p/parse_num.c
> @@ -35,10 +35,16 @@ unsigned long long parse_num_blocks2(const char *arg, int log_block_size)
> num <<= 10;
> /* fallthrough */
> case 'K': case 'k':
> - num >>= log_block_size;
> + if (log_block_size < 0)
> + num <<= 10;
> + else
> + num >>= log_block_size;
> break;
> case 's':
> - num >>= (1+log_block_size);
> + if (log_block_size < 0)
> + num << 1;
> + else
> + num >>= (1+log_block_size);
> break;
> case '\0':
> break;
> @@ -62,11 +68,21 @@ main(int argc, char **argv)
> unsigned long num;
> int log_block_size = 0;
>
> - if (argc != 2) {
> - fprintf(stderr, "Usage: %s arg\n", argv[0]);
> + if (argc != 2 && argc != 3) {
> + fprintf(stderr, "Usage: %s arg [log_block_size]\n", argv[0]);
> exit(1);
> }
>
> + if (argc == 3) {
> + char *p;
> +
> + log_block_size = strtol(argv[2], &p, 0);
> + if (*p) {
> + fprintf(stderr, "Bad log_block_size: %s\n", argv[2]);
> + exit(1);
> + }
> + }
> +
> num = parse_num_blocks(argv[1], log_block_size);
>
> printf("Parsed number: %lu\n", num);
> --
> 1.7.12.rc0.22.gcdd159b
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M
2013-01-15 0:37 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
2013-01-15 0:37 ` [PATCH 2/5] mke2fs: the -g option will now specify the clusters per block group Theodore Ts'o
2013-01-15 0:37 ` [PATCH 3/5] libe2p: teach parse_num_blocks2() to return bytes if log_block_size < 0 Theodore Ts'o
@ 2013-01-15 0:37 ` Theodore Ts'o
2013-01-15 15:11 ` Eric Sandeen
2013-01-15 15:24 ` Zheng Liu
2013-01-15 0:37 ` [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size Theodore Ts'o
2013-01-15 0:41 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
4 siblings, 2 replies; 32+ messages in thread
From: Theodore Ts'o @ 2013-01-15 0:37 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: gnehzuil.liu, Theodore Ts'o
The -b and -C options now use parse_num_blocks2() insteda of strtol,
so that users can specify -C 256M instead of the much less convenient
-C 268435456.
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
misc/mke2fs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 5cb49b3..c16ab33 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1324,10 +1324,10 @@ profile_error:
"b:cg:i:jl:m:no:qr:s:t:vC:DE:FG:I:J:KL:M:N:O:R:ST:U:V")) != EOF) {
switch (c) {
case 'b':
- blocksize = strtol(optarg, &tmp, 0);
+ blocksize = parse_num_blocks2(optarg, -1);
b = (blocksize > 0) ? blocksize : -blocksize;
if (b < EXT2_MIN_BLOCK_SIZE ||
- b > EXT2_MAX_BLOCK_SIZE || *tmp) {
+ b > EXT2_MAX_BLOCK_SIZE) {
com_err(program_name, 0,
_("invalid block size - %s"), optarg);
exit(1);
@@ -1345,9 +1345,9 @@ profile_error:
cflag++;
break;
case 'C':
- cluster_size = strtoul(optarg, &tmp, 0);
+ cluster_size = parse_num_blocks2(optarg, -1);
if (cluster_size <= EXT2_MIN_CLUSTER_SIZE ||
- cluster_size > EXT2_MAX_CLUSTER_SIZE || *tmp) {
+ cluster_size > EXT2_MAX_CLUSTER_SIZE) {
com_err(program_name, 0,
_("invalid cluster size - %s"),
optarg);
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M
2013-01-15 0:37 ` [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M Theodore Ts'o
@ 2013-01-15 15:11 ` Eric Sandeen
2013-01-15 15:13 ` Eric Sandeen
2013-01-15 15:24 ` Zheng Liu
1 sibling, 1 reply; 32+ messages in thread
From: Eric Sandeen @ 2013-01-15 15:11 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List, gnehzuil.liu
On 1/14/13 6:37 PM, Theodore Ts'o wrote:
> The -b and -C options now use parse_num_blocks2() insteda of strtol,
> so that users can specify -C 256M instead of the much less convenient
> -C 268435456.
Hm and -C isn't in the manpage at all.
Seems like the mke2fs manpage needs an update for bigalloc?
-Eric
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
> misc/mke2fs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 5cb49b3..c16ab33 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -1324,10 +1324,10 @@ profile_error:
> "b:cg:i:jl:m:no:qr:s:t:vC:DE:FG:I:J:KL:M:N:O:R:ST:U:V")) != EOF) {
> switch (c) {
> case 'b':
> - blocksize = strtol(optarg, &tmp, 0);
> + blocksize = parse_num_blocks2(optarg, -1);
> b = (blocksize > 0) ? blocksize : -blocksize;
> if (b < EXT2_MIN_BLOCK_SIZE ||
> - b > EXT2_MAX_BLOCK_SIZE || *tmp) {
> + b > EXT2_MAX_BLOCK_SIZE) {
> com_err(program_name, 0,
> _("invalid block size - %s"), optarg);
> exit(1);
> @@ -1345,9 +1345,9 @@ profile_error:
> cflag++;
> break;
> case 'C':
> - cluster_size = strtoul(optarg, &tmp, 0);
> + cluster_size = parse_num_blocks2(optarg, -1);
> if (cluster_size <= EXT2_MIN_CLUSTER_SIZE ||
> - cluster_size > EXT2_MAX_CLUSTER_SIZE || *tmp) {
> + cluster_size > EXT2_MAX_CLUSTER_SIZE) {
> com_err(program_name, 0,
> _("invalid cluster size - %s"),
> optarg);
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M
2013-01-15 15:11 ` Eric Sandeen
@ 2013-01-15 15:13 ` Eric Sandeen
0 siblings, 0 replies; 32+ messages in thread
From: Eric Sandeen @ 2013-01-15 15:13 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List, gnehzuil.liu
On 1/15/13 9:11 AM, Eric Sandeen wrote:
> On 1/14/13 6:37 PM, Theodore Ts'o wrote:
>> The -b and -C options now use parse_num_blocks2() insteda of strtol,
>> so that users can specify -C 256M instead of the much less convenient
>> -C 268435456.
>
> Hm and -C isn't in the manpage at all.
>
> Seems like the mke2fs manpage needs an update for bigalloc?
I'm sorry, I see that patch has already been sent. I'll catch
up with my email, now. :(
-Eric
> -Eric
>
>> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>> ---
>> misc/mke2fs.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
>> index 5cb49b3..c16ab33 100644
>> --- a/misc/mke2fs.c
>> +++ b/misc/mke2fs.c
>> @@ -1324,10 +1324,10 @@ profile_error:
>> "b:cg:i:jl:m:no:qr:s:t:vC:DE:FG:I:J:KL:M:N:O:R:ST:U:V")) != EOF) {
>> switch (c) {
>> case 'b':
>> - blocksize = strtol(optarg, &tmp, 0);
>> + blocksize = parse_num_blocks2(optarg, -1);
>> b = (blocksize > 0) ? blocksize : -blocksize;
>> if (b < EXT2_MIN_BLOCK_SIZE ||
>> - b > EXT2_MAX_BLOCK_SIZE || *tmp) {
>> + b > EXT2_MAX_BLOCK_SIZE) {
>> com_err(program_name, 0,
>> _("invalid block size - %s"), optarg);
>> exit(1);
>> @@ -1345,9 +1345,9 @@ profile_error:
>> cflag++;
>> break;
>> case 'C':
>> - cluster_size = strtoul(optarg, &tmp, 0);
>> + cluster_size = parse_num_blocks2(optarg, -1);
>> if (cluster_size <= EXT2_MIN_CLUSTER_SIZE ||
>> - cluster_size > EXT2_MAX_CLUSTER_SIZE || *tmp) {
>> + cluster_size > EXT2_MAX_CLUSTER_SIZE) {
>> com_err(program_name, 0,
>> _("invalid cluster size - %s"),
>> optarg);
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M
2013-01-15 0:37 ` [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M Theodore Ts'o
2013-01-15 15:11 ` Eric Sandeen
@ 2013-01-15 15:24 ` Zheng Liu
1 sibling, 0 replies; 32+ messages in thread
From: Zheng Liu @ 2013-01-15 15:24 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
On Mon, Jan 14, 2013 at 07:37:11PM -0500, Theodore Ts'o wrote:
> The -b and -C options now use parse_num_blocks2() insteda of strtol,
> so that users can specify -C 256M instead of the much less convenient
> -C 268435456.
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>
Regards,
- Zheng
> ---
> misc/mke2fs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 5cb49b3..c16ab33 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -1324,10 +1324,10 @@ profile_error:
> "b:cg:i:jl:m:no:qr:s:t:vC:DE:FG:I:J:KL:M:N:O:R:ST:U:V")) != EOF) {
> switch (c) {
> case 'b':
> - blocksize = strtol(optarg, &tmp, 0);
> + blocksize = parse_num_blocks2(optarg, -1);
> b = (blocksize > 0) ? blocksize : -blocksize;
> if (b < EXT2_MIN_BLOCK_SIZE ||
> - b > EXT2_MAX_BLOCK_SIZE || *tmp) {
> + b > EXT2_MAX_BLOCK_SIZE) {
> com_err(program_name, 0,
> _("invalid block size - %s"), optarg);
> exit(1);
> @@ -1345,9 +1345,9 @@ profile_error:
> cflag++;
> break;
> case 'C':
> - cluster_size = strtoul(optarg, &tmp, 0);
> + cluster_size = parse_num_blocks2(optarg, -1);
> if (cluster_size <= EXT2_MIN_CLUSTER_SIZE ||
> - cluster_size > EXT2_MAX_CLUSTER_SIZE || *tmp) {
> + cluster_size > EXT2_MAX_CLUSTER_SIZE) {
> com_err(program_name, 0,
> _("invalid cluster size - %s"),
> optarg);
> --
> 1.7.12.rc0.22.gcdd159b
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size
2013-01-15 0:37 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
` (2 preceding siblings ...)
2013-01-15 0:37 ` [PATCH 4/5] mke2fs: teach mke2fs to understand -b 4k and -C 256M Theodore Ts'o
@ 2013-01-15 0:37 ` Theodore Ts'o
2013-01-15 15:33 ` Zheng Liu
2013-01-15 0:41 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
4 siblings, 1 reply; 32+ messages in thread
From: Theodore Ts'o @ 2013-01-15 0:37 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: gnehzuil.liu, Theodore Ts'o
If the user attemps to create a 512MB cluster, we need to adjust the
defaults to avoid a 32-bit overflow of s_blocks_per_group. Also check
to make sure that the caller of ext2fs_initialize() has not given a
value of s_clusters_per_group that would result in an overflow of
s_blocks_per_group.
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
lib/ext2fs/initialize.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/lib/ext2fs/initialize.c b/lib/ext2fs/initialize.c
index b0c15d2..5afdc27 100644
--- a/lib/ext2fs/initialize.c
+++ b/lib/ext2fs/initialize.c
@@ -207,6 +207,8 @@ errcode_t ext2fs_initialize(const char *name, int flags,
super->s_log_block_size;
if (bigalloc_flag) {
+ unsigned long long bpg;
+
if (param->s_blocks_per_group &&
param->s_clusters_per_group &&
((param->s_clusters_per_group * EXT2FS_CLUSTER_RATIO(fs)) !=
@@ -220,12 +222,19 @@ errcode_t ext2fs_initialize(const char *name, int flags,
super->s_clusters_per_group =
param->s_blocks_per_group /
EXT2FS_CLUSTER_RATIO(fs);
- else
+ else if (super->s_log_cluster_size + 15 < 32)
super->s_clusters_per_group = fs->blocksize * 8;
+ else
+ super->s_clusters_per_group = (fs->blocksize - 1) * 8;
if (super->s_clusters_per_group > EXT2_MAX_CLUSTERS_PER_GROUP(super))
super->s_clusters_per_group = EXT2_MAX_CLUSTERS_PER_GROUP(super);
- super->s_blocks_per_group = EXT2FS_C2B(fs,
- super->s_clusters_per_group);
+ bpg = EXT2FS_C2B(fs,
+ (unsigned long long) super->s_clusters_per_group);
+ if (bpg >= (((unsigned long long) 1) << 32)) {
+ retval = EXT2_ET_INVALID_ARGUMENT;
+ goto cleanup;
+ }
+ super->s_blocks_per_group = bpg;
} else {
set_field(s_blocks_per_group, fs->blocksize * 8);
if (super->s_blocks_per_group > EXT2_MAX_BLOCKS_PER_GROUP(super))
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size
2013-01-15 0:37 ` [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size Theodore Ts'o
@ 2013-01-15 15:33 ` Zheng Liu
2013-01-15 15:36 ` Zheng Liu
2013-01-15 19:10 ` Theodore Ts'o
0 siblings, 2 replies; 32+ messages in thread
From: Zheng Liu @ 2013-01-15 15:33 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
On Mon, Jan 14, 2013 at 07:37:12PM -0500, Theodore Ts'o wrote:
> If the user attemps to create a 512MB cluster, we need to adjust the
> defaults to avoid a 32-bit overflow of s_blocks_per_group. Also check
> to make sure that the caller of ext2fs_initialize() has not given a
> value of s_clusters_per_group that would result in an overflow of
> s_blocks_per_group.
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
The patch itself looks good to me. Feel free to add:
Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>
FWIW, I wonder why we need to add such complex logical to handle a
corner case. I guess no one wants to use a 512MB cluster. So changing
max cluster size from 512MB to 256MB is very simple and straightfoward.
Regards,
- Zheng
> ---
> lib/ext2fs/initialize.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/lib/ext2fs/initialize.c b/lib/ext2fs/initialize.c
> index b0c15d2..5afdc27 100644
> --- a/lib/ext2fs/initialize.c
> +++ b/lib/ext2fs/initialize.c
> @@ -207,6 +207,8 @@ errcode_t ext2fs_initialize(const char *name, int flags,
> super->s_log_block_size;
>
> if (bigalloc_flag) {
> + unsigned long long bpg;
> +
> if (param->s_blocks_per_group &&
> param->s_clusters_per_group &&
> ((param->s_clusters_per_group * EXT2FS_CLUSTER_RATIO(fs)) !=
> @@ -220,12 +222,19 @@ errcode_t ext2fs_initialize(const char *name, int flags,
> super->s_clusters_per_group =
> param->s_blocks_per_group /
> EXT2FS_CLUSTER_RATIO(fs);
> - else
> + else if (super->s_log_cluster_size + 15 < 32)
> super->s_clusters_per_group = fs->blocksize * 8;
> + else
> + super->s_clusters_per_group = (fs->blocksize - 1) * 8;
> if (super->s_clusters_per_group > EXT2_MAX_CLUSTERS_PER_GROUP(super))
> super->s_clusters_per_group = EXT2_MAX_CLUSTERS_PER_GROUP(super);
> - super->s_blocks_per_group = EXT2FS_C2B(fs,
> - super->s_clusters_per_group);
> + bpg = EXT2FS_C2B(fs,
> + (unsigned long long) super->s_clusters_per_group);
> + if (bpg >= (((unsigned long long) 1) << 32)) {
> + retval = EXT2_ET_INVALID_ARGUMENT;
> + goto cleanup;
> + }
> + super->s_blocks_per_group = bpg;
> } else {
> set_field(s_blocks_per_group, fs->blocksize * 8);
> if (super->s_blocks_per_group > EXT2_MAX_BLOCKS_PER_GROUP(super))
> --
> 1.7.12.rc0.22.gcdd159b
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size
2013-01-15 15:33 ` Zheng Liu
@ 2013-01-15 15:36 ` Zheng Liu
2013-01-15 19:10 ` Theodore Ts'o
1 sibling, 0 replies; 32+ messages in thread
From: Zheng Liu @ 2013-01-15 15:36 UTC (permalink / raw)
To: Theodore Ts'o, Ext4 Developers List
On Tue, Jan 15, 2013 at 11:33:31PM +0800, Zheng Liu wrote:
> On Mon, Jan 14, 2013 at 07:37:12PM -0500, Theodore Ts'o wrote:
> > If the user attemps to create a 512MB cluster, we need to adjust the
> > defaults to avoid a 32-bit overflow of s_blocks_per_group. Also check
> > to make sure that the caller of ext2fs_initialize() has not given a
> > value of s_clusters_per_group that would result in an overflow of
> > s_blocks_per_group.
> >
> > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>
> The patch itself looks good to me. Feel free to add:
> Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>
Sorry, forgot to say that '-C' option in manpage will need to be modified
from 256MB to 512MB if this patch is applied.
Thanks,
- Zheng
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size
2013-01-15 15:33 ` Zheng Liu
2013-01-15 15:36 ` Zheng Liu
@ 2013-01-15 19:10 ` Theodore Ts'o
2013-01-16 1:49 ` Zheng Liu
1 sibling, 1 reply; 32+ messages in thread
From: Theodore Ts'o @ 2013-01-15 19:10 UTC (permalink / raw)
To: Ext4 Developers List
On Tue, Jan 15, 2013 at 11:33:31PM +0800, Zheng Liu wrote:
>
> FWIW, I wonder why we need to add such complex logical to handle a
> corner case. I guess no one wants to use a 512MB cluster. So changing
> max cluster size from 512MB to 256MB is very simple and straightfoward.
I agree that it seems very unlikely that there would be much interest
in using a 512MB cluster. However, we would still need to do the
check in the case of a 16k block size (on an PowerPC or Itanium
system) at a 256MB or 128MB cluster. So part of this check would be
needed anyway.
- Ted
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size
2013-01-15 19:10 ` Theodore Ts'o
@ 2013-01-16 1:49 ` Zheng Liu
0 siblings, 0 replies; 32+ messages in thread
From: Zheng Liu @ 2013-01-16 1:49 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
On Tue, Jan 15, 2013 at 02:10:30PM -0500, Theodore Ts'o wrote:
> On Tue, Jan 15, 2013 at 11:33:31PM +0800, Zheng Liu wrote:
> >
> > FWIW, I wonder why we need to add such complex logical to handle a
> > corner case. I guess no one wants to use a 512MB cluster. So changing
> > max cluster size from 512MB to 256MB is very simple and straightfoward.
>
> I agree that it seems very unlikely that there would be much interest
> in using a 512MB cluster. However, we would still need to do the
> check in the case of a 16k block size (on an PowerPC or Itanium
> system) at a 256MB or 128MB cluster. So part of this check would be
> needed anyway.
Yeah, I see. Thanks for the explanation.
Regards,
- Zheng
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size
2013-01-15 0:37 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
` (3 preceding siblings ...)
2013-01-15 0:37 ` [PATCH 5/5] libext2fs: avoid 32-bit overflow in ext2fs_initialize with a 512M cluster size Theodore Ts'o
@ 2013-01-15 0:41 ` Theodore Ts'o
2013-01-15 15:22 ` Zheng Liu
4 siblings, 1 reply; 32+ messages in thread
From: Theodore Ts'o @ 2013-01-15 0:41 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: gnehzuil.liu
On Mon, Jan 14, 2013 at 07:37:08PM -0500, Theodore Ts'o wrote:
> + if (fs_param.s_log_cluster_size &&
> + fs_param.s_log_cluster_size < fs_param.s_log_block_size) {
> + com_err(program_name, 0,
> + _("The cluster size must not be "
> + "smaller than the block size.\n"));
This would read better: "the cluster size may not be smaller than the
block size"
- Ted
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size
2013-01-15 0:41 ` [PATCH 1/5] mke2fs: enforce that the cluster size must be less that the block size Theodore Ts'o
@ 2013-01-15 15:22 ` Zheng Liu
0 siblings, 0 replies; 32+ messages in thread
From: Zheng Liu @ 2013-01-15 15:22 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
On Mon, Jan 14, 2013 at 07:41:58PM -0500, Theodore Ts'o wrote:
> On Mon, Jan 14, 2013 at 07:37:08PM -0500, Theodore Ts'o wrote:
> > + if (fs_param.s_log_cluster_size &&
> > + fs_param.s_log_cluster_size < fs_param.s_log_block_size) {
> > + com_err(program_name, 0,
> > + _("The cluster size must not be "
> > + "smaller than the block size.\n"));
>
> This would read better: "the cluster size may not be smaller than the
> block size"
Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>
Regards,
- Zheng
^ permalink raw reply [flat|nested] 32+ messages in thread