Re: [PATCH V3 1/2] btrfs-progs: Add the single-dev feature (to both mkfs/tune)

[Anand Jain <anand.jain@oracle.com>]

  We may need to fix the command 'btrfs filesystem show' aswell.
  Could you test having more than one single-devices with
  the same fsid and running 'btrfs filesystem show' to ensure
  it can still display all the devices?

Thx.
Anand


On 31/08/2023 08:12, Guilherme G. Piccoli wrote:
> The single-dev feature allows a device to be mounted regardless of
> its fsid already being present in another device - in other words,
> this feature disables RAID modes / metadata_uuid, allowing a single
> device per filesystem. Its goal is mainly to allow mounting the
> same fsid at the same time in the system.
> 
> Introduce hereby the feature to both mkfs (-O single-dev) and
> btrfstune (--convert-to-single-device), syncing the kernel-shared
> headers as well. The feature is a compat_ro, its kernel version was
> set to v6.6.
> 
> Suggested-by: Qu Wenruo <wqu@suse.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
> 
> V3:
> 
> - Changed the small '-s' option on btrfstune to the
> long version "--convert-to-single-device" (thanks Josef!).
> 
> - Moved the kernel version to v6.6.
> 
> 
>   common/fsfeatures.c        |  7 ++++
>   kernel-shared/ctree.h      |  3 +-
>   kernel-shared/uapi/btrfs.h |  7 ++++
>   mkfs/main.c                |  4 +-
>   tune/main.c                | 76 ++++++++++++++++++++++++--------------
>   5 files changed, 67 insertions(+), 30 deletions(-)
> 
> diff --git a/common/fsfeatures.c b/common/fsfeatures.c
> index 00658fa5159f..8813de01d618 100644
> --- a/common/fsfeatures.c
> +++ b/common/fsfeatures.c
> @@ -160,6 +160,13 @@ static const struct btrfs_feature mkfs_features[] = {
>   		VERSION_NULL(default),
>   		.desc		= "RAID1 with 3 or 4 copies"
>   	},
> +	{
> +		.name		= "single-dev",
> +		.compat_ro_flag	= BTRFS_FEATURE_COMPAT_RO_SINGLE_DEV,
> +		.sysfs_name	= "single_dev",
> +		VERSION_TO_STRING2(compat, 6,6),
> +		.desc		= "single device (allows same fsid mounting)"
> +	},
>   #ifdef BTRFS_ZONED
>   	{
>   		.name		= "zoned",
> diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
> index 59533879b939..e3fd834aa6dd 100644
> --- a/kernel-shared/ctree.h
> +++ b/kernel-shared/ctree.h
> @@ -86,7 +86,8 @@ static inline u32 __BTRFS_LEAF_DATA_SIZE(u32 nodesize)
>   	(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |	\
>   	 BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID | \
>   	 BTRFS_FEATURE_COMPAT_RO_VERITY |		\
> -	 BTRFS_FEATURE_COMPAT_RO_BLOCK_GROUP_TREE)
> +	 BTRFS_FEATURE_COMPAT_RO_BLOCK_GROUP_TREE |	\
> +	 BTRFS_FEATURE_COMPAT_RO_SINGLE_DEV)
>   
>   #if EXPERIMENTAL
>   #define BTRFS_FEATURE_INCOMPAT_SUPP			\
> diff --git a/kernel-shared/uapi/btrfs.h b/kernel-shared/uapi/btrfs.h
> index 85b04f89a2a9..2e0ee6ef6446 100644
> --- a/kernel-shared/uapi/btrfs.h
> +++ b/kernel-shared/uapi/btrfs.h
> @@ -336,6 +336,13 @@ _static_assert(sizeof(struct btrfs_ioctl_fs_info_args) == 1024);
>    */
>   #define BTRFS_FEATURE_COMPAT_RO_BLOCK_GROUP_TREE	(1ULL << 3)
>   
> +/*
> + * Single devices (as flagged by the corresponding compat_ro flag) only
> + * gets scanned during mount time; also, a random fsid is generated for
> + * them, in order to cope with same-fsid filesystem mounts.
> + */
> +#define BTRFS_FEATURE_COMPAT_RO_SINGLE_DEV		(1ULL << 4)
> +
>   #define BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF	(1ULL << 0)
>   #define BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL	(1ULL << 1)
>   #define BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS	(1ULL << 2)
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 972ed1112ea6..429799932224 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -1025,6 +1025,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>   	char *label = NULL;
>   	int nr_global_roots = sysconf(_SC_NPROCESSORS_ONLN);
>   	char *source_dir = NULL;
> +	bool single_dev;
>   
>   	cpu_detect_flags();
>   	hash_init_accel();
> @@ -1218,6 +1219,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>   		usage(&mkfs_cmd, 1);
>   
>   	opt_zoned = !!(features.incompat_flags & BTRFS_FEATURE_INCOMPAT_ZONED);
> +	single_dev = !!(features.compat_ro_flags & BTRFS_FEATURE_COMPAT_RO_SINGLE_DEV);
>   
>   	if (source_dir && device_count > 1) {
>   		error("the option -r is limited to a single device");
> @@ -1815,7 +1817,7 @@ out:
>   		device_count = argc - optind;
>   		while (device_count-- > 0) {
>   			file = argv[optind++];
> -			if (path_is_block_device(file) == 1)
> +			if (path_is_block_device(file) == 1 && !single_dev)
>   				btrfs_register_one_device(file);
>   		}
>   	}
> diff --git a/tune/main.c b/tune/main.c
> index 0ca1e01282c9..7b8706274fcc 100644
> --- a/tune/main.c
> +++ b/tune/main.c
> @@ -42,27 +42,31 @@
>   #include "tune/tune.h"
>   #include "check/clear-cache.h"
>   
> +#define SET_SUPER_FLAGS(type) \
> +static int set_super_##type##_flags(struct btrfs_root *root, u64 flags) \
> +{									\
> +	struct btrfs_trans_handle *trans;				\
> +	struct btrfs_super_block *disk_super;				\
> +	u64 super_flags;						\
> +	int ret;							\
> +									\
> +	disk_super = root->fs_info->super_copy;				\
> +	super_flags = btrfs_super_##type##_flags(disk_super);		\
> +	super_flags |= flags;						\
> +	trans = btrfs_start_transaction(root, 1);			\
> +	BUG_ON(IS_ERR(trans));						\
> +	btrfs_set_super_##type##_flags(disk_super, super_flags);	\
> +	ret = btrfs_commit_transaction(trans, root);			\
> +									\
> +	return ret;							\
> +}
> +
> +SET_SUPER_FLAGS(incompat)
> +SET_SUPER_FLAGS(compat_ro)
> +
>   static char *device;
>   static int force = 0;
>   
> -static int set_super_incompat_flags(struct btrfs_root *root, u64 flags)
> -{
> -	struct btrfs_trans_handle *trans;
> -	struct btrfs_super_block *disk_super;
> -	u64 super_flags;
> -	int ret;
> -
> -	disk_super = root->fs_info->super_copy;
> -	super_flags = btrfs_super_incompat_flags(disk_super);
> -	super_flags |= flags;
> -	trans = btrfs_start_transaction(root, 1);
> -	BUG_ON(IS_ERR(trans));
> -	btrfs_set_super_incompat_flags(disk_super, super_flags);
> -	ret = btrfs_commit_transaction(trans, root);
> -
> -	return ret;
> -}
> -
>   static int convert_to_fst(struct btrfs_fs_info *fs_info)
>   {
>   	int ret;
> @@ -108,6 +112,8 @@ static const char * const tune_usage[] = {
>   	OPTLINE("--convert-from-block-group-tree",
>   			"convert the block group tree back to extent tree (remove the incompat bit)"),
>   	OPTLINE("--convert-to-free-space-tree", "convert filesystem to use free space tree (v2 cache)"),
> +	OPTLINE("--convert-to-single-device", "enable the single device feature "
> +			"(mkfs: single-dev, allows same fsid mounting)"),
>   	"",
>   	"UUID changes:",
>   	OPTLINE("-u", "rewrite fsid, use a random one"),
> @@ -146,7 +152,8 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
>   	int csum_type = -1;
>   	char *new_fsid_str = NULL;
>   	int ret;
> -	u64 super_flags = 0;
> +	u64 compat_ro_flags = 0;
> +	u64 incompat_flags = 0;
>   	int fd = -1;
>   
>   	btrfs_config_init();
> @@ -155,7 +162,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_SINGLE_DEV };
>   		static const struct option long_options[] = {
>   			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
>   			{ "convert-to-block-group-tree", no_argument, NULL,
> @@ -164,6 +172,8 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
>   				GETOPT_VAL_DISABLE_BLOCK_GROUP_TREE},
>   			{ "convert-to-free-space-tree", no_argument, NULL,
>   				GETOPT_VAL_ENABLE_FREE_SPACE_TREE},
> +			{ "convert-to-single-device", no_argument, NULL,
> +				GETOPT_VAL_SINGLE_DEV},
>   #if EXPERIMENTAL
>   			{ "csum", required_argument, NULL, GETOPT_VAL_CSUM },
>   #endif
> @@ -179,13 +189,13 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
>   			seeding_value = arg_strtou64(optarg);
>   			break;
>   		case 'r':
> -			super_flags |= BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF;
> +			incompat_flags |= BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF;
>   			break;
>   		case 'x':
> -			super_flags |= BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA;
> +			incompat_flags |= BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA;
>   			break;
>   		case 'n':
> -			super_flags |= BTRFS_FEATURE_INCOMPAT_NO_HOLES;
> +			incompat_flags |= BTRFS_FEATURE_INCOMPAT_NO_HOLES;
>   			break;
>   		case 'f':
>   			force = 1;
> @@ -216,6 +226,9 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
>   		case GETOPT_VAL_ENABLE_FREE_SPACE_TREE:
>   			to_fst = true;
>   			break;
> +		case GETOPT_VAL_SINGLE_DEV:
> +			compat_ro_flags |= BTRFS_FEATURE_COMPAT_RO_SINGLE_DEV;
> +			break;
>   #if EXPERIMENTAL
>   		case GETOPT_VAL_CSUM:
>   			btrfs_warn_experimental(
> @@ -239,9 +252,9 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
>   		error("random fsid can't be used with specified fsid");
>   		return 1;
>   	}
> -	if (!super_flags && !seeding_flag && !(random_fsid || new_fsid_str) &&
> -	    !change_metadata_uuid && csum_type == -1 && !to_bg_tree &&
> -	    !to_extent_tree && !to_fst) {
> +	if (!compat_ro_flags && !incompat_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;
> @@ -363,8 +376,15 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[])
>   		total++;
>   	}
>   
> -	if (super_flags) {
> -		ret = set_super_incompat_flags(root, super_flags);
> +	if (incompat_flags) {
> +		ret = set_super_incompat_flags(root, incompat_flags);
> +		if (!ret)
> +			success++;
> +		total++;
> +	}
> +
> +	if (compat_ro_flags) {
> +		ret = set_super_compat_ro_flags(root, compat_ro_flags);
>   		if (!ret)
>   			success++;
>   		total++;