diff mbox series

btrfs-progs: add slack space for mkfs --shrink

Message ID fbe3a75e21a89d8fb3013c55468de7fd03b5027e.1741651032.git.loemra.dev@gmail.com (mailing list archive)
State New
Headers show
Series btrfs-progs: add slack space for mkfs --shrink | expand

Commit Message

Leo Martins March 11, 2025, 12:10 a.m. UTC
This patch adds an optional argument to the 'mkfs --shrink' option
allowing users to specify slack when shrinking the filesystem.
Previously if you wanted to use --shrink and include extra space in the
filesystem you would need to use btrfs resize, however, this requires
mounting the filesystem which requires CAP_SYS_ADMIN.

The new syntax is:
mkfs.btrfs --shrink[=SLACK SIZE]

Where [SLACK SIZE] is an optional argument specifying the desired
slack size. If not provided, the default slack size is 0.

Signed-off-by: Leo Martins <loemra.dev@gmail.com>
---
 mkfs/main.c    | 15 +++++++++++----
 mkfs/rootdir.c | 15 ++++++++++++---
 mkfs/rootdir.h |  2 +-
 3 files changed, 24 insertions(+), 8 deletions(-)

Comments

David Sterba March 11, 2025, 9:24 p.m. UTC | #1
On Mon, Mar 10, 2025 at 05:10:31PM -0700, Leo Martins wrote:
> This patch adds an optional argument to the 'mkfs --shrink' option
                     ^^^^^^^^^^^^^^^^^
I'd rather avoid optional parameters, it's a bit better for long options
but still can be confusing. Definitely no optional parameters for short
options.  New long option is fine in this case.

> allowing users to specify slack when shrinking the filesystem.
> Previously if you wanted to use --shrink and include extra space in the
> filesystem you would need to use btrfs resize, however, this requires
> mounting the filesystem which requires CAP_SYS_ADMIN.

The use case makes sense.

> The new syntax is:
> mkfs.btrfs --shrink[=SLACK SIZE]
> 
> Where [SLACK SIZE] is an optional argument specifying the desired
> slack size. If not provided, the default slack size is 0.
> 
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> ---
>  mkfs/main.c    | 15 +++++++++++----
>  mkfs/rootdir.c | 15 ++++++++++++---
>  mkfs/rootdir.h |  2 +-
>  3 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/mkfs/main.c b/mkfs/main.c
> index dc73de47..11a5a4a9 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -460,7 +460,7 @@ static const char * const mkfs_usage[] = {
>  	OPTLINE("", "- ro - create the subvolume as read-only"),
>  	OPTLINE("", "- default - the SUBDIR will be a subvolume and also set as default (can be specified only once)"),
>  	OPTLINE("", "- default-ro - like 'default' and is created as read-only subvolume (can be specified only once)"),
> -	OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"),
> +	OPTLINE("--shrink[=SLACK SIZE]", "(with --rootdir) shrink the filled filesystem to minimal size, optionally include extra slack space after shrinking (default 0)"),
>  	OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"),
>  	OPTLINE("-f|--force", "force overwrite of existing filesystem"),
>  	"",
> @@ -1176,6 +1176,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>  	u64 source_dir_size = 0;
>  	u64 min_dev_size;
>  	u64 shrink_size;
> +	u64 shrink_slack_size = 0;
>  	int device_count = 0;
>  	int saved_optind;
>  	pthread_t *t_prepare = NULL;
> @@ -1246,7 +1247,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>  				GETOPT_VAL_DEVICE_UUID },
>  			{ "quiet", 0, NULL, 'q' },
>  			{ "verbose", 0, NULL, 'v' },
> -			{ "shrink", no_argument, NULL, GETOPT_VAL_SHRINK },
> +			{ "shrink", optional_argument, NULL, GETOPT_VAL_SHRINK },
>  			{ "compress", required_argument, NULL,
>  				GETOPT_VAL_COMPRESS },
>  #if EXPERIMENTAL
> @@ -1381,6 +1382,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>  				strncpy_null(dev_uuid, optarg, BTRFS_UUID_UNPARSED_SIZE);
>  				break;
>  			case GETOPT_VAL_SHRINK:
> +				shrink_slack_size = optarg == NULL ? 0 : arg_strtou64_with_suffix(optarg);

The value need to be validated and checked for alignment, something
close to what would work for resize.

