[PATCH 00/10] btrfs-progs: check and tune: add device and noscan options

[PATCH 00/10] btrfs-progs: check and tune: add device and noscan options

[Anand Jain]

By default, btrfstune and btrfs check scans all and only the block devices
in the system.

To scan regular files without mapping them to a loop device, adds the
--device option.

To indicate not to scan the system for other devices, adds the --noscan
option.

For example:

  The command below will scan both regular files and the devices
  provided in the --device option, along with the system block devices.

        btrfstune -m --device /tdev/td1,/tdev/td2 /tdev/td3
  or
        btrfs check --device /tdev/td1 --device /tdev/td2 /tdev/td3

  In some cases, if you need to avoid the default system scan for the
  block device, you can use the --noscan option.

        btrfstune -m --noscan --device /tdev/td1,/tdev/td2 /tdev/td3

        btrfs check --noscan --device /tdev/td1,/tdev/td2 /tdev/td3

 This patch bundle depends on the preparatory patch bundle sent before:
    [PATCH 0/6 v3] btrfs-progs: cleanup and preparatory around device scan

 And, replaces [1] in the mailing list, as the --device option helper
 function is peeled and transformed into a common helper function
 in a separate patch.
    [1] [PATCH 0/4] btrfs-progs: tune: add --device and --noscan option

Anand Jain (10):
  btrfs-progs: common: add --device option helpers
  btrfs-progs: tune: consolidate return goto free-out
  btrfs-progs: tune: introduce --device option
  btrfs-progs: docs: update btrfstune --device option
  btrfs-progs: tune: introduce --noscan option
  btrfs-progs: docs: update btrfstune --noscan option
  btrfs-progs: check: introduce --device option
  btrfs-progs: docs: update btrfs check --device option
  btrfs-progs: check: introduce --noscan option
  btrfs-progs: docs: update btrfs check --noscan option

 Documentation/btrfs-check.rst |  6 +++
 Documentation/btrfstune.rst   |  7 ++++
 check/main.c                  | 45 +++++++++++++++++++++-
 common/device-scan.c          | 32 ++++++++++++++++
 common/device-scan.h          |  2 +
 tune/main.c                   | 71 ++++++++++++++++++++++++++++-------
 6 files changed, 148 insertions(+), 15 deletions(-)

-- 
2.31.1

[PATCH 01/10] btrfs-progs: common: add --device option helpers

[Anand Jain]

Preparatory patch adds two helper functions: array_append() and free_array(),
which facilitate reading the device list provided at the --device option.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/device-scan.c | 32 ++++++++++++++++++++++++++++++++
 common/device-scan.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/common/device-scan.c b/common/device-scan.c
index 68b94ecd9d77..ba11c58d00d2 100644
--- a/common/device-scan.c
+++ b/common/device-scan.c
@@ -31,6 +31,7 @@
 #include <dirent.h>
 #include <limits.h>
 #include <stdbool.h>
+#include <ctype.h>
 #include <blkid/blkid.h>
 #include <uuid/uuid.h>
 #ifdef HAVE_LIBUDEV
@@ -540,3 +541,34 @@ int btrfs_scan_argv_devices(int dev_optind, int dev_argc, char **dev_argv)
 
 	return 0;
 }
+
+bool array_append(char **dest, char *src, int *cnt)
+{
+	char *this_tok = strtok(src, ",");
+	int ret_cnt = *cnt;
+
+	while(this_tok != NULL) {
+		ret_cnt++;
+		dest = realloc(dest, sizeof(char *) * ret_cnt);
+		if (!dest)
+			return false;
+
+		dest[ret_cnt - 1] = strdup(this_tok);
+		*cnt = ret_cnt;
+
+		this_tok = strtok(NULL, ",");
+	}
+
+	return true;
+}
+
+void free_array(char **prt, int cnt)
+{
+	if (!prt)
+		return;
+
+	for (int i = 0; i < cnt; i++)
+		free(prt[i]);
+
+	free(prt);
+}
diff --git a/common/device-scan.h b/common/device-scan.h
index 0d0f081134f2..d3b5f7d2753f 100644
--- a/common/device-scan.h
+++ b/common/device-scan.h
@@ -59,5 +59,7 @@ int add_seen_fsid(u8 *fsid, struct seen_fsid *seen_fsid_hash[],
 void free_seen_fsid(struct seen_fsid *seen_fsid_hash[]);
 int test_uuid_unique(const char *uuid_str);
 int btrfs_scan_argv_devices(int dev_optind, int argc, char **argv);
+bool array_append(char **dest, char *src, int *cnt);
+void free_array(char **prt, int cnt);
 
 #endif
-- 
2.31.1

Re: [PATCH 01/10] btrfs-progs: common: add --device option helpers

[David Sterba]

On Wed, Jun 28, 2023 at 07:56:08PM +0800, Anand Jain wrote:
> Preparatory patch adds two helper functions: array_append() and free_array(),
> which facilitate reading the device list provided at the --device option.

That it's for --device is for later and not that interesting when adding
some API.

> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  common/device-scan.c | 32 ++++++++++++++++++++++++++++++++
>  common/device-scan.h |  2 ++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/common/device-scan.c b/common/device-scan.c
> index 68b94ecd9d77..ba11c58d00d2 100644
> --- a/common/device-scan.c
> +++ b/common/device-scan.c
> @@ -31,6 +31,7 @@
>  #include <dirent.h>
>  #include <limits.h>
>  #include <stdbool.h>
> +#include <ctype.h>
>  #include <blkid/blkid.h>
>  #include <uuid/uuid.h>
>  #ifdef HAVE_LIBUDEV
> @@ -540,3 +541,34 @@ int btrfs_scan_argv_devices(int dev_optind, int dev_argc, char **dev_argv)
>  
>  	return 0;
>  }
> +
> +bool array_append(char **dest, char *src, int *cnt)
> +{
> +	char *this_tok = strtok(src, ",");
> +	int ret_cnt = *cnt;
> +
> +	while(this_tok != NULL) {
> +		ret_cnt++;
> +		dest = realloc(dest, sizeof(char *) * ret_cnt);
> +		if (!dest)
> +			return false;
> +
> +		dest[ret_cnt - 1] = strdup(this_tok);
> +		*cnt = ret_cnt;
> +
> +		this_tok = strtok(NULL, ",");
> +	}
> +
> +	return true;
> +}
> +
> +void free_array(char **prt, int cnt)
> +{
> +	if (!prt)
> +		return;
> +
> +	for (int i = 0; i < cnt; i++)
> +		free(prt[i]);
> +
> +	free(prt);
> +}

