[PATCHv2 0/2] btrfs-progs: Auto resize fs after device replace

[PATCHv2 0/2] btrfs-progs: Auto resize fs after device replace

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

Changes from v1:
* Reworded the help message and the docs telling the user that the fs will be
  resized to it's max size (suggested by Wenruo)
* Added a warning message saying that the resize failed, asking the user to
  resize manually. (suggested by Wenruo)

Both changes were done only in patch 0002.

Anand suggested this job to be done in kernel side, atomically, but as I
received a good review from Wenruo I decided to send a v2 of this patchset.

Please review, thanks!

Original cover-letter[1]:
These two patches make possible to resize the fs after a successful replace
finishes. The flag -a is responsible for doing it (-r is already use, so -a in
this context means "automatically").

The first patch just moves the resize rationale to utils.c and the second patch
adds the flag an calls resize if -a is informed replace finishes successfully.

Please review!

[1]: https://lore.kernel.org/linux-btrfs/20200307224516.16315-1-marcos@mpdesouza.com/T/#t

Marcos Paulo de Souza (2):
  btrfs-progs: Move resize into functionaly into utils.c
  btrfs-progs: replace: New argument to resize the fs after replace

 Documentation/btrfs-replace.asciidoc |  5 ++-
 cmds/filesystem.c                    | 58 +--------------------------
 cmds/replace.c                       | 21 +++++++++-
 common/utils.c                       | 60 ++++++++++++++++++++++++++++
 common/utils.h                       |  1 +
 5 files changed, 86 insertions(+), 59 deletions(-)

-- 
2.25.0

[PATCHv2 1/2] btrfs-progs: Move resize into functionaly into utils.c

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This function will be used in the next patch to auto resize the
filesystem when a bigger disk is added.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 cmds/filesystem.c | 58 ++-------------------------------------------
 common/utils.c    | 60 +++++++++++++++++++++++++++++++++++++++++++++++
 common/utils.h    |  1 +
 3 files changed, 63 insertions(+), 56 deletions(-)

diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 4f22089a..9d31f236 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -1074,11 +1074,7 @@ static const char * const cmd_filesystem_resize_usage[] = {
 static int cmd_filesystem_resize(const struct cmd_struct *cmd,
 				 int argc, char **argv)
 {
-	struct btrfs_ioctl_vol_args	args;
-	int	fd, res, len, e;
-	char	*amount, *path;
-	DIR	*dirstream = NULL;
-	struct stat st;
+	char *amount, *path;
 
 	clean_args_no_options_relaxed(cmd, argc, argv);
 
@@ -1088,57 +1084,7 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
 	amount = argv[optind];
 	path = argv[optind + 1];
 
-	len = strlen(amount);
-	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
-		error("resize value too long (%s)", amount);
-		return 1;
-	}
-
-	res = stat(path, &st);
-	if (res < 0) {
-		error("resize: cannot stat %s: %m", path);
-		return 1;
-	}
-	if (!S_ISDIR(st.st_mode)) {
-		error("resize works on mounted filesystems and accepts only\n"
-			"directories as argument. Passing file containing a btrfs image\n"
-			"would resize the underlying filesystem instead of the image.\n");
-		return 1;
-	}
-
-	fd = btrfs_open_dir(path, &dirstream, 1);
-	if (fd < 0)
-		return 1;
-
-	printf("Resize '%s' of '%s'\n", path, amount);
-	memset(&args, 0, sizeof(args));
-	strncpy_null(args.name, amount);
-	res = ioctl(fd, BTRFS_IOC_RESIZE, &args);
-	e = errno;
-	close_file_or_dir(fd, dirstream);
-	if( res < 0 ){
-		switch (e) {
-		case EFBIG:
-			error("unable to resize '%s': no enough free space",
-				path);
-			break;
-		default:
-			error("unable to resize '%s': %m", path);
-			break;
-		}
-		return 1;
-	} else if (res > 0) {
-		const char *err_str = btrfs_err_str(res);
-
-		if (err_str) {
-			error("resizing of '%s' failed: %s", path, err_str);
-		} else {
-			error("resizing of '%s' failed: unknown error %d",
-				path, res);
-		}
-		return 1;
-	}
-	return 0;
+	return resize_filesystem(amount, path);
 }
 static DEFINE_SIMPLE_COMMAND(filesystem_resize, "resize");
 
diff --git a/common/utils.c b/common/utils.c
index 4ce36836..dddf0a6f 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -461,6 +461,66 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_size, unsigned unit_mod
 	return snprintf(str, str_size, "%.2f%s", fraction, suffix[num_divs]);
 }
 