>  				shrink_rootdir = true;
>  				break;
>  			case GETOPT_VAL_CHECKSUM:
> @@ -2107,9 +2109,14 @@ raid_groups:
>  		}
>  
>  		if (shrink_rootdir) {
> -			pr_verbose(LOG_DEFAULT, "  Shrink:           yes\n");
> +			pr_verbose(
> +				LOG_DEFAULT,
> +				"  Shrink:           yes          (slack size: %s)\n",

Please put it on a separate line if slack is > 0.

> +				pretty_size(shrink_slack_size));
>  			ret = btrfs_mkfs_shrink_fs(fs_info, &shrink_size,
> -						   shrink_rootdir);
> +						   shrink_rootdir,
> +						   shrink_slack_size);
> +
>  			if (ret < 0) {
>  				errno = -ret;
>  				error("error while shrinking filesystem: %m");
> diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
> index 19273947..176e6528 100644
> --- a/mkfs/rootdir.c
> +++ b/mkfs/rootdir.c
> @@ -1924,7 +1924,7 @@ err:
>  }
>  
> @@ -1948,14 +1948,23 @@ int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
>  		return ret;
>  	}
>  
> +	device = list_entry(fs_info->fs_devices->devices.next,
> +			   struct btrfs_device, dev_list);
> +
> +	new_size += slack_size;
> +	if (new_size > device->total_bytes) {
> +		warning("fs size with slack: %llu (%s) is larger than device size: %llu (%s)\n"
> +			"         consider decreasing slack size or increasing device size",

Please use "\t..." on the continuation line.

> +			new_size, pretty_size(new_size), device->total_bytes,
> +			pretty_size(device->total_bytes));
> +	}
> +
>  	if (!IS_ALIGNED(new_size, fs_info->sectorsize)) {
>  		error("shrunk filesystem size %llu not aligned to %u",
>  				new_size, fs_info->sectorsize);
>  		return -EUCLEAN;
>  	}
>  
> -	device = list_entry(fs_info->fs_devices->devices.next,
> -			   struct btrfs_device, dev_list);
>  	ret = set_device_size(fs_info, device, new_size);
>  	if (ret < 0)
>  		return ret;
Qu Wenruo March 11, 2025, 9:33 p.m. UTC | #2
在 2025/3/11 10:40, Leo Martins 写道:
> This patch adds an optional argument to the 'mkfs --shrink' option
> allowing users to specify slack when shrinking the filesystem.
> Previously if you wanted to use --shrink and include extra space in the
> filesystem you would need to use btrfs resize, however, this requires
> mounting the filesystem which requires CAP_SYS_ADMIN.
> 
> The new syntax is:
> mkfs.btrfs --shrink[=SLACK SIZE]
> 
> Where [SLACK SIZE] is an optional argument specifying the desired
> slack size. If not provided, the default slack size is 0.
> 
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>

David has already commented on the UI part, thus I'll only focus on the 
implementation details.

[..]
>   
>   int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
> -			 bool shrink_file_size)
> +			 bool shrink_file_size, u64 slack_size)
>   {
>   	u64 new_size;
>   	struct btrfs_device *device;
> @@ -1948,14 +1948,23 @@ int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
>   		return ret;
>   	}
>   
> +	device = list_entry(fs_info->fs_devices->devices.next,
> +			   struct btrfs_device, dev_list);
> +
> +	new_size += slack_size;
> +	if (new_size > device->total_bytes) {
> +		warning("fs size with slack: %llu (%s) is larger than device size: %llu (%s)\n"
> +			"         consider decreasing slack size or increasing device size",
> +			new_size, pretty_size(new_size), device->total_bytes,
> +			pretty_size(device->total_bytes));
> +	}
> +
>   	if (!IS_ALIGNED(new_size, fs_info->sectorsize)) {
>   		error("shrunk filesystem size %llu not aligned to %u",
>   				new_size, fs_info->sectorsize);
>   		return -EUCLEAN;
>   	}

If the slack value is not aligned to the fs block size (sector size) it 
will trigger the error here.
And since the new size is based on the older size with the slack, it's 
better to do an extra check on the slack_size itself, so that the end 
user will be properly notified if the unaligned value is introduced by 
the slack.

Thanks,
Qu

>   
> -	device = list_entry(fs_info->fs_devices->devices.next,
> -			   struct btrfs_device, dev_list);
>   	ret = set_device_size(fs_info, device, new_size);
>   	if (ret < 0)
>   		return ret;
> diff --git a/mkfs/rootdir.h b/mkfs/rootdir.h
> index b32fda5b..1eee3824 100644
> --- a/mkfs/rootdir.h
> +++ b/mkfs/rootdir.h
> @@ -52,6 +52,6 @@ int btrfs_mkfs_fill_dir(struct btrfs_trans_handle *trans, const char *source_dir
>   u64 btrfs_mkfs_size_dir(const char *dir_name, u32 sectorsize, u64 min_dev_size,
>   			u64 meta_profile, u64 data_profile);
>   int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
> -			 bool shrink_file_size);
> +			 bool shrink_file_size, u64 slack_size);
>   
>   #endif
diff mbox series