Looks like this is an extensible pointer array, we could use that in
more places where there are repeated parameters and we need to track all
the values (not just the last one).

Then this should be in a structure and the usage side will do only
something like ptr_array_append(&array, newvalue), and not that all
places will have to track the base double pointer, count and has to
handle allocation failures. This should be wrapped into an API.

Re: [PATCH 01/10] btrfs-progs: common: add --device option helpers

[David Sterba]

On Thu, Jul 13, 2023 at 08:41:01PM +0200, David Sterba wrote:
> On Wed, Jun 28, 2023 at 07:56:08PM +0800, Anand Jain wrote:
> > +bool array_append(char **dest, char *src, int *cnt)
> > +{
> > +	char *this_tok = strtok(src, ",");
> > +	int ret_cnt = *cnt;
> > +
> > +	while(this_tok != NULL) {
> > +		ret_cnt++;
> > +		dest = realloc(dest, sizeof(char *) * ret_cnt);
> > +		if (!dest)
> > +			return false;
> > +
> > +		dest[ret_cnt - 1] = strdup(this_tok);
> > +		*cnt = ret_cnt;
> > +
> > +		this_tok = strtok(NULL, ",");
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +void free_array(char **prt, int cnt)
> > +{
> > +	if (!prt)
> > +		return;
> > +
> > +	for (int i = 0; i < cnt; i++)
> > +		free(prt[i]);
> > +
> > +	free(prt);
> > +}
> 
> Looks like this is an extensible pointer array, we could use that in
> more places where there are repeated parameters and we need to track all
> the values (not just the last one).
> 
> Then this should be in a structure and the usage side will do only
> something like ptr_array_append(&array, newvalue), and not that all
> places will have to track the base double pointer, count and has to
> handle allocation failures. This should be wrapped into an API.

I did a quick search for reallocs, that usually mean there's an
extensible array and there are too many, so I get you wanted add one more
and be done. But now it looks like a proper structure should be used.
I'll try to implement something.

Re: [PATCH 01/10] btrfs-progs: common: add --device option helpers

[Anand Jain]

On 15/07/2023 05:28, David Sterba wrote:
> On Thu, Jul 13, 2023 at 08:41:01PM +0200, David Sterba wrote:
>> On Wed, Jun 28, 2023 at 07:56:08PM +0800, Anand Jain wrote:
>>> +bool array_append(char **dest, char *src, int *cnt)
>>> +{
>>> +	char *this_tok = strtok(src, ",");
>>> +	int ret_cnt = *cnt;
>>> +
>>> +	while(this_tok != NULL) {
>>> +		ret_cnt++;
>>> +		dest = realloc(dest, sizeof(char *) * ret_cnt);
>>> +		if (!dest)
>>> +			return false;
>>> +
>>> +		dest[ret_cnt - 1] = strdup(this_tok);
>>> +		*cnt = ret_cnt;
>>> +
>>> +		this_tok = strtok(NULL, ",");
>>> +	}
>>> +
>>> +	return true;
>>> +}
>>> +
>>> +void free_array(char **prt, int cnt)
>>> +{
>>> +	if (!prt)
>>> +		return;
>>> +
>>> +	for (int i = 0; i < cnt; i++)
>>> +		free(prt[i]);
>>> +
>>> +	free(prt);
>>> +}
>>
>> Looks like this is an extensible pointer array, we could use that in
>> more places where there are repeated parameters and we need to track all
>> the values (not just the last one).
>>
>> Then this should be in a structure and the usage side will do only
>> something like ptr_array_append(&array, newvalue), and not that all
>> places will have to track the base double pointer, count and has to
>> handle allocation failures. This should be wrapped into an API.
> 
> I did a quick search for reallocs, that usually mean there's an
> extensible array and there are too many, so I get you wanted add one more
> and be done. But now it looks like a proper structure should be used.
> I'll try to implement something.


Yes, indeed, this has to be an API. I thought it could be done separately.

Thanks.

[PATCH 02/10] btrfs-progs: tune: consolidate return goto free-out

[Anand Jain]

The upcoming "--device" option requires memory to parse devices, which
should be freed before returning from the main() function. As a
preparation for adding the "--device" option to the "btrfstune" command,
provide a consolidated error return exit from the main function with a
"goto" labeled "free_out". The label "free_out" may not make sense
currently, but it will when the "--device" option is added.

There are several return statements within the main function, and
changing all of them in the main "--device" feature patch would deviate
from the actual for the feature changes. Hence, it made sense to create
a preparatory patch.

The return value for any failure remains the same as in the original code.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tune/main.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tune/main.c b/tune/main.c
index 0ca1e01282c9..0c8872dcdee5 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -145,7 +145,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	bool to_fst = false;
 	int csum_type = -1;
 	char *new_fsid_str = NULL;
-	int ret;
+	int ret = 1;
 	u64 super_flags = 0;
 	int fd = -1;
 
@@ -233,18 +233,18 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	set_argv0(argv);
 	device = argv[optind];
 	if (check_argc_exact(argc - optind, 1))
-		return 1;
+		goto free_out;
 
 	if (random_fsid && new_fsid_str) {
 		error("random fsid can't be used with specified fsid");
-		return 1;
+		goto free_out;
 	}
 	if (!super_flags && !seeding_flag && !(random_fsid || new_fsid_str) &&
 	    !change_metadata_uuid && csum_type == -1 && !to_bg_tree &&
 	    !to_extent_tree && !to_fst) {
 		error("at least one option should be specified");
 		usage(&tune_cmd, 1);
-		return 1;
+		goto free_out;
 	}
 
 	if (new_fsid_str) {
@@ -253,18 +253,21 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		ret = uuid_parse(new_fsid_str, tmp);
 		if (ret < 0) {
 			error("could not parse UUID: %s", new_fsid_str);
-			return 1;
+			ret = 1;
+			goto free_out;
 		}
 		if (!test_uuid_unique(new_fsid_str)) {
 			error("fsid %s is not unique", new_fsid_str);
-			return 1;
+			ret = 1;
+			goto free_out;
 		}
 	}
 
 	fd = open(device, O_RDWR);
 	if (fd < 0) {
 		error("mount check: cannot open %s: %m", device);
-		return 1;
+		ret = 1;
+		goto free_out;
 	}
 
 	ret = check_mounted_where(fd, device, NULL, 0, NULL,
@@ -273,18 +276,21 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		errno = -ret;
 		error("could not check mount status of %s: %m", device);
 		close(fd);
-		return 1;
+		ret = 1;
+		goto free_out;
 	} else if (ret) {
 		error("%s is mounted", device);
 		close(fd);
-		return 1;
+		ret = 1;
+		goto free_out;
 	}
 
 	root = open_ctree_fd(fd, device, 0, ctree_flags);
 
 	if (!root) {
 		error("open ctree failed");
-		return 1;
+		ret = 1;
+		goto free_out;
 	}
 
  	if (to_bg_tree) {
@@ -431,5 +437,6 @@ out:
 	close_ctree(root);
 	btrfs_close_all_devices();
 
+free_out:
 	return ret;
 }
-- 
2.31.1

[PATCH 03/10] btrfs-progs: tune: introduce --device option

[Anand Jain]

As of now, btrfstune only accepts one device from the command line and
then scans the system to find other parter devices if any.

However, this method mandates always accessing the file raw image as a
loop device.

This patch modifies btrfstune to accept other devices or reg-files from
the command line using an option --device and scans/registers them.

For example:

	btrfstune -m --device /tdev/td1,/tdev/td2 /tdev/td3
  or
	btrfstune -m --device /tdev/td1 --device /tdev/td2 /tdev/td3

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tune/main.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/tune/main.c b/tune/main.c
index 0c8872dcdee5..63e3ecc934cc 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -23,6 +23,7 @@
 #include <getopt.h>
 #include <errno.h>
 #include <stdbool.h>
+#include <ctype.h>
 #include <uuid/uuid.h>
 #include "kernel-shared/ctree.h"
 #include "kernel-shared/disk-io.h"
@@ -117,6 +118,7 @@ static const char * const tune_usage[] = {
 	"",
 	"General:",
 	OPTLINE("-f", "allow dangerous operations, make sure that you are aware of the dangers"),
+	OPTLINE("--device", "devices or regular-files of the filesystem to be scanned"),
 	OPTLINE("--help", "print this help"),
 #if EXPERIMENTAL
 	"",
@@ -144,6 +146,8 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	bool to_bg_tree = false;
 	bool to_fst = false;
 	int csum_type = -1;
+	int argc_devices = 0;
+	char **argv_devices = NULL;
 	char *new_fsid_str = NULL;
 	int ret = 1;
 	u64 super_flags = 0;
@@ -155,7 +159,8 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		enum { GETOPT_VAL_CSUM = GETOPT_VAL_FIRST,
 		       GETOPT_VAL_ENABLE_BLOCK_GROUP_TREE,
 		       GETOPT_VAL_DISABLE_BLOCK_GROUP_TREE,
-		       GETOPT_VAL_ENABLE_FREE_SPACE_TREE };
+		       GETOPT_VAL_ENABLE_FREE_SPACE_TREE,
+		       GETOPT_VAL_DEVICE };
 		static const struct option long_options[] = {
 			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
 			{ "convert-to-block-group-tree", no_argument, NULL,
@@ -167,6 +172,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 #if EXPERIMENTAL
 			{ "csum", required_argument, NULL, GETOPT_VAL_CSUM },
 #endif
+			{ "device", required_argument, NULL, GETOPT_VAL_DEVICE },
 			{ NULL, 0, NULL, 0 }
 		};
 		int c = getopt_long(argc, argv, "S:rxfuU:nmM:", long_options, NULL);
@@ -210,6 +216,21 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		case GETOPT_VAL_ENABLE_BLOCK_GROUP_TREE:
 			to_bg_tree = true;
 			break;
+		case GETOPT_VAL_DEVICE:
+			if (!argv_devices) {
+				argv_devices = malloc(sizeof(char *));
+				if (!argv_devices) {
+					error("memory alloc failed");
+					return 1;
+				}
+			}
+
+			if (!array_append(argv_devices, optarg,
+					  &argc_devices)) {
+				error("memory alloc failed");
+				goto free_out;
+			}
+			break;
 		case GETOPT_VAL_DISABLE_BLOCK_GROUP_TREE:
 			to_extent_tree = true;
 			break;
@@ -285,6 +306,16 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		goto free_out;
 	}
 
+	/*
+	 * check_mounted_where() with noscan == true frees the scanned devices
+	 * scan the command line provided device list now.
+	 */
+	if (argv_devices) {
+		ret = btrfs_scan_argv_devices(0, argc_devices, argv_devices);
+		if (ret)
+			goto free_out;
+	}
+
 	root = open_ctree_fd(fd, device, 0, ctree_flags);
 
 	if (!root) {
@@ -438,5 +469,6 @@ out:
 	btrfs_close_all_devices();
 
 free_out:
+	free_array(argv_devices, argc_devices);
 	return ret;
 }
-- 
2.31.1

[PATCH 04/10] btrfs-progs: docs: update btrfstune --device option

[Anand Jain]

Update the Documentation/btrfstune.rst to carry the new --device
option.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 Documentation/btrfstune.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/btrfstune.rst b/Documentation/btrfstune.rst
index 0510ad1f4c26..89f4494bbaf0 100644
--- a/Documentation/btrfstune.rst
+++ b/Documentation/btrfstune.rst
@@ -46,6 +46,9 @@ OPTIONS
         Allow dangerous changes, e.g. clear the seeding flag or change fsid.
         Make sure that you are aware of the dangers.
 
+--device
+        List of block devices or regular files that are part of the filesystem.
+
 -m
         (since kernel: 5.0)
 
-- 
2.31.1

Re: [PATCH 04/10] btrfs-progs: docs: update btrfstune --device option

[David Sterba]

On Wed, Jun 28, 2023 at 07:56:11PM +0800, Anand Jain wrote:
> Update the Documentation/btrfstune.rst to carry the new --device
> option.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  Documentation/btrfstune.rst | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/btrfstune.rst b/Documentation/btrfstune.rst
> index 0510ad1f4c26..89f4494bbaf0 100644
> --- a/Documentation/btrfstune.rst
> +++ b/Documentation/btrfstune.rst
> @@ -46,6 +46,9 @@ OPTIONS
>          Allow dangerous changes, e.g. clear the seeding flag or change fsid.
>          Make sure that you are aware of the dangers.
>  
> +--device

Options that take a value should also document it.

> +        List of block devices or regular files that are part of the filesystem.
> +
>  -m
>          (since kernel: 5.0)
>  
> -- 
> 2.31.1

Re: [PATCH 04/10] btrfs-progs: docs: update btrfstune --device option

[Anand Jain]

On 14/07/2023 02:41, David Sterba wrote:
> On Wed, Jun 28, 2023 at 07:56:11PM +0800, Anand Jain wrote:
>> Update the Documentation/btrfstune.rst to carry the new --device
>> option.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   Documentation/btrfstune.rst | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/btrfstune.rst b/Documentation/btrfstune.rst
>> index 0510ad1f4c26..89f4494bbaf0 100644
>> --- a/Documentation/btrfstune.rst
>> +++ b/Documentation/btrfstune.rst
>> @@ -46,6 +46,9 @@ OPTIONS
>>           Allow dangerous changes, e.g. clear the seeding flag or change fsid.
>>           Make sure that you are aware of the dangers.
>>   
>> +--device
> 
> Options that take a value should also document it.

Oh. yes. Now added for both tune and check.

Thanks.

> 
>> +        List of block devices or regular files that are part of the filesystem.
>> +
>>   -m
>>           (since kernel: 5.0)
>>   
>> -- 
>> 2.31.1

[PATCH 05/10] btrfs-progs: tune: introduce --noscan option

[Anand Jain]

The function check_where_mounted() scans the system for other btrfs
devices, but in certain cases, we may need a way to instruct
btrfstune not to perform the system scan and instead only work on the
devices provided through the command line. And so, add an option
--noscan.

For example:

  $ mkfs.btrfs -fq -draid0 -mraid0 ./td1 ./td2

  $ btrfstune -m ./td1
	  warning, device 2 is missing
	  ERROR: could not setup extent tree
	  ERROR: open ctree failed

  $ losetup --find --show ./td2
	/dev/loop4
  $ btrfstune -m ./td1

	Or just
  $ btrfstune -m --device ./td1 ./td2

  The 'noscan' option is optional because there may be scenarios where we
  have a copy that we don't want to modify the fsid. In the following
  scenario, we keep 'td2' out of the metadata_uuid changes, even though
  its loop device is created.

  $ cp td2 td3
  $ btrsftune --noscan --device ./td3 -m ./td1

Thanks

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tune/main.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tune/main.c b/tune/main.c
index 63e3ecc934cc..570e3908ef8a 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -119,6 +119,7 @@ static const char * const tune_usage[] = {
 	"General:",
 	OPTLINE("-f", "allow dangerous operations, make sure that you are aware of the dangers"),
 	OPTLINE("--device", "devices or regular-files of the filesystem to be scanned"),
+	OPTLINE("--noscan", "do not scan the devices from the system, use only the listed ones"),
 	OPTLINE("--help", "print this help"),
 #if EXPERIMENTAL
 	"",
@@ -145,6 +146,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	bool to_extent_tree = false;
 	bool to_bg_tree = false;
 	bool to_fst = false;
+	bool noscan = false;
 	int csum_type = -1;
 	int argc_devices = 0;
 	char **argv_devices = NULL;
@@ -160,7 +162,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		       GETOPT_VAL_ENABLE_BLOCK_GROUP_TREE,
 		       GETOPT_VAL_DISABLE_BLOCK_GROUP_TREE,
 		       GETOPT_VAL_ENABLE_FREE_SPACE_TREE,
-		       GETOPT_VAL_DEVICE };
+		       GETOPT_VAL_DEVICE, GETOPT_VAL_NOSCAN };
 		static const struct option long_options[] = {
 			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
 			{ "convert-to-block-group-tree", no_argument, NULL,
@@ -173,6 +175,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 			{ "csum", required_argument, NULL, GETOPT_VAL_CSUM },
 #endif
 			{ "device", required_argument, NULL, GETOPT_VAL_DEVICE },
+			{ "noscan", no_argument, NULL, GETOPT_VAL_NOSCAN },
 			{ NULL, 0, NULL, 0 }
 		};
 		int c = getopt_long(argc, argv, "S:rxfuU:nmM:", long_options, NULL);
@@ -245,6 +248,10 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 			csum_type = parse_csum_type(optarg);
 			break;
 #endif
+		case GETOPT_VAL_NOSCAN:
+			ctree_flags |= OPEN_CTREE_NO_DEVICES;
+			noscan = true;
+			break;
 		case GETOPT_VAL_HELP:
 		default:
 			usage(&tune_cmd, c != GETOPT_VAL_HELP);
@@ -292,7 +299,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	}
 
 	ret = check_mounted_where(fd, device, NULL, 0, NULL,
-				  SBREAD_IGNORE_FSID_MISMATCH, false);
+				  SBREAD_IGNORE_FSID_MISMATCH, noscan);
 	if (ret < 0) {
 		errno = -ret;
 		error("could not check mount status of %s: %m", device);
@@ -317,7 +324,6 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	}
 
 	root = open_ctree_fd(fd, device, 0, ctree_flags);
