[PATCH v2 0/3] btrfs-progs: device: update btrfs device remove missing

[PATCH v2 0/3] btrfs-progs: device: update btrfs device remove missing

[Misono, Tomohiro]

This series updates "btrfs device remove missing".

Currently, the document lacks the description of "remove missing" which is
written only in wikipage. First and second patch updates the documents.
Third patch introduces new keyword "missing-all" to remove the all missing
devices at once for improving usability.

changelog:
v1->v2:
 split the patch and add missing-all
 
Tomohiro Misono (3):
	btrfs-progs: device: add description of alias to help message
	btrfs-progs: doc: add description of missing and example of device remove
	btrfs-progs: device: add remove missing-all

 Documentation/btrfs-device.asciidoc | 20 +++++++++++++++++++-
 cmds-device.c                       | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.9.5

[PATCH v2 1/3] btrfs-progs: device: add description of alias to help message

[Misono, Tomohiro]

State the 'delete' is the alias of 'remove' as the man page says.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Reviewed-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
---
 cmds-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-device.c b/cmds-device.c
index 4337eb2..3b6b985 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -237,7 +237,7 @@ static int cmd_device_remove(int argc, char **argv)
 
 static const char * const cmd_device_delete_usage[] = {
 	"btrfs device delete <device>|<devid> [<device>|<devid>...] <path>",
-	"Remove a device from a filesystem",
+	"Remove a device from a filesystem (alias of \"btrfs device remove\")",
 	NULL
 };
 
-- 
2.9.5

[PATCH v2 2/3] btrfs-progs: doc: add description of missing and example of device remove

[Misono, Tomohiro]

This patch updates help/document of "btrfs device remove" in two points:

