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

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

[Marcos Paulo de Souza]

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

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!

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 |  4 +-
 cmds/filesystem.c                    | 58 +--------------------------
 cmds/replace.c                       | 19 ++++++++-
 common/utils.c                       | 60 ++++++++++++++++++++++++++++
 common/utils.h                       |  1 +
 5 files changed, 83 insertions(+), 59 deletions(-)

-- 
2.25.0

[PATCH 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

[PATCH 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>
---
 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>.
 
 *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>",
 	"-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;
+	}
+
 	close_file_or_dir(fdmnt, dirstream);
 	return 0;
 
-- 
2.25.0

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

[Qu Wenruo]

[-- 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 --]

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

[Marcos Paulo de Souza]

On Wed, Mar 18, 2020 at 06:56:51PM +0800, Qu Wenruo wrote:
> 
> 
> 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.

Function resize_filesystem already checks for errors, and it even print's a
resize message before calling the resize ioctl. Or do you mean another message
specifying that "replace finished, but resize failed" along the messages already
being provided by resize_filesystem function?

Thanks,
  Marcos

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

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

[Qu Wenruo]

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



On 2020/3/19 上午4:47, Marcos Paulo de Souza wrote:
> On Wed, Mar 18, 2020 at 06:56:51PM +0800, Qu Wenruo wrote:
>>
>>
>> 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.
> 
> Function resize_filesystem already checks for errors, and it even print's a
> resize message before calling the resize ioctl. Or do you mean another message
> specifying that "replace finished, but resize failed" along the messages already
> being provided by resize_filesystem function?

Yes, that's what I mean.

Thanks,
Qu
> 
> Thanks,
>   Marcos
> 
>>
>> Thanks,
>> Qu
>>
>>>  	close_file_or_dir(fdmnt, dirstream);
>>>  	return 0;
>>>  
>>>
>>
> 
> 
> 


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

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

[Anand Jain]

On 3/8/20 6:45 AM, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> 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").

If resize fails we should be able to fail the replace as well which does
not happen here. I am thinking this should be kernel feature, do the
resize part before calling btrfs_dev_replace_finishing().

IMO, it makes sense that replace and resize be in one command as
proposed in this patch. We had similar discussion whether to combine
replace and resize of missing device here:
    https://patchwork.kernel.org/patch/11249009/

-Anand

> 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!
> 
> 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 |  4 +-
>   cmds/filesystem.c                    | 58 +--------------------------
>   cmds/replace.c                       | 19 ++++++++-
>   common/utils.c                       | 60 ++++++++++++++++++++++++++++
>   common/utils.h                       |  1 +
>   5 files changed, 83 insertions(+), 59 deletions(-)
>