-
 	if (!root) {
 		error("open ctree failed");
 		ret = 1;
-- 
2.31.1

[PATCH 06/10] btrfs-progs: docs: update btrfstune --noscan option

[Anand Jain]

Update the Documentation/btrfstune.rst to carry the new --noscan
option.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 Documentation/btrfstune.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/btrfstune.rst b/Documentation/btrfstune.rst
index 89f4494bbaf0..71a05506ab13 100644
--- a/Documentation/btrfstune.rst
+++ b/Documentation/btrfstune.rst
@@ -49,6 +49,10 @@ OPTIONS
 --device
         List of block devices or regular files that are part of the filesystem.
 
+--noscan
+        Do not automatically scan the system for other devices of the same
+        filesystem, only use the devices provided as the arguments.
+
 -m
         (since kernel: 5.0)
 
-- 
2.31.1

[PATCH 07/10] btrfs-progs: check: introduce --device option

[Anand Jain]

As of now, btrfs check only accepts one device from the command line and
then scans the system to find other parter devices if any.

However, this method mandates always accessing the file raw image as a
loop device.

This patch modifies btrfs check to accept other devices or reg-files from
the command line using an option --device and scans/registers them.

    For example:

            btrfs check --device /tdev/td1,/tdev/td2 /tdev/td3
      or
            btrfs check --device /tdev/td1 --device /tdev/td2 /tdev/td3

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 check/main.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/check/main.c b/check/main.c
index 2f4fa5ada339..7eb57f10aded 100644
--- a/check/main.c
+++ b/check/main.c
@@ -51,6 +51,7 @@
 #include "common/messages.h"
 #include "common/task-utils.h"
 #include "common/device-utils.h"
+#include "common/device-scan.h"
 #include "common/utils.h"
 #include "common/rbtree-utils.h"
 #include "common/help.h"
@@ -9953,6 +9954,7 @@ static const char * const cmd_check_usage[] = {
 	OPTLINE("-b|--backup", "use the first valid backup root copy"),
 	OPTLINE("-r|--tree-root <bytenr>", "use the given bytenr for the tree root"),
 	OPTLINE("--chunk-root <bytenr>", "use the given bytenr for the chunk tree root"),
+	OPTLINE("--device <device>", "use the given device or regular-file"),
 	"",
 	"Operation modes:",
 	OPTLINE("--readonly", "run in read-only mode (default)"),
@@ -9989,6 +9991,8 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 	u64 tree_root_bytenr = 0;
 	u64 chunk_root_bytenr = 0;
 	char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
+	char **argv_devices = NULL;
+	int argc_devices = 0;
 	int ret = 0;
 	int err = 0;
 	u64 num;
@@ -10010,7 +10014,8 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 			GETOPT_VAL_INIT_EXTENT, GETOPT_VAL_CHECK_CSUM,
 			GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE,
 			GETOPT_VAL_MODE, GETOPT_VAL_CLEAR_SPACE_CACHE,
-			GETOPT_VAL_CLEAR_INO_CACHE, GETOPT_VAL_FORCE };
+			GETOPT_VAL_CLEAR_INO_CACHE, GETOPT_VAL_FORCE,
+			GETOPT_VAL_DEVICE };
 		static const struct option long_options[] = {
 			{ "super", required_argument, NULL, 's' },
 			{ "repair", no_argument, NULL, GETOPT_VAL_REPAIR },
@@ -10035,6 +10040,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 			{ "clear-ino-cache", no_argument , NULL,
 				GETOPT_VAL_CLEAR_INO_CACHE},
 			{ "force", no_argument, NULL, GETOPT_VAL_FORCE },
+			{ "device", required_argument, NULL, GETOPT_VAL_DEVICE },
 			{ NULL, 0, NULL, 0}
 		};
 
