[PATCH 0/9] btrfs-progs: btrfstune: accept multiple devices and cleanup

[PATCH 0/9] btrfs-progs: btrfstune: accept multiple devices and cleanup

[Anand Jain]

In an attempt to enable btrfstune to accept multiple devices from the
command line, this patch includes some cleanup around the related code
and functions.

Patches 1 to 5 primarily consist of cleanups. Patches 6 and 8 serve as
preparatory changes. Patch 7 enables btrfstune to accept multiple
devices. Patch 9 ensures that btrfstune no longer automatically uses the
system block devices when --noscan option is specified.
Patches 10 and 11 are help and documentation part.

Anand Jain (11):
  btrfs-progs: check_mounted_where declare is_btrfs as bool
  btrfs-progs: check_mounted_where pack varibles type by size
  btrfs-progs: rename struct open_ctree_flags to open_ctree_args
  btrfs-progs: optimize device_list_add
  btrfs-progs: simplify btrfs_scan_one_device()
  btrfs-progs: factor out btrfs_scan_stdin_devices
  btrfs-progs: tune: add stdin device list
  btrfs-progs: refactor check_where_mounted with noscan option
  btrfs-progs: tune: add noscan option
  btrfs-progs: tune: add help for multiple devices and noscan option
  btrfs-progs: Documentation: update btrfstune --noscan option

 Documentation/btrfstune.rst |  4 ++++
 btrfs-find-root.c           |  2 +-
 check/main.c                |  2 +-
 cmds/filesystem.c           |  2 +-
 cmds/inspect-dump-tree.c    | 39 ++++---------------------------------
 cmds/rescue.c               |  4 ++--
 cmds/restore.c              |  2 +-
 common/device-scan.c        | 39 +++++++++++++++++++++++++++++++++++++
 common/device-scan.h        |  1 +
 common/open-utils.c         | 21 +++++++++++---------
 common/open-utils.h         |  3 ++-
 common/utils.c              |  3 ++-
 image/main.c                |  4 ++--
 kernel-shared/disk-io.c     |  8 ++++----
 kernel-shared/disk-io.h     |  4 ++--
 kernel-shared/volumes.c     | 14 +++++--------
 mkfs/main.c                 |  2 +-
 tune/main.c                 | 25 +++++++++++++++++++-----
 18 files changed, 104 insertions(+), 75 deletions(-)

-- 
2.31.1

