[PATCH 1/2] btrfs-progs: limit the max value of leafsize and nodesize

[PATCH 1/2] btrfs-progs: limit the max value of leafsize and nodesize

[Robin Dong]

From: Robin Dong <sanbai@taobao.com>

Using mkfs.btrfs like:

	mkfs.btrfs -l 131072 /dev/sda

will return no error, but after mount it, the dmesg will report:

	BTRFS: couldn't mount because metadata blocksize (131072) was too large

The user tools should use BTRFS_MAX_METADATA_BLOCKSIZE to limit leaf and node size.

Signed-off-by: Robin Dong <sanbai@taobao.com>
---
 ctree.h |    6 ++++++
 mkfs.c  |    6 ++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ctree.h b/ctree.h
index 7f55229..75c1e0a 100644
--- a/ctree.h
+++ b/ctree.h
@@ -111,6 +111,12 @@ struct btrfs_trans_handle;
 #define BTRFS_DEV_ITEMS_OBJECTID 1ULL
 
 /*
+ * the max metadata block size.  This limit is somewhat artificial,
+ * but the memmove costs go through the roof for larger blocks.
+ */
+#define BTRFS_MAX_METADATA_BLOCKSIZE 65536
+
+/*
  * we can actually store much bigger names, but lets not confuse the rest
  * of linux
  */
diff --git a/mkfs.c b/mkfs.c
index dff5eb8..bb01f64 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1291,11 +1291,13 @@ int main(int ac, char **av)
 		}
 	}
 	sectorsize = max(sectorsize, (u32)getpagesize());
-	if (leafsize < sectorsize || (leafsize & (sectorsize - 1))) {
+	if (leafsize < sectorsize || leafsize > BTRFS_MAX_METADATA_BLOCKSIZE ||
+			(leafsize & (sectorsize - 1))) {
 		fprintf(stderr, "Illegal leafsize %u\n", leafsize);
 		exit(1);
 	}
-	if (nodesize < sectorsize || (nodesize & (sectorsize - 1))) {
+	if (nodesize < sectorsize || nodesize > BTRFS_MAX_METADATA_BLOCKSIZE ||
+			(nodesize & (sectorsize - 1))) {
 		fprintf(stderr, "Illegal nodesize %u\n", nodesize);
 		exit(1);
 	}
-- 
1.7.3.2

[PATCH 2/2] btrfs-progs: limit the min value of total_bytes

[Robin Dong]

From: Robin Dong <sanbai@taobao.com>

Using mkfs.btrfs like:
	
	mkfs.btrfs -b 1048576 /dev/sda

will report error:

	mkfs.btrfs: volumes.c:796: btrfs_alloc_chunk: Assertion `!(ret)' failed.
	Aborted

because the length of dev_extent is 4MB.

But if we use mkfs.btrfs with 8MB total bytes, the newly mounted btrfs filesystem
would not contain even one empty file. So 12MB will be good min value for block_count.

Signed-off-by: Robin Dong <sanbai@taobao.com>
---
 mkfs.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/mkfs.c b/mkfs.c
index bb01f64..23bde2d 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1330,7 +1330,11 @@ int main(int ac, char **av)
 				&dev_block_count, &mixed, nodiscard);
 		if (block_count == 0)
 			block_count = dev_block_count;
-		else if (block_count > dev_block_count) {
+		else if (block_count < 3 * BTRFS_MKFS_SYSTEM_GROUP_SIZE) {
+			fprintf(stderr, "Illegal total number of bytes %u\n",
+					block_count);
+			exit(1);
+		} else if (block_count > dev_block_count) {
 			fprintf(stderr, "%s is smaller than requested size\n", file);
 			exit(1);
 		}
-- 
1.7.3.2

Re: [PATCH 1/2] btrfs-progs: limit the max value of leafsize and nodesize

[David Sterba]

On Wed, Sep 26, 2012 at 03:52:07PM +0800, Robin Dong wrote:
> Using mkfs.btrfs like:
> 	mkfs.btrfs -l 131072 /dev/sda
> 
> will return no error, but after mount it, the dmesg will report:
> 	BTRFS: couldn't mount because metadata blocksize (131072) was too large
> 
> The user tools should use BTRFS_MAX_METADATA_BLOCKSIZE to limit leaf and node size.

Good catch.

> @@ -1291,11 +1291,13 @@ int main(int ac, char **av)
>  		}
>  	}
>  	sectorsize = max(sectorsize, (u32)getpagesize());
> -	if (leafsize < sectorsize || (leafsize & (sectorsize - 1))) {
> +	if (leafsize < sectorsize || leafsize > BTRFS_MAX_METADATA_BLOCKSIZE ||
> +			(leafsize & (sectorsize - 1))) {

Could you please separate the BTRFS_MAX_METADATA_BLOCKSIZE check and add
appropriate error message that actually informs the user what kind of
error happened?

>  		fprintf(stderr, "Illegal leafsize %u\n", leafsize);
>  		exit(1);
>  	}
> -	if (nodesize < sectorsize || (nodesize & (sectorsize - 1))) {
> +	if (nodesize < sectorsize || nodesize > BTRFS_MAX_METADATA_BLOCKSIZE ||
> +			(nodesize & (sectorsize - 1))) {

(same here)

>  		fprintf(stderr, "Illegal nodesize %u\n", nodesize);
>  		exit(1);
>  	}

Thanks!
david