+int resize_filesystem(const char *amount, const char *path)
+{
+	struct btrfs_ioctl_vol_args	args;
+	int	fd, res, len, e;
+	DIR	*dirstream = NULL;
+	struct stat st;
+
+	len = strlen(amount);
+	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
+		error("resize value too long (%s)", amount);
+		return 1;
+	}
+
+	res = stat(path, &st);
+	if (res < 0) {
+		error("resize: cannot stat %s: %m", path);
+		return 1;
+	}
+	if (!S_ISDIR(st.st_mode)) {
+		error("resize works on mounted filesystems and accepts only\n"
+			"directories as argument. Passing file containing a btrfs image\n"
+			"would resize the underlying filesystem instead of the image.\n");
+		return 1;
+	}
+
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
+		return 1;
+
+	printf("Resize '%s' of '%s'\n", path, amount);
+	memset(&args, 0, sizeof(args));
+	strncpy_null(args.name, amount);
+	res = ioctl(fd, BTRFS_IOC_RESIZE, &args);
+	e = errno;
+	close_file_or_dir(fd, dirstream);
+	if( res < 0 ){
+		switch (e) {
+		case EFBIG:
+			error("unable to resize '%s': no enough free space",
+				path);
+			break;
+		default:
+			error("unable to resize '%s': %m", path);
+			break;
+		}
+		return 1;
+	} else if (res > 0) {
+		const char *err_str = btrfs_err_str(res);
+
+		if (err_str) {
+			error("resizing of '%s' failed: %s", path, err_str);
+		} else {
+			error("resizing of '%s' failed: unknown error %d",
+				path, res);
+		}
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * Checks to make sure that the label matches our requirements.
  * Returns:
diff --git a/common/utils.h b/common/utils.h
index 5c1afda9..8609d3c9 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -62,6 +62,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
 		struct btrfs_fs_devices **fs_devices_mnt, unsigned sbflags);
 
 int pretty_size_snprintf(u64 size, char *str, size_t str_bytes, unsigned unit_mode);
+int resize_filesystem(const char *amount, const char *path);
 #define pretty_size(size) 	pretty_size_mode(size, UNITS_DEFAULT)
 const char *pretty_size_mode(u64 size, unsigned mode);
 
-- 
2.25.0

[PATCHv2 2/2] btrfs-progs: replace: New argument to resize the fs after replace

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

By using the -a flag on replace makes btrfs issue a resize ioctl after
the replace finishes. This argument is a shortcut for

btrfs replace start -f 3 /dev/sdf BTRFS/
btrfs fi resize 3:max BTRFS/

The -a stands for "automatically resize"

Fixes: #21

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Changes from v1:
* Reworded the help message and the docs telling the user that the fs will be
  resized to it's max size (suggested by Wenruo)
* Added a warning message saying that the resize failed, asking the user to
  resize manually. (suggested by Wenruo)

 Documentation/btrfs-replace.asciidoc |  5 ++++-
 cmds/replace.c                       | 21 +++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/btrfs-replace.asciidoc b/Documentation/btrfs-replace.asciidoc
index b73bf1b3..07cf2ae9 100644
--- a/Documentation/btrfs-replace.asciidoc
+++ b/Documentation/btrfs-replace.asciidoc
@@ -18,7 +18,7 @@ SUBCOMMAND
 *cancel* <mount_point>::
 Cancel a running device replace operation.
 
-*start* [-Bfr] <srcdev>|<devid> <targetdev> <path>::
+*start* [-aBfr] <srcdev>|<devid> <targetdev> <path>::
 Replace device of a btrfs filesystem.
 +
 On a live filesystem, duplicate the data to the target device which
@@ -53,6 +53,9 @@ never allowed to be used as the <targetdev>.
 +
 -B::::
 no background replace.
++a::::
+automatically resizes the filesystem to it's max size if the <targetdev> is
+bigger than <srcdev>.
 
 *status* [-1] <mount_point>::
 Print status and progress information of a running device replace operation.