@@ -10126,6 +10132,22 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 			case GETOPT_VAL_FORCE:
 				force = 1;
 				break;
+			case GETOPT_VAL_DEVICE:
+				if (!argv_devices) {
+					argv_devices = malloc(sizeof(char *));
+					if (!argv_devices) {
+						error("memory alloc failed");
+						exit(1);
+					}
+				}
+
+				if (!array_append(argv_devices, optarg,
+						  &argc_devices)) {
+					error("memory alloc failed");
+					free(argv_devices);
+					exit(1);
+				}
+				break;
 		}
 	}
 
@@ -10200,6 +10222,19 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 		ctree_flags &= ~OPEN_CTREE_EXCLUSIVE;
 	}
 
+	/*
+	 * check_mounted_where() with noscan == true frees the scanned devices
+	 * scan the command line provided device list now.
+	 */
+	if (argv_devices) {
+		ret = btrfs_scan_argv_devices(0, argc_devices, argv_devices);
+		if (ret) {
+			ret = -EIO;
+			err |= !!ret;
+			goto free_out;
+		}
+	}
+
 	/* only allow partial opening under repair mode */
 	if (opt_check_repair)
 		ctree_flags |= OPEN_CTREE_PARTIAL;
@@ -10214,7 +10249,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 		error("cannot open file system");
 		ret = -EIO;
 		err |= !!ret;