[PATCH 01/11] btrfs-progs: check_mounted_where declare is_btrfs as bool

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/open-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/open-utils.c b/common/open-utils.c
index d4fdb2b01c7f..01d747d8ac43 100644
--- a/common/open-utils.c
+++ b/common/open-utils.c
@@ -57,7 +57,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
 {
 	int ret;
 	u64 total_devs = 1;
-	int is_btrfs;
+	bool is_btrfs;
 	struct btrfs_fs_devices *fs_devices_mnt = NULL;
 	FILE *f;
 	struct mntent *mnt;
-- 
2.31.1

[PATCH 02/11] btrfs-progs: check_mounted_where pack varibles type by size

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/open-utils.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/open-utils.c b/common/open-utils.c
index 01d747d8ac43..1e18fa905b51 100644
--- a/common/open-utils.c
+++ b/common/open-utils.c
@@ -55,16 +55,16 @@ static int blk_file_in_dev_list(struct btrfs_fs_devices* fs_devices,
 int check_mounted_where(int fd, const char *file, char *where, int size,
 			struct btrfs_fs_devices **fs_dev_ret, unsigned sbflags)
 {
-	int ret;
-	u64 total_devs = 1;
-	bool is_btrfs;
 	struct btrfs_fs_devices *fs_devices_mnt = NULL;
-	FILE *f;
 	struct mntent *mnt;
+	u64 total_devs = 1;
+	FILE *f;
+	int ret;
+	bool is_btrfs;
 
 	/* scan the initial device */
-	ret = btrfs_scan_one_device(fd, file, &fs_devices_mnt,
-		    &total_devs, BTRFS_SUPER_INFO_OFFSET, sbflags);
+	ret = btrfs_scan_one_device(fd, file, &fs_devices_mnt, &total_devs,
+				    BTRFS_SUPER_INFO_OFFSET, sbflags);
 	is_btrfs = (ret >= 0);
 
 	/* scan other devices */
-- 
2.31.1

[PATCH 03/11] btrfs-progs: rename struct open_ctree_flags to open_ctree_args

[Anand Jain]

The struct open_ctree_flags currently holds arguments for
open_ctree_fs_info(), it can be confusing when mixed with a local variable
named open_ctree_flags as below.

cmds/inspect-dump-tree.c:

	cmd_inspect_dump_tree()
	::
	struct open_ctree_flags ocf = { 0 };
	::
	unsigned open_ctree_flags;

So rename struct open_ctree_flags to struct open_ctree_args.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfs-find-root.c        | 2 +-
 check/main.c             | 2 +-
 cmds/filesystem.c        | 2 +-
 cmds/inspect-dump-tree.c | 2 +-
 cmds/rescue.c            | 4 ++--
 cmds/restore.c           | 2 +-
 image/main.c             | 4 ++--
 kernel-shared/disk-io.c  | 8 ++++----
 kernel-shared/disk-io.h  | 4 ++--
 mkfs/main.c              | 2 +-
 10 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 398d7f216ee7..52041d82c182 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -335,7 +335,7 @@ int main(int argc, char **argv)
 	struct btrfs_find_root_filter filter = {0};
 	struct cache_tree result;
 	struct cache_extent *found;
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 	int ret;
 
 	/* Default to search root tree */
diff --git a/check/main.c b/check/main.c
index 7542b49f44f5..f7a2d446370a 100644
--- a/check/main.c
+++ b/check/main.c
@@ -9983,7 +9983,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 {
 	struct cache_tree root_cache;
 	struct btrfs_root *root;
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 	u64 bytenr = 0;
 	u64 subvolid = 0;
 	u64 tree_root_bytenr = 0;
diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 47fd2377f5f4..c9e641b2fa9a 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -636,7 +636,7 @@ static int map_seed_devices(struct list_head *all_uuids)
 	fs_uuids = btrfs_scanned_uuids();
 
 	list_for_each_entry(cur_fs, all_uuids, list) {
-		struct open_ctree_flags ocf = { 0 };
+		struct open_ctree_args ocf = { 0 };
 
 		device = list_first_entry(&cur_fs->devices,
 						struct btrfs_device, dev_list);
diff --git a/cmds/inspect-dump-tree.c b/cmds/inspect-dump-tree.c
index bfc0fff148dd..35920d14b7e9 100644
--- a/cmds/inspect-dump-tree.c
+++ b/cmds/inspect-dump-tree.c
@@ -317,7 +317,7 @@ static int cmd_inspect_dump_tree(const struct cmd_struct *cmd,
 	struct btrfs_disk_key disk_key;
 	struct btrfs_key found_key;
 	struct cache_tree block_root;	/* for multiple --block parameters */
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 	char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
 	int ret = 0;
 	int slot;
diff --git a/cmds/rescue.c b/cmds/rescue.c
index 5551374d4b75..aee5446e66d0 100644
--- a/cmds/rescue.c
+++ b/cmds/rescue.c
@@ -233,7 +233,7 @@ static int cmd_rescue_fix_device_size(const struct cmd_struct *cmd,
 				      int argc, char **argv)
 {
 	struct btrfs_fs_info *fs_info;
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 	char *devname;
 	int ret;
 
@@ -368,7 +368,7 @@ static int cmd_rescue_clear_uuid_tree(const struct cmd_struct *cmd,
 				      int argc, char **argv)
 {
 	struct btrfs_fs_info *fs_info;
-	struct open_ctree_flags ocf = {};
+	struct open_ctree_args ocf = {};
 	char *devname;
 	int ret;
 
diff --git a/cmds/restore.c b/cmds/restore.c
index 9fe7b4d2d07d..aa78d0799c4a 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -1216,7 +1216,7 @@ static struct btrfs_root *open_fs(const char *dev, u64 root_location,
 {
 	struct btrfs_fs_info *fs_info = NULL;
 	struct btrfs_root *root = NULL;
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 	u64 bytenr;
 	int i;
 
diff --git a/image/main.c b/image/main.c
index 50c3f2ca7db5..9e460e7076e7 100644
--- a/image/main.c
+++ b/image/main.c
@@ -2795,7 +2795,7 @@ static int restore_metadump(const char *input, FILE *out, int old_restore,
 
 	/* NOTE: open with write mode */
 	if (fixup_offset) {
-		struct open_ctree_flags ocf = { 0 };
+		struct open_ctree_args ocf = { 0 };
 
 		ocf.filename = target;
 		ocf.flags = OPEN_CTREE_WRITES | OPEN_CTREE_RESTORE |
@@ -3223,7 +3223,7 @@ int BOX_MAIN(image)(int argc, char *argv[])
 
 	 /* extended support for multiple devices */
 	if (!create && multi_devices) {
-		struct open_ctree_flags ocf = { 0 };
+		struct open_ctree_args ocf = { 0 };
 		struct btrfs_fs_info *info;
 		u64 total_devs;
 		int i;
diff --git a/kernel-shared/disk-io.c b/kernel-shared/disk-io.c
index 442d3af8bc01..3b709b2c0f7f 100644
--- a/kernel-shared/disk-io.c
+++ b/kernel-shared/disk-io.c
@@ -1437,7 +1437,7 @@ int btrfs_setup_chunk_tree_and_device_map(struct btrfs_fs_info *fs_info,
 	return 0;
 }
 
-static struct btrfs_fs_info *__open_ctree_fd(int fp, struct open_ctree_flags *ocf)
+static struct btrfs_fs_info *__open_ctree_fd(int fp, struct open_ctree_args *ocf)
 {
 	struct btrfs_fs_info *fs_info;
 	struct btrfs_super_block *disk_super;
@@ -1608,7 +1608,7 @@ out:
 	return NULL;
 }
 
-struct btrfs_fs_info *open_ctree_fs_info(struct open_ctree_flags *ocf)
+struct btrfs_fs_info *open_ctree_fs_info(struct open_ctree_args *ocf)
 {
 	int fp;
 	int ret;
@@ -1646,7 +1646,7 @@ struct btrfs_root *open_ctree(const char *filename, u64 sb_bytenr,
 			      unsigned flags)
 {
 	struct btrfs_fs_info *info;
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 
 	/* This flags may not return fs_info with any valid root */
 	BUG_ON(flags & OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR);
@@ -1665,7 +1665,7 @@ struct btrfs_root *open_ctree_fd(int fp, const char *path, u64 sb_bytenr,
 				 unsigned flags)
 {
 	struct btrfs_fs_info *info;
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 
 	/* This flags may not return fs_info with any valid root */
 	if (flags & OPEN_CTREE_IGNORE_CHUNK_TREE_ERROR) {
diff --git a/kernel-shared/disk-io.h b/kernel-shared/disk-io.h
index 3a31667967cc..93572c4297ad 100644
--- a/kernel-shared/disk-io.h
+++ b/kernel-shared/disk-io.h
@@ -175,7 +175,7 @@ struct btrfs_root *open_ctree(const char *filename, u64 sb_bytenr,
 			      unsigned flags);
 struct btrfs_root *open_ctree_fd(int fp, const char *path, u64 sb_bytenr,
 				 unsigned flags);
-struct open_ctree_flags {
+struct open_ctree_args {
 	const char *filename;
 	u64 sb_bytenr;
 	u64 root_tree_bytenr;
@@ -183,7 +183,7 @@ struct open_ctree_flags {
 	unsigned flags;
 };
 
-struct btrfs_fs_info *open_ctree_fs_info(struct open_ctree_flags *ocf);
+struct btrfs_fs_info *open_ctree_fs_info(struct open_ctree_args *ctree_args);
 int close_ctree_fs_info(struct btrfs_fs_info *fs_info);
 static inline int close_ctree(struct btrfs_root *root)
 {
diff --git a/mkfs/main.c b/mkfs/main.c
index a31b32c644c9..23db58b7186d 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -990,7 +990,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 	struct btrfs_root *root;
 	struct btrfs_fs_info *fs_info;
 	struct btrfs_trans_handle *trans;
-	struct open_ctree_flags ocf = { 0 };
+	struct open_ctree_args ocf = { 0 };
 	int ret = 0;
 	int close_ret;
 	int i;
-- 
2.31.1

[PATCH 04/11] btrfs-progs: optimize device_list_add

[Anand Jain]

Drop devid argument, instead it can be fetched from the disk_super
argument.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 kernel-shared/volumes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 95d5930b95d8..81abda3f7d1c 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -334,11 +334,12 @@ static struct btrfs_fs_devices *find_fsid(u8 *fsid, u8 *metadata_uuid)
 
 static int device_list_add(const char *path,
 			   struct btrfs_super_block *disk_super,
-			   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
+			   struct btrfs_fs_devices **fs_devices_ret)
 {
 	struct btrfs_device *device;
 	struct btrfs_fs_devices *fs_devices;
 	u64 found_transid = btrfs_super_generation(disk_super);
+	u64 devid = btrfs_stack_device_id(&disk_super->dev_item);
 	bool metadata_uuid = (btrfs_super_incompat_flags(disk_super) &
 		BTRFS_FEATURE_INCOMPAT_METADATA_UUID);
 
@@ -545,18 +546,17 @@ int btrfs_scan_one_device(int fd, const char *path,
 {
 	struct btrfs_super_block disk_super;
 	int ret;
-	u64 devid;
 
 	ret = btrfs_read_dev_super(fd, &disk_super, super_offset, sbflags);
 	if (ret < 0)
 		return -EIO;
-	devid = btrfs_stack_device_id(&disk_super.dev_item);
+
 	if (btrfs_super_flags(&disk_super) & BTRFS_SUPER_FLAG_METADUMP)
 		*total_devs = 1;
 	else
 		*total_devs = btrfs_super_num_devices(&disk_super);
 
-	ret = device_list_add(path, &disk_super, devid, fs_devices_ret);
+	ret = device_list_add(path, &disk_super, fs_devices_ret);
 
 	return ret;
 }
-- 
2.31.1

[PATCH 05/11] btrfs-progs: simplify btrfs_scan_one_device()

[Anand Jain]

Local variable int ret is unnecessary, drop it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 kernel-shared/volumes.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 81abda3f7d1c..c8053ae1c7f7 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -545,10 +545,8 @@ int btrfs_scan_one_device(int fd, const char *path,
 			  u64 *total_devs, u64 super_offset, unsigned sbflags)
 {
 	struct btrfs_super_block disk_super;
-	int ret;
 
-	ret = btrfs_read_dev_super(fd, &disk_super, super_offset, sbflags);
-	if (ret < 0)
+	if (btrfs_read_dev_super(fd, &disk_super, super_offset, sbflags) < 0)
 		return -EIO;
 
 	if (btrfs_super_flags(&disk_super) & BTRFS_SUPER_FLAG_METADUMP)
@@ -556,9 +554,7 @@ int btrfs_scan_one_device(int fd, const char *path,
 	else
 		*total_devs = btrfs_super_num_devices(&disk_super);
 
-	ret = device_list_add(path, &disk_super, fs_devices_ret);
-
-	return ret;
+	return device_list_add(path, &disk_super, fs_devices_ret);
 }
 
 static u64 dev_extent_search_start(struct btrfs_device *device, u64 start)
-- 
2.31.1

[PATCH 06/11] btrfs-progs: factor out btrfs_scan_stdin_devices

[Anand Jain]

As a preparatory to take btrfstune device list from the arguments factor
out btrfs_scan_stdin_devices().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds/inspect-dump-tree.c | 37 +++----------------------------------
 common/device-scan.c     | 39 +++++++++++++++++++++++++++++++++++++++
 common/device-scan.h     |  1 +
 3 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/cmds/inspect-dump-tree.c b/cmds/inspect-dump-tree.c
index 35920d14b7e9..311dfbfddab6 100644
--- a/cmds/inspect-dump-tree.c
+++ b/cmds/inspect-dump-tree.c
@@ -327,7 +327,6 @@ static int cmd_inspect_dump_tree(const struct cmd_struct *cmd,
 	bool roots_only = false;
 	bool root_backups = false;
 	int traverse = BTRFS_PRINT_TREE_DEFAULT;
-	int dev_optind;
 	unsigned open_ctree_flags;
 	u64 block_bytenr;
 	struct btrfs_root *tree_root_scan;
@@ -456,39 +455,9 @@ static int cmd_inspect_dump_tree(const struct cmd_struct *cmd,
 	if (check_argc_min(argc - optind, 1))
 		return 1;
 
-	dev_optind = optind;
-	while (dev_optind < argc) {
-		int fd;
-		struct btrfs_fs_devices *fs_devices;
-		u64 num_devices;
-
-		ret = check_arg_type(argv[optind]);
-		if (ret != BTRFS_ARG_BLKDEV && ret != BTRFS_ARG_REG) {
-			if (ret < 0) {
-				errno = -ret;
-				error("invalid argument %s: %m", argv[dev_optind]);
-			} else {
-				error("not a block device or regular file: %s",
-				       argv[dev_optind]);
-			}
-		}
-		fd = open(argv[dev_optind], O_RDONLY);
-		if (fd < 0) {
-			error("cannot open %s: %m", argv[dev_optind]);
-			return -EINVAL;
-		}
-		ret = btrfs_scan_one_device(fd, argv[dev_optind], &fs_devices,
-					    &num_devices,
-					    BTRFS_SUPER_INFO_OFFSET,
-					    SBREAD_DEFAULT);
-		close(fd);
-		if (ret < 0) {
-			errno = -ret;
-			error("device scan %s: %m", argv[dev_optind]);
-			return ret;
-		}
-		dev_optind++;
-	}
+	ret = btrfs_scan_stdin_devices(optind, argc, argv);
+	if (ret)
+		return ret;
 
 	pr_verbose(LOG_DEFAULT, "%s\n", PACKAGE_STRING);
 
diff --git a/common/device-scan.c b/common/device-scan.c
index 515481a6efa9..38f986df810f 100644
--- a/common/device-scan.c
+++ b/common/device-scan.c
@@ -500,3 +500,42 @@ int btrfs_scan_devices(int verbose)
 	return 0;
 }
 
+int btrfs_scan_stdin_devices(int dev_optind, int argc, char **argv)
+{
+	int ret;
+
+	while (dev_optind < argc) {
+		int fd;
+		u64 num_devices;
+		struct btrfs_fs_devices *fs_devices;
+
+		ret = check_arg_type(argv[optind]);
+		if (ret != BTRFS_ARG_BLKDEV && ret != BTRFS_ARG_REG) {
+			if (ret < 0) {
+				errno = -ret;
+				error("invalid argument %s: %m", argv[dev_optind]);
+			} else {
+				error("not a block device or regular file: %s",
+				       argv[dev_optind]);
+			}
+		}
+		fd = open(argv[dev_optind], O_RDONLY);
+		if (fd < 0) {
+			error("cannot open %s: %m", argv[dev_optind]);
+			return -EINVAL;
+		}
+		ret = btrfs_scan_one_device(fd, argv[dev_optind], &fs_devices,
+					    &num_devices,
+					    BTRFS_SUPER_INFO_OFFSET,
+					    SBREAD_DEFAULT);
+		close(fd);
+		if (ret < 0) {
+			errno = -ret;
+			error("device scan %s: %m", argv[dev_optind]);
+			return ret;
+		}
+		dev_optind++;
+	}
+
+	return 0;
+}
diff --git a/common/device-scan.h b/common/device-scan.h
index f805b489f595..e2480d3eb168 100644
--- a/common/device-scan.h
+++ b/common/device-scan.h
@@ -58,5 +58,6 @@ int add_seen_fsid(u8 *fsid, struct seen_fsid *seen_fsid_hash[],
 		int fd, DIR *dirstream);
 void free_seen_fsid(struct seen_fsid *seen_fsid_hash[]);
 int test_uuid_unique(const char *uuid_str);
+int btrfs_scan_stdin_devices(int dev_optind, int argc, char **argv);
 
 #endif
-- 
2.31.1

[PATCH 07/11] btrfs-progs: tune: add stdin device list

[Anand Jain]

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

diff --git a/tune/main.c b/tune/main.c
index e38c1f6d3729..2ea737bd0c0f 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -232,7 +232,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 
 	set_argv0(argv);
 	device = argv[optind];
-	if (check_argc_exact(argc - optind, 1))
+	if (check_argc_min(argc - optind, 1))
 		return 1;
 
 	if (random_fsid && new_fsid_str) {
@@ -280,6 +280,14 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		return 1;
 	}
 
+	/*
+	 * check_mounted_where() with noscan == true frees the scanned devices
+	 * scan the command line provided device list now.
+	 */
+	ret = btrfs_scan_stdin_devices(optind, argc, argv);
+	if (ret)
+		return 1;
+
 	root = open_ctree_fd(fd, device, 0, ctree_flags);
 
 	if (!root) {
-- 
2.31.1

[PATCH 08/11] btrfs-progs: refactor check_where_mounted with noscan option

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/open-utils.c | 9 ++++++---
 common/open-utils.h | 3 ++-
 common/utils.c      | 3 ++-
 tune/main.c         | 2 +-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/common/open-utils.c b/common/open-utils.c
index 1e18fa905b51..e34abee97f60 100644
--- a/common/open-utils.c
+++ b/common/open-utils.c
@@ -53,7 +53,8 @@ static int blk_file_in_dev_list(struct btrfs_fs_devices* fs_devices,
 }
 
 int check_mounted_where(int fd, const char *file, char *where, int size,
-			struct btrfs_fs_devices **fs_dev_ret, unsigned sbflags)
+			struct btrfs_fs_devices **fs_dev_ret, unsigned sbflags,
+			bool noscan)
 {
 	struct btrfs_fs_devices *fs_devices_mnt = NULL;
 	struct mntent *mnt;
@@ -108,6 +109,8 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
 	}
 	if (fs_dev_ret)
 		*fs_dev_ret = fs_devices_mnt;
+	else if (noscan)
+		btrfs_close_all_devices();
 
 	ret = (mnt != NULL);
 
@@ -132,7 +135,7 @@ int check_mounted(const char* file)
 		return -errno;
 	}
 
-	ret =  check_mounted_where(fd, file, NULL, 0, NULL, SBREAD_DEFAULT);
+	ret =  check_mounted_where(fd, file, NULL, 0, NULL, SBREAD_DEFAULT, false);
 	close(fd);
 
 	return ret;
@@ -168,7 +171,7 @@ int get_btrfs_mount(const char *dev, char *mp, size_t mp_size)
 		goto out;
 	}
 
-	ret = check_mounted_where(fd, dev, mp, mp_size, NULL, SBREAD_DEFAULT);
+	ret = check_mounted_where(fd, dev, mp, mp_size, NULL, SBREAD_DEFAULT, false);
 	if (!ret) {
 		ret = -EINVAL;
 	} else { /* mounted, all good */
diff --git a/common/open-utils.h b/common/open-utils.h
index 3924be36e2ea..27000cdbd626 100644
--- a/common/open-utils.h
+++ b/common/open-utils.h
@@ -23,7 +23,8 @@
 struct btrfs_fs_devices;
 
 int check_mounted_where(int fd, const char *file, char *where, int size,
-			struct btrfs_fs_devices **fs_dev_ret, unsigned sbflags);
+			struct btrfs_fs_devices **fs_dev_ret, unsigned sbflags,
+			bool noscan);
 int check_mounted(const char* file);
 int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
 int open_path_or_dev_mnt(const char *path, DIR **dirstream, int verbose);
diff --git a/common/utils.c b/common/utils.c
index 436ff8c2a827..b62f9f04ad5a 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -230,7 +230,8 @@ int get_fs_info(const char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 			goto out;
 		}
 		ret = check_mounted_where(fd, path, mp, sizeof(mp),
-					  &fs_devices_mnt, SBREAD_DEFAULT);
+					  &fs_devices_mnt, SBREAD_DEFAULT,
+					  false);
 		if (!ret) {
 			ret = -EINVAL;
 			goto out;
diff --git a/tune/main.c b/tune/main.c
index 2ea737bd0c0f..9a6223f4aa0c 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -268,7 +268,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 	}
 
 	ret = check_mounted_where(fd, device, NULL, 0, NULL,
-			SBREAD_IGNORE_FSID_MISMATCH);
+				  SBREAD_IGNORE_FSID_MISMATCH, false);
 	if (ret < 0) {
 		errno = -ret;
 		error("could not check mount status of %s: %m", device);
-- 
2.31.1

[PATCH 09/11] btrfs-progs: tune: add noscan option

[Anand Jain]

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 9a6223f4aa0c..fa49f1685e0f 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -143,6 +143,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;
 	char *new_fsid_str = NULL;
 	int ret;
@@ -155,7 +156,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_NOSCAN };
 		static const struct option long_options[] = {
 			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
 			{ "convert-to-block-group-tree", no_argument, NULL,
@@ -167,6 +169,7 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 #if EXPERIMENTAL
 			{ "csum", required_argument, NULL, GETOPT_VAL_CSUM },
 #endif
+			{ "noscan", no_argument, NULL, GETOPT_VAL_NOSCAN },
 			{ NULL, 0, NULL, 0 }
 		};
 		int c = getopt_long(argc, argv, "S:rxfuU:nmM:", long_options, NULL);
@@ -224,6 +227,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);
@@ -268,7 +275,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);
@@ -289,7 +296,6 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
 		return 1;
 
 	root = open_ctree_fd(fd, device, 0, ctree_flags);
-
 	if (!root) {
 		error("open ctree failed");
 		return 1;
-- 
2.31.1

[PATCH 10/11] btrfs-progs: tune: add help for multiple devices and noscan option

[Anand Jain]

Updates btrfstune --help to carry information about the multiple devices
in the command line argument and about the noscan option.

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

diff --git a/tune/main.c b/tune/main.c
index fa49f1685e0f..dd412113ec4c 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -94,7 +94,7 @@ static int convert_to_fst(struct btrfs_fs_info *fs_info)
 }
 
 static const char * const tune_usage[] = {
-	"btrfstune [options] device",
+	"btrfstune [options] device [device...]",
 	"Tune settings of filesystem features on an unmounted device",
 	"",
 	"Options:",
@@ -117,6 +117,7 @@ static const char * const tune_usage[] = {
 	"",
 	"General:",
 	OPTLINE("-f", "allow dangerous operations, make sure that you are aware of the dangers"),
+	OPTLINE("--noscan", "do not scan the devices from the system, use only the listed ones"),
 	OPTLINE("--help", "print this help"),
 #if EXPERIMENTAL
 	"",
-- 
2.31.1

[PATCH 11/11] btrfs-progs: Documentation: update btrfstune --noscan option

[Anand Jain]

Update the Documentation/btrsftune.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 fb643fb8d27a..4cc89de7359f 100644
--- a/Documentation/btrfstune.rst
+++ b/Documentation/btrfstune.rst
@@ -46,6 +46,10 @@ OPTIONS
         Allow dangerous changes, e.g. clear the seeding flag or change fsid.
         Make sure that you are aware of the dangers.
 
+--noscan
+        Do not automatically scan the system for other devices from the same
+        filesystem, only use the devices provided as the arguments.
+
 -m
         (since kernel: 5.0)
 
-- 
2.31.1

Re: [PATCH 0/9] btrfs-progs: btrfstune: accept multiple devices and cleanup

[Anand Jain]

Now I notice that for many patches, the Git changelog is empty,
even though I had it at one time. I am not sure where they went.
I am recreating and resending the whole set again.

Thanks, Anand


On 07/06/2023 17:59, Anand Jain wrote:
> In an attempt to enable btrfstune to accept multiple devices from the
> command line, this patch includes some cleanup around the related code
> and functions.
> 
> Patches 1 to 5 primarily consist of cleanups. Patches 6 and 8 serve as
> preparatory changes. Patch 7 enables btrfstune to accept multiple
> devices. Patch 9 ensures that btrfstune no longer automatically uses the
> system block devices when --noscan option is specified.
> Patches 10 and 11 are help and documentation part.
> 
> Anand Jain (11):
>    btrfs-progs: check_mounted_where declare is_btrfs as bool
>    btrfs-progs: check_mounted_where pack varibles type by size
>    btrfs-progs: rename struct open_ctree_flags to open_ctree_args
>    btrfs-progs: optimize device_list_add
>    btrfs-progs: simplify btrfs_scan_one_device()
>    btrfs-progs: factor out btrfs_scan_stdin_devices
>    btrfs-progs: tune: add stdin device list
>    btrfs-progs: refactor check_where_mounted with noscan option
>    btrfs-progs: tune: add noscan option
>    btrfs-progs: tune: add help for multiple devices and noscan option
>    btrfs-progs: Documentation: update btrfstune --noscan option
> 
>   Documentation/btrfstune.rst |  4 ++++
>   btrfs-find-root.c           |  2 +-
>   check/main.c                |  2 +-
>   cmds/filesystem.c           |  2 +-
>   cmds/inspect-dump-tree.c    | 39 ++++---------------------------------
>   cmds/rescue.c               |  4 ++--
>   cmds/restore.c              |  2 +-
>   common/device-scan.c        | 39 +++++++++++++++++++++++++++++++++++++
>   common/device-scan.h        |  1 +
>   common/open-utils.c         | 21 +++++++++++---------
>   common/open-utils.h         |  3 ++-
>   common/utils.c              |  3 ++-
>   image/main.c                |  4 ++--
>   kernel-shared/disk-io.c     |  8 ++++----
>   kernel-shared/disk-io.h     |  4 ++--
>   kernel-shared/volumes.c     | 14 +++++--------
>   mkfs/main.c                 |  2 +-
>   tune/main.c                 | 25 +++++++++++++++++++-----
>   18 files changed, 104 insertions(+), 75 deletions(-)
> 

Re: [PATCH 0/9] btrfs-progs: btrfstune: accept multiple devices and cleanup

[Qu Wenruo]

On 2023/6/7 17:59, Anand Jain wrote:
> In an attempt to enable btrfstune to accept multiple devices from the
> command line, this patch includes some cleanup around the related code
> and functions.

Mind to share the use case of the new ability?

My concern related to multi-device parameters are:

- What if the provided devices are belonging to different filesystems?
   Should we still do the tune operation on all of them or just the
   first/last device?

- What's the proper error handling if operation on one of the parameter
   failed if we choose to do the tune for all involved devices?
   Should we revert the operation on the succeeded ones?
   Should we continue on the remaining ones?

I understand it's better to add the ability to do manual scan, but it
looks like the multi-device arguments can be a little more complex than
what we thought.

At least I think we should add a dedicate --scan/--device option, and
allow multiple --scan/--device to be provided for device list assembly,
then still keep the single argument to avoid possible confusion.

This also solves the problem I mentioned above. If multiple filesystems
are provided, they are just assembled into device list, won't have an
impact on the tune target.

And since we still have a single device to tune, there is no extra error
handling, nor confusion.

Thanks,
Qu

>
> Patches 1 to 5 primarily consist of cleanups. Patches 6 and 8 serve as
> preparatory changes. Patch 7 enables btrfstune to accept multiple
> devices. Patch 9 ensures that btrfstune no longer automatically uses the
> system block devices when --noscan option is specified.
> Patches 10 and 11 are help and documentation part.
>
> Anand Jain (11):
>    btrfs-progs: check_mounted_where declare is_btrfs as bool
>    btrfs-progs: check_mounted_where pack varibles type by size
>    btrfs-progs: rename struct open_ctree_flags to open_ctree_args
>    btrfs-progs: optimize device_list_add
>    btrfs-progs: simplify btrfs_scan_one_device()
>    btrfs-progs: factor out btrfs_scan_stdin_devices
>    btrfs-progs: tune: add stdin device list
>    btrfs-progs: refactor check_where_mounted with noscan option
>    btrfs-progs: tune: add noscan option
>    btrfs-progs: tune: add help for multiple devices and noscan option
>    btrfs-progs: Documentation: update btrfstune --noscan option
>
>   Documentation/btrfstune.rst |  4 ++++
>   btrfs-find-root.c           |  2 +-
>   check/main.c                |  2 +-
>   cmds/filesystem.c           |  2 +-
>   cmds/inspect-dump-tree.c    | 39 ++++---------------------------------
>   cmds/rescue.c               |  4 ++--
>   cmds/restore.c              |  2 +-
>   common/device-scan.c        | 39 +++++++++++++++++++++++++++++++++++++
>   common/device-scan.h        |  1 +
>   common/open-utils.c         | 21 +++++++++++---------
>   common/open-utils.h         |  3 ++-
>   common/utils.c              |  3 ++-
>   image/main.c                |  4 ++--
>   kernel-shared/disk-io.c     |  8 ++++----
>   kernel-shared/disk-io.h     |  4 ++--
>   kernel-shared/volumes.c     | 14 +++++--------
>   mkfs/main.c                 |  2 +-
>   tune/main.c                 | 25 +++++++++++++++++++-----
>   18 files changed, 104 insertions(+), 75 deletions(-)
>

Re: [PATCH 0/9] btrfs-progs: btrfstune: accept multiple devices and cleanup

[Anand Jain]

On 07/06/2023 19:06, Qu Wenruo wrote:
> 
> 
> On 2023/6/7 17:59, Anand Jain wrote:
>> In an attempt to enable btrfstune to accept multiple devices from the
>> command line, this patch includes some cleanup around the related code
>> and functions.
> 
> Mind to share the use case of the new ability?
> 

  As of now btrfstune works with only one regular file. Not possible
  to use multiple regular files. Unless loop device is used. This
  set fixes this limitation.


> My concern related to multi-device parameters are:
> 
> - What if the provided devices are belonging to different filesystems?
>    Should we still do the tune operation on all of them or just the
>    first/last device?
> 

  Hmm, the scan part remains same with/without this patchset.
  The device_list_add() function organizes the devices based on the fsid.
  Any tool within the btrfs-progs uses this list to obtain the partner5
  device list. This patch set still relies the same thing.

  btrfstune gets the fsid to work on from the first deivce in the list.

  Here is an example:

$ btrfs in dump-super ./td1 ./td2 ./td3 | egrep 
'device=|^fsid|^metadata_uuid'
superblock: bytenr=65536, device=./td1
fsid			c931379a-a119-4eda-a338-badb0a197512
metadata_uuid		f761b688-2642-4c94-be90-22f58e2a66d7
superblock: bytenr=65536, device=./td2
fsid			f9643d74-1d3d-4b0d-b56b-b05ada340f57
metadata_uuid		f761b688-2642-4c94-be90-22f58e2a66d7
superblock: bytenr=65536, device=./td3
fsid			c931379a-a119-4eda-a338-badb0a197512
metadata_uuid		f761b688-2642-4c94-be90-22f58e2a66d7


$ btrfstune -m --noscan ./td2 ./td1 ./td3
warning, device 1 is missing
ERROR: cannot read chunk root
ERROR: open ctree failed

$ btrfstune -m --noscan ./td1 ./td2 ./td3
$ echo $?
0

  If you are concerned about the lack of explicit device's fsid to work
  on. How about,

  (proposal only, these options does not exists yet)

  btrfstune -m --noscan --devices=./td2,./td3  ./td1


> - What's the proper error handling if operation on one of the parameter
>    failed if we choose to do the tune for all involved devices?
>    Should we revert the operation on the succeeded ones?
>    Should we continue on the remaining ones?

  Hm. That's a possible scenario even without this patch.!
  However, we use the CHANGING_FSID flag to handle split-brain scenarios
  with incomplete metadata_uuid changes. Currently, the kernel
  fixes this situation based on the flag and generation number.
  However, kernel should fail these split-brain scenarios and
  instead address them in the btrfs-progs, which is wip.

> I understand it's better to add the ability to do manual scan, but it
> looks like the multi-device arguments can be a little more complex than
> what we thought.

  Hmm How? The device list enumeration logic which handles the automatic
  scan also handle the command line provided device list. So both are
  same.

> At least I think we should add a dedicate --scan/--device option, and
> allow multiple --scan/--device to be provided for device list assembly,
> then still keep the single argument to avoid possible confusion.

  btrfs-progs scans all the block devices in the system, by default.
  so IMO,
  "--noscan" is reasonable, similar to 'btrfs in dump-tree --noscan'.

  I am ok with with --device/--devices option.
  So we could scan only commnd line provided devices
  with --noscan:

    btrfstune -m --noscan --devices=./td1,/dev/sda1 ./td3

  And to scan both command line and the block devices
  without --noscan:

    btrfstune -m --devices=./td1 ./td3


Thanks, Anand

> 
> This also solves the problem I mentioned above. If multiple filesystems
> are provided, they are just assembled into device list, won't have an
> impact on the tune target.
> 
> And since we still have a single device to tune, there is no extra error
> handling, nor confusion.
> 
> Thanks,
> Qu
> 
>>
>> Patches 1 to 5 primarily consist of cleanups. Patches 6 and 8 serve as
>> preparatory changes. Patch 7 enables btrfstune to accept multiple
>> devices. Patch 9 ensures that btrfstune no longer automatically uses the
>> system block devices when --noscan option is specified.
>> Patches 10 and 11 are help and documentation part.
>>
>> Anand Jain (11):
>>    btrfs-progs: check_mounted_where declare is_btrfs as bool
>>    btrfs-progs: check_mounted_where pack varibles type by size
>>    btrfs-progs: rename struct open_ctree_flags to open_ctree_args
>>    btrfs-progs: optimize device_list_add
>>    btrfs-progs: simplify btrfs_scan_one_device()
>>    btrfs-progs: factor out btrfs_scan_stdin_devices
>>    btrfs-progs: tune: add stdin device list
>>    btrfs-progs: refactor check_where_mounted with noscan option
>>    btrfs-progs: tune: add noscan option
>>    btrfs-progs: tune: add help for multiple devices and noscan option
>>    btrfs-progs: Documentation: update btrfstune --noscan option
>>
>>   Documentation/btrfstune.rst |  4 ++++
>>   btrfs-find-root.c           |  2 +-
>>   check/main.c                |  2 +-
>>   cmds/filesystem.c           |  2 +-
>>   cmds/inspect-dump-tree.c    | 39 ++++---------------------------------
>>   cmds/rescue.c               |  4 ++--
>>   cmds/restore.c              |  2 +-
>>   common/device-scan.c        | 39 +++++++++++++++++++++++++++++++++++++
>>   common/device-scan.h        |  1 +
>>   common/open-utils.c         | 21 +++++++++++---------
>>   common/open-utils.h         |  3 ++-
>>   common/utils.c              |  3 ++-
>>   image/main.c                |  4 ++--
>>   kernel-shared/disk-io.c     |  8 ++++----
>>   kernel-shared/disk-io.h     |  4 ++--
>>   kernel-shared/volumes.c     | 14 +++++--------
>>   mkfs/main.c                 |  2 +-
>>   tune/main.c                 | 25 +++++++++++++++++++-----
>>   18 files changed, 104 insertions(+), 75 deletions(-)
>>

Re: [PATCH 0/9] btrfs-progs: btrfstune: accept multiple devices and cleanup

[Qu Wenruo]

On 2023/6/8 08:20, Anand Jain wrote:
> On 07/06/2023 19:06, Qu Wenruo wrote:
>>
>>
>> On 2023/6/7 17:59, Anand Jain wrote:
>>> In an attempt to enable btrfstune to accept multiple devices from the
>>> command line, this patch includes some cleanup around the related code
>>> and functions.
>>
>> Mind to share the use case of the new ability?
>>
>
>   As of now btrfstune works with only one regular file. Not possible
>   to use multiple regular files. Unless loop device is used. This
>   set fixes this limitation.

Here I want to make the point clear, is the patchset intended to handle
ONE multi-device btrfs?

If that's the case, then my initial concerns on the multiple different
fses case is still a concern.

>
>
>> My concern related to multi-device parameters are:
>>
>> - What if the provided devices are belonging to different filesystems?
>>    Should we still do the tune operation on all of them or just the
>>    first/last device?
>>
>
>   Hmm, the scan part remains same with/without this patchset.
>   The device_list_add() function organizes the devices based on the fsid.
>   Any tool within the btrfs-progs uses this list to obtain the partner5
>   device list. This patch set still relies the same thing.
>
>   btrfstune gets the fsid to work on from the first deivce in the list.
>
>   Here is an example:
>
> $ btrfs in dump-super ./td1 ./td2 ./td3 | egrep
> 'device=|^fsid|^metadata_uuid'
> superblock: bytenr=65536, device=./td1
> fsid            c931379a-a119-4eda-a338-badb0a197512
> metadata_uuid        f761b688-2642-4c94-be90-22f58e2a66d7
> superblock: bytenr=65536, device=./td2
> fsid            f9643d74-1d3d-4b0d-b56b-b05ada340f57
> metadata_uuid        f761b688-2642-4c94-be90-22f58e2a66d7
> superblock: bytenr=65536, device=./td3
> fsid            c931379a-a119-4eda-a338-badb0a197512
> metadata_uuid        f761b688-2642-4c94-be90-22f58e2a66d7
>
>
> $ btrfstune -m --noscan ./td2 ./td1 ./td3
> warning, device 1 is missing
> ERROR: cannot read chunk root
> ERROR: open ctree failed
>
> $ btrfstune -m --noscan ./td1 ./td2 ./td3
> $ echo $?
> 0

This is exactly my concern.

We're combining target fs and device assembly into the same argument list.

Thus changing the order of argument would lead to different results.

>
>   If you are concerned about the lack of explicit device's fsid to work
>   on. How about,
>
>   (proposal only, these options does not exists yet)
>
>   btrfstune -m --noscan --devices=./td2,./td3  ./td1

That much better, less confusion.

Furthermore, the --devices (Although my initial proposal looks more like
"--device td2 --device td3", which makes parsing a little simpler) can
be applied to all other btrfs-progs, allowing a global way to assemble
the device list.

>
>
>> - What's the proper error handling if operation on one of the parameter
>>    failed if we choose to do the tune for all involved devices?
>>    Should we revert the operation on the succeeded ones?
>>    Should we continue on the remaining ones?
>
>   Hm. That's a possible scenario even without this patch.!
>   However, we use the CHANGING_FSID flag to handle split-brain scenarios
>   with incomplete metadata_uuid changes. Currently, the kernel
>   fixes this situation based on the flag and generation number.
>   However, kernel should fail these split-brain scenarios and
>   instead address them in the btrfs-progs, which is wip.
>
>> I understand it's better to add the ability to do manual scan, but it
>> looks like the multi-device arguments can be a little more complex than
>> what we thought.
>
>   Hmm How? The device list enumeration logic which handles the automatic
>   scan also handle the command line provided device list. So both are
>   same.

The "--device=" option you proposed is exactly the way to handle it.

Thanks,
Qu
>
>> At least I think we should add a dedicate --scan/--device option, and
>> allow multiple --scan/--device to be provided for device list assembly,
>> then still keep the single argument to avoid possible confusion.
>
>   btrfs-progs scans all the block devices in the system, by default.
>   so IMO,
>   "--noscan" is reasonable, similar to 'btrfs in dump-tree --noscan'.
>
>   I am ok with with --device/--devices option.
>   So we could scan only commnd line provided devices
>   with --noscan:
>
>     btrfstune -m --noscan --devices=./td1,/dev/sda1 ./td3
>
>   And to scan both command line and the block devices
>   without --noscan:
>
>     btrfstune -m --devices=./td1 ./td3
>
>
> Thanks, Anand
>
>>
>> This also solves the problem I mentioned above. If multiple filesystems
>> are provided, they are just assembled into device list, won't have an
>> impact on the tune target.
>>
>> And since we still have a single device to tune, there is no extra error
>> handling, nor confusion.
>>
>> Thanks,
>> Qu
>>
>>>
>>> Patches 1 to 5 primarily consist of cleanups. Patches 6 and 8 serve as
>>> preparatory changes. Patch 7 enables btrfstune to accept multiple
>>> devices. Patch 9 ensures that btrfstune no longer automatically uses the
>>> system block devices when --noscan option is specified.
>>> Patches 10 and 11 are help and documentation part.
>>>
>>> Anand Jain (11):
>>>    btrfs-progs: check_mounted_where declare is_btrfs as bool
>>>    btrfs-progs: check_mounted_where pack varibles type by size
>>>    btrfs-progs: rename struct open_ctree_flags to open_ctree_args
>>>    btrfs-progs: optimize device_list_add
>>>    btrfs-progs: simplify btrfs_scan_one_device()
>>>    btrfs-progs: factor out btrfs_scan_stdin_devices
>>>    btrfs-progs: tune: add stdin device list
>>>    btrfs-progs: refactor check_where_mounted with noscan option
>>>    btrfs-progs: tune: add noscan option
>>>    btrfs-progs: tune: add help for multiple devices and noscan option
>>>    btrfs-progs: Documentation: update btrfstune --noscan option
>>>
>>>   Documentation/btrfstune.rst |  4 ++++
>>>   btrfs-find-root.c           |  2 +-
>>>   check/main.c                |  2 +-
>>>   cmds/filesystem.c           |  2 +-
>>>   cmds/inspect-dump-tree.c    | 39 ++++---------------------------------
>>>   cmds/rescue.c               |  4 ++--
>>>   cmds/restore.c              |  2 +-
>>>   common/device-scan.c        | 39 +++++++++++++++++++++++++++++++++++++
>>>   common/device-scan.h        |  1 +
>>>   common/open-utils.c         | 21 +++++++++++---------
>>>   common/open-utils.h         |  3 ++-
>>>   common/utils.c              |  3 ++-
>>>   image/main.c                |  4 ++--
>>>   kernel-shared/disk-io.c     |  8 ++++----
>>>   kernel-shared/disk-io.h     |  4 ++--
>>>   kernel-shared/volumes.c     | 14 +++++--------
>>>   mkfs/main.c                 |  2 +-
>>>   tune/main.c                 | 25 +++++++++++++++++++-----
>>>   18 files changed, 104 insertions(+), 75 deletions(-)
>>>
>

Re: [PATCH 0/9] btrfs-progs: btrfstune: accept multiple devices and cleanup

[Anand Jain]

>>   As of now btrfstune works with only one regular file. Not possible
>>   to use multiple regular files. Unless loop device is used. This
>>   set fixes this limitation.
> 
> Here I want to make the point clear, is the patchset intended to handle
> ONE multi-device btrfs?
> 
> If that's the case, then my initial concerns on the multiple different
> fses case is still a concern.


>> $ btrfstune -m --noscan ./td2 ./td1 ./td3
>> warning, device 1 is missing
>> ERROR: cannot read chunk root
>> ERROR: open ctree failed
>>
>> $ btrfstune -m --noscan ./td1 ./td2 ./td3
>> $ echo $?
>> 0
> 
> This is exactly my concern. > We're combining target fs and device assembly into the same argument 
list.

> Thus changing the order of argument would lead to different results.

  yeah. --device solves it.


>>   btrfstune -m --noscan --devices=./td2,./td3  ./td1
> 
> That much better, less confusion.
> 
> Furthermore, the --devices (Although my initial proposal looks more like
> "--device td2 --device td3", which makes parsing a little simpler) can
> be applied to all other btrfs-progs, allowing a global way to assemble
> the device list.
> 

  --device td2 --device td3.. that makes it same as mount.
  I am not yet sure if it can be a global option though.

>>
>>
>>> - What's the proper error handling if operation on one of the parameter
>>>    failed if we choose to do the tune for all involved devices?
>>>    Should we revert the operation on the succeeded ones?
>>>    Should we continue on the remaining ones?
>>
>>   Hm. That's a possible scenario even without this patch.!
>>   However, we use the CHANGING_FSID flag to handle split-brain scenarios
>>   with incomplete metadata_uuid changes. Currently, the kernel
>>   fixes this situation based on the flag and generation number.
>>   However, kernel should fail these split-brain scenarios and
>>   instead address them in the btrfs-progs, which is wip.
>>
>>> I understand it's better to add the ability to do manual scan, but it
>>> looks like the multi-device arguments can be a little more complex than
>>> what we thought.
>>
>>   Hmm How? The device list enumeration logic which handles the automatic
>>   scan also handle the command line provided device list. So both are
>>   same.
> 
> The "--device=" option you proposed is exactly the way to handle it.

   Ok.


> 
> Thanks,
> Qu
>>
>>> At least I think we should add a dedicate --scan/--device option, and
>>> allow multiple --scan/--device to be provided for device list assembly,
>>> then still keep the single argument to avoid possible confusion.
>>
>>   btrfs-progs scans all the block devices in the system, by default.
>>   so IMO,
>>   "--noscan" is reasonable, similar to 'btrfs in dump-tree --noscan'.
>>
>>   I am ok with with --device/--devices option.
>>   So we could scan only commnd line provided devices
>>   with --noscan:
>>
>>     btrfstune -m --noscan --devices=./td1,/dev/sda1 ./td3
>>
>>   And to scan both command line and the block devices
>>   without --noscan:
>>
>>     btrfstune -m --devices=./td1 ./td3
>>
>>
>> Thanks, Anand
>>
>>>
>>> This also solves the problem I mentioned above. If multiple filesystems
>>> are provided, they are just assembled into device list, won't have an
>>> impact on the tune target.
>>>
>>> And since we still have a single device to tune, there is no extra error
>>> handling, nor confusion.
>>>
>>> Thanks,
>>> Qu
>>>
>>>>
>>>> Patches 1 to 5 primarily consist of cleanups. Patches 6 and 8 serve as
>>>> preparatory changes. Patch 7 enables btrfstune to accept multiple
>>>> devices. Patch 9 ensures that btrfstune no longer automatically uses 
>>>> the
>>>> system block devices when --noscan option is specified.
>>>> Patches 10 and 11 are help and documentation part.
>>>>
>>>> Anand Jain (11):
>>>>    btrfs-progs: check_mounted_where declare is_btrfs as bool
>>>>    btrfs-progs: check_mounted_where pack varibles type by size
>>>>    btrfs-progs: rename struct open_ctree_flags to open_ctree_args
>>>>    btrfs-progs: optimize device_list_add
>>>>    btrfs-progs: simplify btrfs_scan_one_device()
>>>>    btrfs-progs: factor out btrfs_scan_stdin_devices
>>>>    btrfs-progs: tune: add stdin device list
>>>>    btrfs-progs: refactor check_where_mounted with noscan option
>>>>    btrfs-progs: tune: add noscan option
>>>>    btrfs-progs: tune: add help for multiple devices and noscan option
>>>>    btrfs-progs: Documentation: update btrfstune --noscan option
>>>>
>>>>   Documentation/btrfstune.rst |  4 ++++
>>>>   btrfs-find-root.c           |  2 +-
>>>>   check/main.c                |  2 +-
>>>>   cmds/filesystem.c           |  2 +-
>>>>   cmds/inspect-dump-tree.c    | 39 
>>>> ++++---------------------------------
>>>>   cmds/rescue.c               |  4 ++--
>>>>   cmds/restore.c              |  2 +-
>>>>   common/device-scan.c        | 39 
>>>> +++++++++++++++++++++++++++++++++++++
>>>>   common/device-scan.h        |  1 +
>>>>   common/open-utils.c         | 21 +++++++++++---------
>>>>   common/open-utils.h         |  3 ++-
>>>>   common/utils.c              |  3 ++-
>>>>   image/main.c                |  4 ++--
>>>>   kernel-shared/disk-io.c     |  8 ++++----
>>>>   kernel-shared/disk-io.h     |  4 ++--
>>>>   kernel-shared/volumes.c     | 14 +++++--------
>>>>   mkfs/main.c                 |  2 +-
>>>>   tune/main.c                 | 25 +++++++++++++++++++-----
>>>>   18 files changed, 104 insertions(+), 75 deletions(-)
>>>>
>>