diff mbox series

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

Message ID 20200307224516.16315-3-marcos@mpdesouza.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: Auto resize fs after device replace | expand

Commit Message

Marcos Paulo de Souza March 7, 2020, 10:45 p.m. UTC
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(-)

Comments

Qu Wenruo March 18, 2020, 10:56 a.m. UTC | #1
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;
>  
>
Marcos Paulo de Souza March 18, 2020, 8:47 p.m. UTC | #2
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;
> >  
> > 
>
Qu Wenruo March 19, 2020, 7:03 a.m. UTC | #3
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;
>>>  
>>>
>>
> 
> 
>
diff mbox series

Patch

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;