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

Commit Message

Robin Dong Sept. 26, 2012, 7:52 a.m. UTC

From: Robin Dong <[e-mail]>

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 <[e-mail]>
---
 ctree.h |    6 ++++++
 mkfs.c  |    6 ++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

David Sterba Sept. 27, 2012, 8:54 a.m. UTC | #1

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

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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);
 	}