[PATCH] btrfs-progs: add a warning and countdown for RAID5/6 conversion

[PATCH] btrfs-progs: add a warning and countdown for RAID5/6 conversion

[Josef Bacik]

Similar to the mkfs warning, add a warning to btrfs balance -*convert
options, with a countdown to allow the user to have time to cancel the
operation.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 cmds/balance.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/cmds/balance.c b/cmds/balance.c
index b0535d40..d7b2b3d6 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -515,6 +515,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
 	int force = 0;
 	int background = 0;
 	unsigned start_flags = 0;
+	bool warned = false;
 	int i;
 
 	memset(&args, 0, sizeof(args));
@@ -616,11 +617,36 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
 
 	/* soft makes sense only when convert for corresponding type is set */
 	for (i = 0; ptrs[i]; i++) {
+		int delay = 10;
+
 		if ((ptrs[i]->flags & BTRFS_BALANCE_ARGS_SOFT) &&
 		    !(ptrs[i]->flags & BTRFS_BALANCE_ARGS_CONVERT)) {
 			error("'soft' option can be used only when converting profiles");
 			return 1;
 		}
+
+		if (!(ptrs[i]->flags & BTRFS_BALANCE_ARGS_CONVERT))
+			continue;
+
+		if (!(ptrs[i]->flags & (BTRFS_BLOCK_GROUP_RAID6 |
+					BTRFS_BLOCK_GROUP_RAID5)))
+			continue;
+
+		if (warned)
+			continue;
+
+		warned = true;
+		printf("WARNING:\n\n");
+		printf("\tRAID5/6 support is still experimental and has known issues.\n");
+		printf("\tIt is recommended that you use one of the other RAID profiles.\n");
+		printf("\tThe operation will continue in %d seconds.\n", delay);
+		printf("\tUse Ctrl-C to stop.\n");
+		while (delay) {
+			printf("%2d", delay--);
+			fflush(stdout);
+			sleep(1);
+		}
+		printf("\nStarting conversion to RAID5/6.\n");
 	}
 
 	if (!(start_flags & BALANCE_START_FILTERS) && !(start_flags & BALANCE_START_NOWARN)) {
-- 
2.24.1

Re: [PATCH] btrfs-progs: add a warning and countdown for RAID5/6 conversion

[Goffredo Baroncelli]

On 8/25/20 7:13 PM, Josef Bacik wrote:
> Similar to the mkfs warning, add a warning to btrfs balance -*convert
> options, with a countdown to allow the user to have time to cancel the
> operation.

It is possible to add a switch to skip the countdown ? Something like "--balance-raid56-i-know-what-i-am-doing". I am thinking to the developers which are doing some tests
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   cmds/balance.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/cmds/balance.c b/cmds/balance.c
> index b0535d40..d7b2b3d6 100644
> --- a/cmds/balance.c
> +++ b/cmds/balance.c
> @@ -515,6 +515,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
>   	int force = 0;
>   	int background = 0;
>   	unsigned start_flags = 0;
> +	bool warned = false;
>   	int i;
>   
>   	memset(&args, 0, sizeof(args));
> @@ -616,11 +617,36 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
>   
>   	/* soft makes sense only when convert for corresponding type is set */
>   	for (i = 0; ptrs[i]; i++) {
> +		int delay = 10;
> +
>   		if ((ptrs[i]->flags & BTRFS_BALANCE_ARGS_SOFT) &&
>   		    !(ptrs[i]->flags & BTRFS_BALANCE_ARGS_CONVERT)) {
>   			error("'soft' option can be used only when converting profiles");
>   			return 1;
>   		}
> +
> +		if (!(ptrs[i]->flags & BTRFS_BALANCE_ARGS_CONVERT))
> +			continue;
> +
> +		if (!(ptrs[i]->flags & (BTRFS_BLOCK_GROUP_RAID6 |
> +					BTRFS_BLOCK_GROUP_RAID5)))
> +			continue;
> +
> +		if (warned)
> +			continue;
> +
> +		warned = true;
> +		printf("WARNING:\n\n");
> +		printf("\tRAID5/6 support is still experimental and has known issues.\n");
> +		printf("\tIt is recommended that you use one of the other RAID profiles.\n");
> +		printf("\tThe operation will continue in %d seconds.\n", delay);
> +		printf("\tUse Ctrl-C to stop.\n");
> +		while (delay) {
> +			printf("%2d", delay--);
> +			fflush(stdout);
> +			sleep(1);
> +		}
> +		printf("\nStarting conversion to RAID5/6.\n");
>   	}
>   
>   	if (!(start_flags & BALANCE_START_FILTERS) && !(start_flags & BALANCE_START_NOWARN)) {
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH] btrfs-progs: add a warning and countdown for RAID5/6 conversion

[Josef Bacik]

On 8/25/20 1:28 PM, Goffredo Baroncelli wrote:
> On 8/25/20 7:13 PM, Josef Bacik wrote:
>> Similar to the mkfs warning, add a warning to btrfs balance -*convert
>> options, with a countdown to allow the user to have time to cancel the
>> operation.
> 
> It is possible to add a switch to skip the countdown ? Something like 
> "--balance-raid56-i-know-what-i-am-doing". I am thinking to the developers which 
> are doing some tests

Yeah I can do that, I checked xfstests and there's only one test where we do the 
convert to raid5/6, so we're just going to eat the timeout there.  Thanks,

Josef

Re: [PATCH] btrfs-progs: add a warning and countdown for RAID5/6 conversion

[David Sterba]

On Tue, Aug 25, 2020 at 07:28:45PM +0200, Goffredo Baroncelli wrote:
> On 8/25/20 7:13 PM, Josef Bacik wrote:
> > Similar to the mkfs warning, add a warning to btrfs balance -*convert
> > options, with a countdown to allow the user to have time to cancel the
> > operation.
> 
> It is possible to add a switch to skip the countdown ? Something like
> "--balance-raid56-i-know-what-i-am-doing". I am thinking to the
> developers which are doing some tests

I'd rather not add a new option for that, I think reusing --force for
that reason should be enough. It's otherwise used for redundancy
reduction, which does not get affected when the raid56 profiles are not
used.