-		goto err_out;
+		goto free_out;
 	}
 
 	root = gfs_info->fs_root;
@@ -10560,6 +10595,8 @@ out:
 	free_root_recs_tree(&root_cache);
 close_out:
 	close_ctree(root);
+free_out:
+	free_array(argv_devices, argc_devices);
 err_out:
 	if (g_task_ctx.progress_enabled)
 		task_deinit(g_task_ctx.info);
-- 
2.31.1

[PATCH 08/10] btrfs-progs: docs: update btrfs check --device option

[Anand Jain]

Update the Documentation/btrfs-check.rst to carry the new --device
option.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 Documentation/btrfs-check.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/btrfs-check.rst b/Documentation/btrfs-check.rst
index 6ade9d51b4d2..bfdd48bbcbb7 100644
--- a/Documentation/btrfs-check.rst
+++ b/Documentation/btrfs-check.rst
@@ -86,6 +86,8 @@ SAFE OR ADVISORY OPTIONS
 --clear-ino-cache
         remove leftover items pertaining to the deprecated inode map feature
 
+--device
+        List of block devices or regular files that are part of the filesystem.
 
 DANGEROUS OPTIONS
 -----------------
-- 
2.31.1

[PATCH 09/10] btrfs-progs: check: introduce --noscan option

