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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4162 bytes --]



On 2020/3/8 上午6:45, 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"
> 
> Fixes: #21
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  Documentation/btrfs-replace.asciidoc |  4 +++-
>  cmds/replace.c                       | 19 +++++++++++++++++--
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/btrfs-replace.asciidoc b/Documentation/btrfs-replace.asciidoc
> index b73bf1b3..e0b30066 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,8 @@ never allowed to be used as the <targetdev>.
>  +
>  -B::::
>  no background replace.
> ++a::::
> +automatically resizes the filesystem if the <targetdev> is bigger than <srcdev>.

Resizes "to its max size".

>  
>  *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..48f470cd 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 resize the filesystem if the <targetdev> is bigger",
> +	"       than <srcdev>",

Same here, resize to its max size.

>  	"-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,15 @@ 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))
> +			goto leave_with_error;
> +	}
> +

I'm pretty happy since it's all done in user space.

But this is a different error, here we succeeded in replace, but only
failed in resize (which should be pretty rare to hit though).

We should provide some better error message for it other than just error
out.

Thanks,
Qu

>  	close_file_or_dir(fdmnt, dirstream);
>  	return 0;
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]