Patch

diff --git a/mkfs/main.c b/mkfs/main.c
index dc73de47..11a5a4a9 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -460,7 +460,7 @@  static const char * const mkfs_usage[] = {
 	OPTLINE("", "- ro - create the subvolume as read-only"),
 	OPTLINE("", "- default - the SUBDIR will be a subvolume and also set as default (can be specified only once)"),
 	OPTLINE("", "- default-ro - like 'default' and is created as read-only subvolume (can be specified only once)"),
-	OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"),
+	OPTLINE("--shrink[=SLACK SIZE]", "(with --rootdir) shrink the filled filesystem to minimal size, optionally include extra slack space after shrinking (default 0)"),
 	OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"),
 	OPTLINE("-f|--force", "force overwrite of existing filesystem"),
 	"",
@@ -1176,6 +1176,7 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 	u64 source_dir_size = 0;
 	u64 min_dev_size;
 	u64 shrink_size;
+	u64 shrink_slack_size = 0;
 	int device_count = 0;
 	int saved_optind;
 	pthread_t *t_prepare = NULL;
@@ -1246,7 +1247,7 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 				GETOPT_VAL_DEVICE_UUID },
 			{ "quiet", 0, NULL, 'q' },
 			{ "verbose", 0, NULL, 'v' },
-			{ "shrink", no_argument, NULL, GETOPT_VAL_SHRINK },
+			{ "shrink", optional_argument, NULL, GETOPT_VAL_SHRINK },
 			{ "compress", required_argument, NULL,
 				GETOPT_VAL_COMPRESS },
 #if EXPERIMENTAL
@@ -1381,6 +1382,7 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 				strncpy_null(dev_uuid, optarg, BTRFS_UUID_UNPARSED_SIZE);
 				break;
 			case GETOPT_VAL_SHRINK:
+				shrink_slack_size = optarg == NULL ? 0 : arg_strtou64_with_suffix(optarg);
 				shrink_rootdir = true;
 				break;
 			case GETOPT_VAL_CHECKSUM:
@@ -2107,9 +2109,14 @@  raid_groups:
 		}
 
 		if (shrink_rootdir) {
-			pr_verbose(LOG_DEFAULT, "  Shrink:           yes\n");
+			pr_verbose(
+				LOG_DEFAULT,
+				"  Shrink:           yes          (slack size: %s)\n",
+				pretty_size(shrink_slack_size));
 			ret = btrfs_mkfs_shrink_fs(fs_info, &shrink_size,
-						   shrink_rootdir);
+						   shrink_rootdir,
+						   shrink_slack_size);
+
 			if (ret < 0) {
 				errno = -ret;
 				error("error while shrinking filesystem: %m");
diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
index 19273947..176e6528 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -1924,7 +1924,7 @@  err:
 }
 
 int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
-			 bool shrink_file_size)
+			 bool shrink_file_size, u64 slack_size)
 {
 	u64 new_size;
 	struct btrfs_device *device;
@@ -1948,14 +1948,23 @@  int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
 		return ret;
 	}
 
+	device = list_entry(fs_info->fs_devices->devices.next,
+			   struct btrfs_device, dev_list);
+
+	new_size += slack_size;
+	if (new_size > device->total_bytes) {
+		warning("fs size with slack: %llu (%s) is larger than device size: %llu (%s)\n"
+			"         consider decreasing slack size or increasing device size",
+			new_size, pretty_size(new_size), device->total_bytes,
+			pretty_size(device->total_bytes));
+	}
+
 	if (!IS_ALIGNED(new_size, fs_info->sectorsize)) {
 		error("shrunk filesystem size %llu not aligned to %u",
 				new_size, fs_info->sectorsize);
 		return -EUCLEAN;
 	}
 
-	device = list_entry(fs_info->fs_devices->devices.next,
-			   struct btrfs_device, dev_list);
 	ret = set_device_size(fs_info, device, new_size);
 	if (ret < 0)
 		return ret;
diff --git a/mkfs/rootdir.h b/mkfs/rootdir.h
index b32fda5b..1eee3824 100644
--- a/mkfs/rootdir.h
+++ b/mkfs/rootdir.h
@@ -52,6 +52,6 @@  int btrfs_mkfs_fill_dir(struct btrfs_trans_handle *trans, const char *source_dir
 u64 btrfs_mkfs_size_dir(const char *dir_name, u32 sectorsize, u64 min_dev_size,
 			u64 meta_profile, u64 data_profile);
 int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
-			 bool shrink_file_size);
+			 bool shrink_file_size, u64 slack_size);
 
 #endif