[Anand Jain]

The function check_where_mounted() scans the system for other btrfs
devices, but in certain cases, we may need a way to instruct
btrfs check not to perform the system scan and instead only work on the
devices provided through the command line. And so, add an option
--noscan.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 check/main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/check/main.c b/check/main.c
index 7eb57f10aded..d4aa0db2514a 100644
--- a/check/main.c
+++ b/check/main.c
@@ -10015,7 +10015,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 			GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE,
 			GETOPT_VAL_MODE, GETOPT_VAL_CLEAR_SPACE_CACHE,
 			GETOPT_VAL_CLEAR_INO_CACHE, GETOPT_VAL_FORCE,
-			GETOPT_VAL_DEVICE };
+			GETOPT_VAL_DEVICE, GETOPT_VAL_NOSCAN };
 		static const struct option long_options[] = {
 			{ "super", required_argument, NULL, 's' },
 			{ "repair", no_argument, NULL, GETOPT_VAL_REPAIR },
@@ -10041,6 +10041,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 				GETOPT_VAL_CLEAR_INO_CACHE},
 			{ "force", no_argument, NULL, GETOPT_VAL_FORCE },
 			{ "device", required_argument, NULL, GETOPT_VAL_DEVICE },
+			{ "noscan", no_argument, NULL, GETOPT_VAL_NOSCAN },
 			{ NULL, 0, NULL, 0}
 		};
 