1. Add explanation of 'missing' for 'device remove'. This is only
written in wikipage currently.
(https://btrfs.wiki.kernel.org/index.php/Using_Btrfs_with_Multiple_Devices)

2. Add example of device removal in the man document. This is because
that explanation of "remove" says "See the example section below", but
there is no example of removal currently.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Reviewed-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
---
 Documentation/btrfs-device.asciidoc | 19 ++++++++++++++++++-
 cmds-device.c                       |  8 ++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-device.asciidoc b/Documentation/btrfs-device.asciidoc
index 88822ec..dd60415 100644
--- a/Documentation/btrfs-device.asciidoc
+++ b/Documentation/btrfs-device.asciidoc
@@ -68,13 +68,17 @@ Remove device(s) from a filesystem identified by <path>
 Device removal must satisfy the profile constraints, otherwise the command
 fails. The filesystem must be converted to profile(s) that would allow the
 removal. This can typically happen when going down from 2 devices to 1 and
-using the RAID1 profile. See the example section below.
+using the RAID1 profile. See the *TYPICAL USECASES* section below.
 +
 The operation can take long as it needs to move all data from the device.
 +
 It is possible to delete the device that was used to mount the filesystem. The
 device entry in mount table will be replaced by another device name with the
 lowest device id.
++
+If device is mounted as degraded mode (-o degraded), special term "missing"
+can be used for <device>. In that case, the first device that is described by
+the filesystem metadata, but not preseted at the mount time will be removed.
 
 *delete* <device>|<devid> [<device>|<devid>...] <path>::
 Alias of remove kept for backward compatibility
@@ -206,6 +210,19 @@ data or the block groups occupy the whole first device.
 The device size of '/dev/sdb' as seen by the filesystem remains unchanged, but
 the logical space from 50-100GiB will be unused.
 
+==== REMOVE DEVICE ====
+
+Device removal must satisfy the profile constraints, otherwise the command
+fails. For example:
+
+ $ btrfs device remove /dev/sda /mnt
+ ERROR: error removing device '/dev/sda': unable to go below two devices on raid1
+
+In order to remove a device, you need to convert the profile in this case:
+
+ $ btrfs balance start -mconvert=dup -dconvert=single /mnt
+ $ btrfs device remove /dev/sda /mnt
+
 DEVICE STATS
 ------------
 
diff --git a/cmds-device.c b/cmds-device.c
index 3b6b985..d28ed0f 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -224,9 +224,16 @@ static int _cmd_device_remove(int argc, char **argv,
 	return !!ret;
 }
 
+#define COMMON_USAGE_REMOVE_DELETE \
+	"", \
+	"If 'missing' is specified for <device>, the first device that is", \
+	"described by the filesystem metadata, but not presented at the", \
+	"mount time will be removed."
+
 static const char * const cmd_device_remove_usage[] = {
 	"btrfs device remove <device>|<devid> [<device>|<devid>...] <path>",
 	"Remove a device from a filesystem",
+	COMMON_USAGE_REMOVE_DELETE,
 	NULL
 };
 
@@ -238,6 +245,7 @@ static int cmd_device_remove(int argc, char **argv)
 static const char * const cmd_device_delete_usage[] = {
 	"btrfs device delete <device>|<devid> [<device>|<devid>...] <path>",
 	"Remove a device from a filesystem (alias of \"btrfs device remove\")",
+	COMMON_USAGE_REMOVE_DELETE,
 	NULL
 };
 
-- 
2.9.5

[PATCH v2 3/3] btrfs-progs: device: add remove missing-all

[Misono, Tomohiro]

Add 'btrfs remove missing-all' to remove all the missing devices
at once for improving usability.

Example:
 sudo mkfs.btrfs -f -d raid1 /dev/sdb1 /dev/sdb2 /dev/sdb3 /dev/sdb4
 sudo wipefs -a /dev/sdb1 /dev/sdb3
 sudo mount -o degraded /dev/sdb2 /mnt
 sudo btrfs filesystem show /mnt
 sudo btrfs device remove missing-all /mnt
 sudo btrfs filesystem show /mnt

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 Documentation/btrfs-device.asciidoc |  1 +
 cmds-device.c                       | 24 +++++++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-device.asciidoc b/Documentation/btrfs-device.asciidoc
index dd60415..f08d64d 100644
--- a/Documentation/btrfs-device.asciidoc
+++ b/Documentation/btrfs-device.asciidoc
@@ -79,6 +79,7 @@ lowest device id.
 If device is mounted as degraded mode (-o degraded), special term "missing"
 can be used for <device>. In that case, the first device that is described by
 the filesystem metadata, but not preseted at the mount time will be removed.
+Also, "missing-all" can be used to remove all the missing devices.
 
 *delete* <device>|<devid> [<device>|<devid>...] <path>::
 Alias of remove kept for backward compatibility
diff --git a/cmds-device.c b/cmds-device.c
index d28ed0f..507ad04 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -164,6 +164,8 @@ static int _cmd_device_remove(int argc, char **argv,
 		struct	btrfs_ioctl_vol_args arg;
 		struct btrfs_ioctl_vol_args_v2 argv2 = {0};
 		int is_devid = 0;
+		int is_missing_all = 0;
+		int num_missing = 0;
 		int	res;
 
 		if (string_is_numerical(argv[i])) {
@@ -173,12 +175,16 @@ static int _cmd_device_remove(int argc, char **argv,
 		} else if (is_block_device(argv[i]) == 1 ||
 				strcmp(argv[i], "missing") == 0) {
 			strncpy_null(argv2.name, argv[i]);
+		} else if (strcmp(argv[i], "missing-all") == 0) {
+			is_missing_all = 1;
+			strncpy_null(argv2.name, "missing");
 		} else {
 			error("not a block device: %s", argv[i]);
 			ret++;
 			continue;
 		}
 
+again:
 		/*
 		 * Positive values are from BTRFS_ERROR_DEV_*,
 		 * otherwise it's a generic error, one of errnos
@@ -202,6 +208,21 @@ static int _cmd_device_remove(int argc, char **argv,
 			res = ioctl(fdmnt, BTRFS_IOC_RM_DEV, &arg);
 		}
 
+		if (is_missing_all) {
+			if (!res) {
+				num_missing++;
+				goto again;
+			}
+
+			if (num_missing > 0 &&
+				(res == BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET ||
+				 res == BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET ||
+				 res == BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET ||
+				 res == BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET ||
+				 res == BTRFS_ERROR_DEV_MISSING_NOT_FOUND))
+				continue;
+		}
+
 		if (res) {
 			const char *msg;
 
@@ -228,7 +249,8 @@ static int _cmd_device_remove(int argc, char **argv,
 	"", \
 	"If 'missing' is specified for <device>, the first device that is", \
 	"described by the filesystem metadata, but not presented at the", \
-	"mount time will be removed."
+	"mount time will be removed. Use 'missing-all' to remove all the", \
+	"missing devices."
 
 static const char * const cmd_device_remove_usage[] = {
 	"btrfs device remove <device>|<devid> [<device>|<devid>...] <path>",
-- 
2.9.5

Re: [PATCH v2 3/3] btrfs-progs: device: add remove missing-all

[Duncan]

Misono, Tomohiro posted on Wed, 11 Oct 2017 11:18:50 +0900 as excerpted:

> Add 'btrfs remove missing-all' to remove all the missing devices at once
> for improving usability.
> 
> Example:
>  sudo mkfs.btrfs -f -d raid1 /dev/sdb1 /dev/sdb2 /dev/sdb3 /dev/sdb4
>  sudo wipefs -a /dev/sdb1 /dev/sdb3
>  sudo mount -o degraded /dev/sdb2 /mnt
>  sudo btrfs filesystem show /mnt
>  sudo btrfs device remove missing-all /mnt
>  sudo btrfs filesystem show /mnt


There's a reason remove missing-all hasn't yet been implemented.

Note that the above would be very unlikely to work once a filesystem has 
been used in any significant way, because raid1 and raid10 are explicitly 
chunk pairs, *NOT* duplicated N times across N devices.  So with two 
devices missing, chances are that both copies of some chunks will be 
missing as well, so the filesystem would no longer be mountable degraded-
writable, only degraded-readonly, in which case device remove won't work 
at all because the filesystem is readonly.

In fact, until the recent per-chunk check patches went in, it was 
impossible to mount-writable a raid1 missing two devices at all, because 
the safeguards simply assumed some chunks would be entirely missing.

The only case in which more than a single device missing is likely to be 
mountable degraded-writable (so device remove will work at all) is raid6, 
tho with recent patches there's narrow cases in which it /might/ be 
doable with raid1 as well.

Now you may still wish to implement remove missing-all for raid6 mode and 
for the unusual corner-case raid1/raid10 in which it might work, but the 
documentation should be pretty clear that save for raid6 it can't be 
expected to work in most cases.

Given that, I think remove missing-all hasn't been implemented as it 
simply hasn't been considered to be worth the bother for the narrow use-
cases in which it will actually work.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [PATCH v2 3/3] btrfs-progs: device: add remove missing-all

[Anand Jain]

On 10/13/2017 01:27 PM, Duncan wrote:
> Misono, Tomohiro posted on Wed, 11 Oct 2017 11:18:50 +0900 as excerpted:
> 
>> Add 'btrfs remove missing-all' to remove all the missing devices at once
>> for improving usability.
>>
>> Example:
>>   sudo mkfs.btrfs -f -d raid1 /dev/sdb1 /dev/sdb2 /dev/sdb3 /dev/sdb4
>>   sudo wipefs -a /dev/sdb1 /dev/sdb3
>>   sudo mount -o degraded /dev/sdb2 /mnt <-- 


   I agree with Duncan here. This step itself will fail even with RO
   option. Do you have any patch that is not in the ML which will
   make this step a success in the first place ?

Thanks, Anand


>>   sudo btrfs filesystem show /mnt
>>   sudo btrfs device remove missing-all /mnt
>>   sudo btrfs filesystem show /mnt
> 
> 
> There's a reason remove missing-all hasn't yet been implemented.
> 
> Note that the above would be very unlikely to work once a filesystem has
> been used in any significant way, because raid1 and raid10 are explicitly
> chunk pairs, *NOT* duplicated N times across N devices.  So with two
> devices missing, chances are that both copies of some chunks will be
> missing as well, so the filesystem would no longer be mountable degraded-
> writable, only degraded-readonly, in which case device remove won't work
> at all because the filesystem is readonly.
> 
> In fact, until the recent per-chunk check patches went in, it was
> impossible to mount-writable a raid1 missing two devices at all, because
> the safeguards simply assumed some chunks would be entirely missing.
> 
> The only case in which more than a single device missing is likely to be
> mountable degraded-writable (so device remove will work at all) is raid6,
> tho with recent patches there's narrow cases in which it /might/ be
> doable with raid1 as well.
> 
> Now you may still wish to implement remove missing-all for raid6 mode and
> for the unusual corner-case raid1/raid10 in which it might work, but the
> documentation should be pretty clear that save for raid6 it can't be
> expected to work in most cases.
> 
> Given that, I think remove missing-all hasn't been implemented as it
> simply hasn't been considered to be worth the bother for the narrow use-
> cases in which it will actually work.
> 

Re: [PATCH v2 3/3] btrfs-progs: device: add remove missing-all

[Misono, Tomohiro]

On 2017/10/16 12:30, Anand Jain wrote:
> 
> 
> On 10/13/2017 01:27 PM, Duncan wrote:
>> Misono, Tomohiro posted on Wed, 11 Oct 2017 11:18:50 +0900 as excerpted:
>>
>>> Add 'btrfs remove missing-all' to remove all the missing devices at once
>>> for improving usability.
>>>
>>> Example:
>>>   sudo mkfs.btrfs -f -d raid1 /dev/sdb1 /dev/sdb2 /dev/sdb3 /dev/sdb4
>>>   sudo wipefs -a /dev/sdb1 /dev/sdb3
>>>   sudo mount -o degraded /dev/sdb2 /mnt <-- 
> 
> 
>    I agree with Duncan here. This step itself will fail even with RO
>    option. Do you have any patch that is not in the ML which will
>    make this step a success in the first place ?
> 
> Thanks, Anand
> 

commit 21634a19f646 ("btrfs: Introduce a function to check if all chunks a OK
for degraded rw mount") allow this from 4.14 (I checked on 4.14-rc4).
But I will withdraw this patch as Duncan suggests.

Thanks,
Tomohiro

Re: [PATCH v2 3/3] btrfs-progs: device: add remove missing-all

[Anand Jain]

On 10/16/2017 12:35 PM, Misono, Tomohiro wrote:
> 
> 
> On 2017/10/16 12:30, Anand Jain wrote:
>>
>>
>> On 10/13/2017 01:27 PM, Duncan wrote:
>>> Misono, Tomohiro posted on Wed, 11 Oct 2017 11:18:50 +0900 as excerpted:
>>>
>>>> Add 'btrfs remove missing-all' to remove all the missing devices at once
>>>> for improving usability.
>>>>
>>>> Example:
>>>>    sudo mkfs.btrfs -f -d raid1 /dev/sdb1 /dev/sdb2 /dev/sdb3 /dev/sdb4
>>>>    sudo wipefs -a /dev/sdb1 /dev/sdb3
>>>>    sudo mount -o degraded /dev/sdb2 /mnt <--
>>
>>
>>     I agree with Duncan here. This step itself will fail even with RO
>>     option.

  Correction. RO mount is possible based on which disk you choose to mount.

Thanks - Anand


>> Do you have any patch that is not in the ML which will
>>     make this step a success in the first place ?
>>
>> Thanks, Anand
>>
> 
> commit 21634a19f646 ("btrfs: Introduce a function to check if all chunks a OK
> for degraded rw mount") allow this from 4.14 (I checked on 4.14-rc4).
> But I will withdraw this patch as Duncan suggests.
> 
> Thanks,
> Tomohiro
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 3/3] btrfs-progs: device: add remove missing-all

[Duncan]

Misono, Tomohiro posted on Mon, 16 Oct 2017 13:35:08 +0900 as excerpted:

> On 2017/10/16 12:30, Anand Jain wrote:
>> 
>> 
>> On 10/13/2017 01:27 PM, Duncan wrote:
>>> Misono, Tomohiro posted on Wed, 11 Oct 2017 11:18:50 +0900 as
>>> excerpted:
>>>
>>>> Add 'btrfs remove missing-all' to remove all the missing devices at
>>>> once for improving usability.
>>>>
>>>> Example:
>>>>   sudo mkfs.btrfs -f -d raid1 /dev/sdb1 /dev/sdb2 /dev/sdb3 /dev/sdb4
>>>>   sudo wipefs -a /dev/sdb1 /dev/sdb3 sudo mount -o degraded /dev/sdb2
>>>>   /mnt <--
>> 
>> 
>>    I agree with Duncan here. This step itself will fail even with RO
>>    option. Do you have any patch that is not in the ML which will make
>>    this step a success in the first place ?
>> 
>> Thanks, Anand
>> 
>> 
> commit 21634a19f646 ("btrfs: Introduce a function to check if all chunks
> a OK for degraded rw mount") allow this from 4.14 (I checked on
> 4.14-rc4).

That's why I said recent patches allow it in corner-cases.  However, I 
think those corner-cases would I think be difficult to document concisely 
in the manpage, and without that, I believe the option would be more 
confusing than helpful, since people would expect it to actually work 
when it won't, for them.

> But I will withdraw this patch as Duncan suggests.

If someone comes up with a satisfactory way to explain at a manpage level 
the corner-cases in which removing more than one missing device at a time 
can work...

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [PATCH v2 3/3] btrfs-progs: device: add remove missing-all

[Misono, Tomohiro]

On 2017/10/13 14:27, Duncan wrote:
> Misono, Tomohiro posted on Wed, 11 Oct 2017 11:18:50 +0900 as excerpted:
> 
>> Add 'btrfs remove missing-all' to remove all the missing devices at once
>> for improving usability.
>>
>> Example:
>>  sudo mkfs.btrfs -f -d raid1 /dev/sdb1 /dev/sdb2 /dev/sdb3 /dev/sdb4
>>  sudo wipefs -a /dev/sdb1 /dev/sdb3
>>  sudo mount -o degraded /dev/sdb2 /mnt
>>  sudo btrfs filesystem show /mnt
>>  sudo btrfs device remove missing-all /mnt
>>  sudo btrfs filesystem show /mnt
> 
> 
> There's a reason remove missing-all hasn't yet been implemented.
> 
> Note that the above would be very unlikely to work once a filesystem has 
> been used in any significant way, because raid1 and raid10 are explicitly 
> chunk pairs, *NOT* duplicated N times across N devices.  So with two 
> devices missing, chances are that both copies of some chunks will be 
> missing as well, so the filesystem would no longer be mountable degraded-
> writable, only degraded-readonly, in which case device remove won't work 
> at all because the filesystem is readonly.
> 
> In fact, until the recent per-chunk check patches went in, it was 
> impossible to mount-writable a raid1 missing two devices at all, because 
> the safeguards simply assumed some chunks would be entirely missing.
> 
> The only case in which more than a single device missing is likely to be 
> mountable degraded-writable (so device remove will work at all) is raid6, 
> tho with recent patches there's narrow cases in which it /might/ be 
> doable with raid1 as well.
> 
> Now you may still wish to implement remove missing-all for raid6 mode and 
> for the unusual corner-case raid1/raid10 in which it might work, but the 
> documentation should be pretty clear that save for raid6 it can't be 
> expected to work in most cases.
> 
> Given that, I think remove missing-all hasn't been implemented as it 
> simply hasn't been considered to be worth the bother for the narrow use-
> cases in which it will actually work.

Thanks for the comments.

I thought this is useful, but agree that this is for rare case and might be
confusing. So, I will drop 3rd patch and just resend 1st/2nd again.

Regards,
Tomohiro