diff --git a/cmds/replace.c b/cmds/replace.c
index 2321aa15..8761bfc0 100644
--- a/cmds/replace.c
+++ b/cmds/replace.c
@@ -91,7 +91,7 @@ static int dev_replace_handle_sigint(int fd)
 }
 
 static const char *const cmd_replace_start_usage[] = {
-	"btrfs replace start [-Bfr] <srcdev>|<devid> <targetdev> <mount_point>",
+	"btrfs replace start [-aBfr] <srcdev>|<devid> <targetdev> <mount_point>",
 	"Replace device of a btrfs filesystem.",
 	"On a live filesystem, duplicate the data to the target device which",
 	"is currently stored on the source device. If the source device is not",
@@ -104,6 +104,8 @@ static const char *const cmd_replace_start_usage[] = {
 	"from the system, you have to use the <devid> parameter format.",
 	"The <targetdev> needs to be same size or larger than the <srcdev>.",
 	"",
+	"-a     automatically resizes the filesystem to it's max size if the",
+	"       <targetdev> is bigger than <srcdev>",
 	"-r     only read from <srcdev> if no other zero-defect mirror exists",
 	"       (enable this if your drive has lots of read errors, the access",
 	"       would be very slow)",
@@ -129,6 +131,7 @@ static int cmd_replace_start(const struct cmd_struct *cmd,
 	char *path;
 	char *srcdev;
 	char *dstdev = NULL;
+	bool auto_resize = false;
 	int avoid_reading_from_srcdev = 0;
 	int force_using_targetdev = 0;
 	u64 dstdev_block_count;
@@ -138,8 +141,11 @@ static int cmd_replace_start(const struct cmd_struct *cmd,
 	u64 dstdev_size;
 
 	optind = 0;
-	while ((c = getopt(argc, argv, "Brf")) != -1) {
+	while ((c = getopt(argc, argv, "aBrf")) != -1) {
 		switch (c) {
+		case 'a':
+			auto_resize = true;
+			break;
 		case 'B':
 			do_not_background = 1;
 			break;
@@ -309,6 +315,17 @@ static int cmd_replace_start(const struct cmd_struct *cmd,
 			goto leave_with_error;
 		}
 	}
+
+	if (ret == 0 && auto_resize && dstdev_size > srcdev_size) {
+		char amount[BTRFS_PATH_NAME_MAX + 1];
+		snprintf(amount, BTRFS_PATH_NAME_MAX, "%s:max", srcdev);
+
+		if (resize_filesystem(amount, path)) {
+			warning("resize failed, please resize the filesystem manually");
+			goto leave_with_error;
+		}
+	}
+
 	close_file_or_dir(fdmnt, dirstream);
 	return 0;
 
-- 
2.25.0

Re: [PATCHv2 1/2] btrfs-progs: Move resize into functionaly into utils.c

[David Sterba]

On Fri, Mar 20, 2020 at 12:32:26AM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> This function will be used in the next patch to auto resize the
> filesystem when a bigger disk is added.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  cmds/filesystem.c | 58 ++-------------------------------------------
>  common/utils.c    | 60 +++++++++++++++++++++++++++++++++++++++++++++++
>  common/utils.h    |  1 +
>  3 files changed, 63 insertions(+), 56 deletions(-)
> 
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index 4f22089a..9d31f236 100644
> --- a/cmds/filesystem.c
> +++ b/cmds/filesystem.c
> @@ -1074,11 +1074,7 @@ static const char * const cmd_filesystem_resize_usage[] = {
>  static int cmd_filesystem_resize(const struct cmd_struct *cmd,
>  				 int argc, char **argv)
>  {
> -	struct btrfs_ioctl_vol_args	args;
> -	int	fd, res, len, e;
> -	char	*amount, *path;
> -	DIR	*dirstream = NULL;
> -	struct stat st;
> +	char *amount, *path;
>  
>  	clean_args_no_options_relaxed(cmd, argc, argv);
>  
> @@ -1088,57 +1084,7 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
>  	amount = argv[optind];
>  	path = argv[optind + 1];
>  
> -	len = strlen(amount);
> -	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
> -		error("resize value too long (%s)", amount);
> -		return 1;
> -	}
> -
> -	res = stat(path, &st);
> -	if (res < 0) {
> -		error("resize: cannot stat %s: %m", path);
> -		return 1;
> -	}
> -	if (!S_ISDIR(st.st_mode)) {
> -		error("resize works on mounted filesystems and accepts only\n"
> -			"directories as argument. Passing file containing a btrfs image\n"
> -			"would resize the underlying filesystem instead of the image.\n");
> -		return 1;
> -	}
> -
> -	fd = btrfs_open_dir(path, &dirstream, 1);
> -	if (fd < 0)
> -		return 1;
> -
> -	printf("Resize '%s' of '%s'\n", path, amount);
> -	memset(&args, 0, sizeof(args));
> -	strncpy_null(args.name, amount);
> -	res = ioctl(fd, BTRFS_IOC_RESIZE, &args);
> -	e = errno;
> -	close_file_or_dir(fd, dirstream);
> -	if( res < 0 ){
> -		switch (e) {
> -		case EFBIG:
> -			error("unable to resize '%s': no enough free space",
> -				path);
> -			break;
> -		default:
> -			error("unable to resize '%s': %m", path);
> -			break;
> -		}
> -		return 1;
> -	} else if (res > 0) {
> -		const char *err_str = btrfs_err_str(res);
> -
> -		if (err_str) {
> -			error("resizing of '%s' failed: %s", path, err_str);
> -		} else {
> -			error("resizing of '%s' failed: unknown error %d",
> -				path, res);
> -		}
> -		return 1;
> -	}
> -	return 0;
> +	return resize_filesystem(amount, path);
>  }
>  static DEFINE_SIMPLE_COMMAND(filesystem_resize, "resize");
>  
> diff --git a/common/utils.c b/common/utils.c
> index 4ce36836..dddf0a6f 100644
> --- a/common/utils.c
> +++ b/common/utils.c
> @@ -461,6 +461,66 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_size, unsigned unit_mod
>  	return snprintf(str, str_size, "%.2f%s", fraction, suffix[num_divs]);
>  }
>  
> +int resize_filesystem(const char *amount, const char *path)
> +{
> +	struct btrfs_ioctl_vol_args	args;
> +	int	fd, res, len, e;
> +	DIR	*dirstream = NULL;

Please fix the code formatting in code that's being moved

> +	struct stat st;
> +
> +	len = strlen(amount);
> +	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
> +		error("resize value too long (%s)", amount);
> +		return 1;
> +	}
> +
> +	res = stat(path, &st);
> +	if (res < 0) {
> +		error("resize: cannot stat %s: %m", path);
> +		return 1;
> +	}
> +	if (!S_ISDIR(st.st_mode)) {
> +		error("resize works on mounted filesystems and accepts only\n"
> +			"directories as argument. Passing file containing a btrfs image\n"
> +			"would resize the underlying filesystem instead of the image.\n");
> +		return 1;
> +	}
> +
> +	fd = btrfs_open_dir(path, &dirstream, 1);
> +	if (fd < 0)
> +		return 1;
> +
> +	printf("Resize '%s' of '%s'\n", path, amount);
> +	memset(&args, 0, sizeof(args));
> +	strncpy_null(args.name, amount);
> +	res = ioctl(fd, BTRFS_IOC_RESIZE, &args);
> +	e = errno;
> +	close_file_or_dir(fd, dirstream);
> +	if( res < 0 ){
> +		switch (e) {
> +		case EFBIG:
> +			error("unable to resize '%s': no enough free space",
> +				path);
> +			break;
> +		default:
> +			error("unable to resize '%s': %m", path);
> +			break;
> +		}
> +		return 1;
> +	} else if (res > 0) {
> +		const char *err_str = btrfs_err_str(res);
> +
> +		if (err_str) {
> +			error("resizing of '%s' failed: %s", path, err_str);
> +		} else {
> +			error("resizing of '%s' failed: unknown error %d",
> +				path, res);
> +		}
> +		return 1;
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Checks to make sure that the label matches our requirements.
>   * Returns:
> diff --git a/common/utils.h b/common/utils.h
> index 5c1afda9..8609d3c9 100644
> --- a/common/utils.h
> +++ b/common/utils.h
> @@ -62,6 +62,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
>  		struct btrfs_fs_devices **fs_devices_mnt, unsigned sbflags);
>  
>  int pretty_size_snprintf(u64 size, char *str, size_t str_bytes, unsigned unit_mode);
> +int resize_filesystem(const char *amount, const char *path);

This is a bad placement of the prototype, in the middle of functions
that pretty print values. The file utils.h is historically unsorted but
new code should make the things better when possible.

>  #define pretty_size(size) 	pretty_size_mode(size, UNITS_DEFAULT)
>  const char *pretty_size_mode(u64 size, unsigned mode);
>  
> -- 
> 2.25.0

Re: [PATCHv2 2/2] btrfs-progs: replace: New argument to resize the fs after replace

[David Sterba]

On Fri, Mar 20, 2020 at 12:32:27AM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> By using the -a flag on replace makes btrfs issue a resize ioctl after
> the replace finishes. This argument is a shortcut for
> 
> btrfs replace start -f 3 /dev/sdf BTRFS/
> btrfs fi resize 3:max BTRFS/
> 
> The -a stands for "automatically resize"

The single letters are reserved for the most common actions, the long
option is always safe and it's easy to add the short option later. In
case we'd like to make the auto-resize default, the 'no-' variant of
the long option would be good from the begining too.

> Fixes: #21
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> Changes from v1:
> * Reworded the help message and the docs telling the user that the fs will be
>   resized to it's max size (suggested by Wenruo)
> * Added a warning message saying that the resize failed, asking the user to
>   resize manually. (suggested by Wenruo)
> 
>  Documentation/btrfs-replace.asciidoc |  5 ++++-
>  cmds/replace.c                       | 21 +++++++++++++++++++--
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/btrfs-replace.asciidoc b/Documentation/btrfs-replace.asciidoc
> index b73bf1b3..07cf2ae9 100644
> --- a/Documentation/btrfs-replace.asciidoc
> +++ b/Documentation/btrfs-replace.asciidoc
> @@ -18,7 +18,7 @@ SUBCOMMAND
>  *cancel* <mount_point>::
>  Cancel a running device replace operation.
>  
> -*start* [-Bfr] <srcdev>|<devid> <targetdev> <path>::
> +*start* [-aBfr] <srcdev>|<devid> <targetdev> <path>::
>  Replace device of a btrfs filesystem.
>  +
>  On a live filesystem, duplicate the data to the target device which
> @@ -53,6 +53,9 @@ never allowed to be used as the <targetdev>.
>  +
>  -B::::
>  no background replace.
> ++a::::
> +automatically resizes the filesystem to it's max size if the <targetdev> is
> +bigger than <srcdev>.
>  
>  *status* [-1] <mount_point>::
>  Print status and progress information of a running device replace operation.
> diff --git a/cmds/replace.c b/cmds/replace.c
> index 2321aa15..8761bfc0 100644
> --- a/cmds/replace.c
> +++ b/cmds/replace.c
> @@ -91,7 +91,7 @@ static int dev_replace_handle_sigint(int fd)
>  }
>  
>  static const char *const cmd_replace_start_usage[] = {
> -	"btrfs replace start [-Bfr] <srcdev>|<devid> <targetdev> <mount_point>",
> +	"btrfs replace start [-aBfr] <srcdev>|<devid> <targetdev> <mount_point>",
>  	"Replace device of a btrfs filesystem.",
>  	"On a live filesystem, duplicate the data to the target device which",
>  	"is currently stored on the source device. If the source device is not",
> @@ -104,6 +104,8 @@ static const char *const cmd_replace_start_usage[] = {
>  	"from the system, you have to use the <devid> parameter format.",
>  	"The <targetdev> needs to be same size or larger than the <srcdev>.",
>  	"",
> +	"-a     automatically resizes the filesystem to it's max size if the",
> +	"       <targetdev> is bigger than <srcdev>",
>  	"-r     only read from <srcdev> if no other zero-defect mirror exists",
>  	"       (enable this if your drive has lots of read errors, the access",
>  	"       would be very slow)",
> @@ -129,6 +131,7 @@ static int cmd_replace_start(const struct cmd_struct *cmd,
>  	char *path;
>  	char *srcdev;
>  	char *dstdev = NULL;
> +	bool auto_resize = false;
>  	int avoid_reading_from_srcdev = 0;
>  	int force_using_targetdev = 0;
>  	u64 dstdev_block_count;
> @@ -138,8 +141,11 @@ static int cmd_replace_start(const struct cmd_struct *cmd,
>  	u64 dstdev_size;
>  
>  	optind = 0;
> -	while ((c = getopt(argc, argv, "Brf")) != -1) {
> +	while ((c = getopt(argc, argv, "aBrf")) != -1) {
>  		switch (c) {
> +		case 'a':
> +			auto_resize = true;
> +			break;
>  		case 'B':
>  			do_not_background = 1;
>  			break;
> @@ -309,6 +315,17 @@ static int cmd_replace_start(const struct cmd_struct *cmd,
>  			goto leave_with_error;
>  		}
>  	}
> +
> +	if (ret == 0 && auto_resize && dstdev_size > srcdev_size) {
> +		char amount[BTRFS_PATH_NAME_MAX + 1];
> +		snprintf(amount, BTRFS_PATH_NAME_MAX, "%s:max", srcdev);

The format needs the device id, not the path, so for the path-based
source device it needs to be translated.

> +
> +		if (resize_filesystem(amount, path)) {
> +			warning("resize failed, please resize the filesystem manually");

The warning is fine but should also print the exact command line or at
least the important bits.

> +			goto leave_with_error;
> +		}
> +	}
> +
>  	close_file_or_dir(fdmnt, dirstream);
>  	return 0;
>  
> -- 
> 2.25.0