@@ -10148,6 +10149,9 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 					exit(1);
 				}
 				break;
+			case GETOPT_VAL_NOSCAN:
+				ctree_flags |= OPEN_CTREE_NO_DEVICES;
+				break;
 		}
 	}
 
-- 
2.31.1

[PATCH 10/10] btrfs-progs: docs: update btrfs check --noscan option

[Anand Jain]

Update the Documentation/btrsf-check.rst to carry the new --noscan
option.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 Documentation/btrfs-check.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/btrfs-check.rst b/Documentation/btrfs-check.rst
index bfdd48bbcbb7..5f4f724111b9 100644
--- a/Documentation/btrfs-check.rst
+++ b/Documentation/btrfs-check.rst
@@ -89,6 +89,10 @@ SAFE OR ADVISORY OPTIONS
 --device
         List of block devices or regular files that are part of the filesystem.
 
+--noscan
+        Do not automatically scan the system for other devices for the same
+        filesystem, only use the devices provided as the arguments.
+
 DANGEROUS OPTIONS
 -----------------
 
-- 
2.31.1

Re: [PATCH 00/10] btrfs-progs: check and tune: add device and noscan options

[David Sterba]

On Wed, Jun 28, 2023 at 07:56:07PM +0800, Anand Jain wrote:
> By default, btrfstune and btrfs check scans all and only the block devices
> in the system.
> 
> To scan regular files without mapping them to a loop device, adds the
> --device option.
> 
> To indicate not to scan the system for other devices, adds the --noscan
> option.
> 
> For example:
> 
>   The command below will scan both regular files and the devices
>   provided in the --device option, along with the system block devices.
> 
>         btrfstune -m --device /tdev/td1,/tdev/td2 /tdev/td3
>   or
>         btrfs check --device /tdev/td1 --device /tdev/td2 /tdev/td3
> 
>   In some cases, if you need to avoid the default system scan for the
>   block device, you can use the --noscan option.
> 
>         btrfstune -m --noscan --device /tdev/td1,/tdev/td2 /tdev/td3
> 
>         btrfs check --noscan --device /tdev/td1,/tdev/td2 /tdev/td3

From the examples above I don't understand which devices get scanned or
not, there are the --device ones and then the agtument. Also for
examples please use something recognizable like /dev/sdx, /dev/sdy,
otherwise it looks like it requires some special type of device.

I'd expect that --noscan will not scan any device that is part of the
filesystem that is pointed to by the agrument (/tdev/td3 in this case).

If the option --noscan + --device are meant to work together that only
the given devices are either scanned or not (this is the part I don't
see clearly), then I'd suggest to let --noscan take a value of device
that won't be scanned.

Eventually there could be a special value --noscan=all to not scan all
devices. Also please drop the syntax where the devices are separated by
",", one option per device should work for everybody and you can avoid
parsting the option value.

Re: [PATCH 00/10] btrfs-progs: check and tune: add device and noscan options

[Anand Jain]

On 14/07/2023 02:35, David Sterba wrote:
> On Wed, Jun 28, 2023 at 07:56:07PM +0800, Anand Jain wrote:
>> By default, btrfstune and btrfs check scans all and only the block devices
>> in the system.
>>
>> To scan regular files without mapping them to a loop device, adds the
>> --device option.
>>
>> To indicate not to scan the system for other devices, adds the --noscan
>> option.
>>
>> For example:
>>
>>    The command below will scan both regular files and the devices
>>    provided in the --device option, along with the system block devices.
>>
>>          btrfstune -m --device /tdev/td1,/tdev/td2 /tdev/td3
>>    or
>>          btrfs check --device /tdev/td1 --device /tdev/td2 /tdev/td3
>>
>>    In some cases, if you need to avoid the default system scan for the
>>    block device, you can use the --noscan option.
>>
>>          btrfstune -m --noscan --device /tdev/td1,/tdev/td2 /tdev/td3
>>
>>          btrfs check --noscan --device /tdev/td1,/tdev/td2 /tdev/td3
> 
>  From the examples above I don't understand which devices get scanned or
> not, there are the --device ones and then the agtument. Also for
> examples please use something recognizable like /dev/sdx, /dev/sdy,
> otherwise it looks like it requires some special type of device.

Let me try again..

The --noscan option in btrfstune is designed to disable automatic 
system-wide scanning + detection and assembly of Btrfs devices.
Instead, it utilizes the devices provided in the command line argument,
at the option --device and the last argument.

I used devices like "/tdev/td1," and so on to indicate that it's
not just the block device as an argument but also the regular file
images. Seems like I didn't explain it clearly. I'll try update this.


> I'd expect that --noscan will not scan any device that is part of the
> filesystem that is pointed to by the agrument (/tdev/td3 in this case).

Indeed true, except for the devices specified in the --device option and 
the last argument of the 'btrfstune' command. I propose this approach 
due to the split brain (CHANGING_FSID) scenario, where devices to be 
assembled may have different fsids but matching metadata_uuid, making 
automatic assembly unsafe.


In my upcoming kernel patch, I have a test case that demonstrates bug in 
the kernel's automatic device assembly during the split brain scenarios 
and as below:

------------
Consider two filesystems with two unique fsids.

	267baf3a-d949-4e8d-bb09-79edab766d39 (/dev/loop0 /dev/loop1)
	b698d8f4-cc56-4480-b202-c29882cfa13e (/dev/loop2 /dev/loop3)

The metadata_uuid (271ca08e-ed4c-4c80-8919-fe59533a635b) is the same
for both fsids.

Suppose 267baf3a-d949-4e8d-bb09-79edab766d39 attempts to change its fsid 
using 'btrfstune -m' option, but encounters a write failure on devid 1.

	$ /incomplete-fsid-change/btrfstune -m /dev/loop0
	dbg: Wrote devid 2 skip_write_sb_cnt 0
	dbg: Wrote devid 1 skip_write_sb_cnt 1
	dbg: Wrote devid 2 skip_write_sb_cnt 1
	dbg: SKIP devid 1 skip_write_sb_cnt 2  <-- Last sb write failed


Check if one of the device has CHANGING_FSID set.
Also, note that
  All devices /dev/loop[0-3] have same metadata_uuid.
  /dev/loop[0-1] have different fsids.
  /dev/loop[2-3] have greater generation number.


	$ cat sb
	btrfs inspect dump-super $1 | \
			egrep
	"device|devid|^metadata_uuid|^fsid|^incom|^generation|^flags"



	$ sb /dev/loo0
superblock: bytenr=65536, device=/dev/loop0
flags			0x1000000001   <-- CHANGEING_FSID_V2
fsid			267baf3a-d949-4e8d-bb09-79edab766d39 <--
metadata_uuid		271ca08e-ed4c-4c80-8919-fe59533a635b
generation		13
num_devices		2
incompat_flags		0x761
dev_item.devid		1

	$ sb /dev/loop1
superblock: bytenr=65536, device=/dev/loop1
flags			0x1
fsid			54ba1e89-b754-4a18-b464-0cc5ec47187a <--
metadata_uuid		271ca08e-ed4c-4c80-8919-fe59533a635b
generation		14 <--
num_devices		2
incompat_flags		0x761
dev_item.devid		2

	$ sb /dev/loop2
superblock: bytenr=65536, device=/dev/loop2
flags			0x1
fsid			b698d8f4-cc56-4480-b202-c29882cfa13e
metadata_uuid		271ca08e-ed4c-4c80-8919-fe59533a635b
generation		15 <--
num_devices		2
incompat_flags		0x761
dev_item.devid		1

	$ sb /dev/loop3
superblock: bytenr=65536, device=/dev/loop3
flags			0x1
fsid			b698d8f4-cc56-4480-b202-c29882cfa13e
metadata_uuid		271ca08e-ed4c-4c80-8919-fe59533a635b
generation		15
num_devices		2
incompat_flags		0x761
dev_item.devid		2



Despite /dev/loop0 and /dev/loop3 not belonging to the same fsid, in the
context of recovering from the CHANGING_FSID scenario, kernel can
assemble them together.

$ btrfs dev scan --forget
$ btrfs dev scan /dev/loop3
Scanning for btrfs filesystems on '/dev/loop3'

$ mount /dev/loop0 /btrfs
$ btrfs fi show -m /btrfs
Label: none  uuid: b698d8f4-cc56-4480-b202-c29882cfa13e
	Total devices 2 FS bytes used 144.00KiB
	devid    1 size 300.00MiB used 48.00MiB path /dev/loop0  <--
	devid    2 size 300.00MiB used 40.00MiB path /dev/loop3  <--


[  984.959694] BTRFS error (device loop3): parent transid verify failed 
on 30507008 wanted 15 found 14


In my view, automatic device assembly is unsafe. Devices with the
CHANGING_FSID flag should be treated similar to a filesystem needing
fsck and fix or assemble them back in the userland.
----------------

> If the option --noscan + --device are meant to work together that only
> the given devices are either scanned or not (this is the part I don't
> see clearly),

Only the specified devices are scanned and used for assembling the
volume if their metadata_uuids match and devids line up.

> then I'd suggest to let --noscan take a value of device
> that won't be scanned.

Oh. Do you mean something like, for example:

  (not related to any of the test case):

     btrfstune -m --noscan=/dev/sda --noscan=/dev/sdb  /dev/sdc

I'm okay, but I think we need to rename 'noscan' to something else?

> 
> Eventually there could be a special value --noscan=all to not scan all
> devices.

--noscan and --device options are also present in the
"btrfs inspect dump-tree" sub-command.
I hope its different usage won't confuse the users.

> Also please drop the syntax where the devices are separated by
> ",", one option per device should work for everybody and you can avoid
> parsting the option value.

I kept it the same as in the mount -o device option;
I'm ok dropping support for ','.

Thanks, Anand