[PATCH 0/1] btrfs-progs: btrfstune: print seeding status

[PATCH 0/1] btrfs-progs: btrfstune: print seeding status

[David Disseldorp]

AFAICT there's currently no way to easily determine whether or not
seeding is enabled for a given device. The following patch adds new
"btrfstune -s" functionality to perform this.

An xfstests test case will follow if/when the interface is deemed
acceptable.

Cheers, David

[PATCH 1/1] btrfs-progs: btrfstune: print seeding status

[David Disseldorp]

btrfstune -s <dev> prints "Seeding flag is currently [un]set", based on
whether or not BTRFS_SUPER_FLAG_SEEDING is enabled for the given device.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 Documentation/btrfstune.asciidoc |  3 +++
 btrfstune.c                      | 29 +++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/btrfstune.asciidoc b/Documentation/btrfstune.asciidoc
index 27100964..992da95d 100644
--- a/Documentation/btrfstune.asciidoc
+++ b/Documentation/btrfstune.asciidoc
@@ -30,6 +30,9 @@ Enable seeding on a given device. Value 1 will enable seeding, 0 will disable it
 A seeding filesystem is forced to be mounted read-only. A new device can be added
 to the filesystem and will capture all writes keeping the seeding device intact.
 
+-s::
+Print whether or not seeding is enabled for a given device.
+
 -r::
 (since kernel: 3.7)
 +
diff --git a/btrfstune.c b/btrfstune.c
index 1e378ba1..603242b7 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -73,6 +73,15 @@ static int update_seeding_flag(struct btrfs_root *root, int set_flag)
 	return ret;
 }
 
+static void check_seeding_flag(struct btrfs_root *root)
+{
+	struct btrfs_super_block *disk_super = root->fs_info->super_copy;
+	u64 super_flags = btrfs_super_flags(disk_super);
+
+	printf("Seeding flag is currently %sset\n",
+	       (super_flags & BTRFS_SUPER_FLAG_SEEDING ? "" : "un"));
+}
+
 static int set_super_incompat_flags(struct btrfs_root *root, u64 flags)
 {
 	struct btrfs_trans_handle *trans;
@@ -374,6 +383,7 @@ static void print_usage(void)
 {
 	printf("usage: btrfstune [options] device\n");
 	printf("\t-S value\tpositive value will enable seeding, zero to disable, negative is not allowed\n");
+	printf("\t-s \t\tcheck whether seeding is enabled\n");
 	printf("\t-r \t\tenable extended inode refs\n");
 	printf("\t-x \t\tenable skinny metadata extent refs\n");
 	printf("\t-n \t\tenable no-holes feature (more efficient sparse file representation)\n");
@@ -390,6 +400,7 @@ int main(int argc, char *argv[])
 	int total = 0;
 	int seeding_flag = 0;
 	u64 seeding_value = 0;
+	int check_seeding = 0;
 	int random_fsid = 0;
 	char *new_fsid_str = NULL;
 	int ret;
@@ -401,7 +412,7 @@ int main(int argc, char *argv[])
 			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
 			{ NULL, 0, NULL, 0 }
 		};
-		int c = getopt_long(argc, argv, "S:rxfuU:n", long_options, NULL);
+		int c = getopt_long(argc, argv, "S:srxfuU:n", long_options, NULL);
 
 		if (c < 0)
 			break;
@@ -410,6 +421,9 @@ int main(int argc, char *argv[])
 			seeding_flag = 1;
 			seeding_value = arg_strtou64(optarg);
 			break;
+		case 's':
+			check_seeding = 1;
+			break;
 		case 'r':
 			super_flags |= BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF;
 			break;
@@ -448,7 +462,12 @@ int main(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)) {
+	if (seeding_flag && check_seeding) {
+		error("check seeding can't be used alongside set seeding");
+		return 1;
+	}
+	if (!super_flags && !seeding_flag && !check_seeding &&
+	    !(random_fsid || new_fsid_str)) {
 		error("at least one option should be specified");
 		print_usage();
 		return 1;
@@ -512,6 +531,12 @@ int main(int argc, char *argv[])
 		total++;
 	}
 
+	if (check_seeding) {
+		check_seeding_flag(root);
+		success++;
+		total++;
+	}
+
 	if (super_flags) {
 		ret = set_super_incompat_flags(root, super_flags);
 		if (!ret)
-- 
2.13.7

Re: [PATCH 1/1] btrfs-progs: btrfstune: print seeding status

[Nikolay Borisov]

On 4.01.19 г. 1:22 ч., David Disseldorp wrote:
> btrfstune -s <dev> prints "Seeding flag is currently [un]set", based on
> whether or not BTRFS_SUPER_FLAG_SEEDING is enabled for the given device.
> 
> Signed-off-by: David Disseldorp <ddiss@suse.de>

btrfs inspect-internal dump-super already supports showing seeding, why
do we need btrfstune support for that as well? Also the way you've
phrased is a bit misleading, since checking the superflag doesn't mean
seeding for a particular _device_ is enabled but that the filesystem
itself is set as seeding.
> ---
>  Documentation/btrfstune.asciidoc |  3 +++
>  btrfstune.c                      | 29 +++++++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/btrfstune.asciidoc b/Documentation/btrfstune.asciidoc
> index 27100964..992da95d 100644
> --- a/Documentation/btrfstune.asciidoc
> +++ b/Documentation/btrfstune.asciidoc
> @@ -30,6 +30,9 @@ Enable seeding on a given device. Value 1 will enable seeding, 0 will disable it
>  A seeding filesystem is forced to be mounted read-only. A new device can be added
>  to the filesystem and will capture all writes keeping the seeding device intact.
>  
> +-s::
> +Print whether or not seeding is enabled for a given device.
> +
>  -r::
>  (since kernel: 3.7)
>  +
> diff --git a/btrfstune.c b/btrfstune.c
> index 1e378ba1..603242b7 100644
> --- a/btrfstune.c
> +++ b/btrfstune.c
> @@ -73,6 +73,15 @@ static int update_seeding_flag(struct btrfs_root *root, int set_flag)
>  	return ret;
>  }
>  
> +static void check_seeding_flag(struct btrfs_root *root)
> +{
> +	struct btrfs_super_block *disk_super = root->fs_info->super_copy;
> +	u64 super_flags = btrfs_super_flags(disk_super);
> +
> +	printf("Seeding flag is currently %sset\n",
> +	       (super_flags & BTRFS_SUPER_FLAG_SEEDING ? "" : "un"));
> +}
> +
>  static int set_super_incompat_flags(struct btrfs_root *root, u64 flags)
>  {
>  	struct btrfs_trans_handle *trans;
> @@ -374,6 +383,7 @@ static void print_usage(void)
>  {
>  	printf("usage: btrfstune [options] device\n");
>  	printf("\t-S value\tpositive value will enable seeding, zero to disable, negative is not allowed\n");
> +	printf("\t-s \t\tcheck whether seeding is enabled\n");
>  	printf("\t-r \t\tenable extended inode refs\n");
>  	printf("\t-x \t\tenable skinny metadata extent refs\n");
>  	printf("\t-n \t\tenable no-holes feature (more efficient sparse file representation)\n");
> @@ -390,6 +400,7 @@ int main(int argc, char *argv[])
>  	int total = 0;
>  	int seeding_flag = 0;
>  	u64 seeding_value = 0;
> +	int check_seeding = 0;
>  	int random_fsid = 0;
>  	char *new_fsid_str = NULL;
>  	int ret;
> @@ -401,7 +412,7 @@ int main(int argc, char *argv[])
>  			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
>  			{ NULL, 0, NULL, 0 }
>  		};
> -		int c = getopt_long(argc, argv, "S:rxfuU:n", long_options, NULL);
> +		int c = getopt_long(argc, argv, "S:srxfuU:n", long_options, NULL);
>  
>  		if (c < 0)
>  			break;
> @@ -410,6 +421,9 @@ int main(int argc, char *argv[])
>  			seeding_flag = 1;
>  			seeding_value = arg_strtou64(optarg);
>  			break;
> +		case 's':
> +			check_seeding = 1;
> +			break;
>  		case 'r':
>  			super_flags |= BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF;
>  			break;
> @@ -448,7 +462,12 @@ int main(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)) {
> +	if (seeding_flag && check_seeding) {
> +		error("check seeding can't be used alongside set seeding");
> +		return 1;
> +	}
> +	if (!super_flags && !seeding_flag && !check_seeding &&
> +	    !(random_fsid || new_fsid_str)) {
>  		error("at least one option should be specified");
>  		print_usage();
>  		return 1;
> @@ -512,6 +531,12 @@ int main(int argc, char *argv[])
>  		total++;
>  	}
>  
> +	if (check_seeding) {
> +		check_seeding_flag(root);
> +		success++;
> +		total++;
> +	}
> +
>  	if (super_flags) {
>  		ret = set_super_incompat_flags(root, super_flags);
>  		if (!ret)
> 

Re: [PATCH 1/1] btrfs-progs: btrfstune: print seeding status

[David Disseldorp]

On Fri, 4 Jan 2019 09:05:00 +0200, Nikolay Borisov wrote:

> btrfs inspect-internal dump-super already supports showing seeding, why
> do we need btrfstune support for that as well?

Ah, I wasn't aware of that. IMO the logical place to look is next to
where the user enables seeding (btrfstune -S1).

> Also the way you've
> phrased is a bit misleading, since checking the superflag doesn't mean
> seeding for a particular _device_ is enabled but that the filesystem
> itself is set as seeding.

Not quite sure I follow. For a multi-dev FS (one seeding), I see:

rapido1:/# btrfstune -s /dev/zram0
Seeding flag is currently unset
rapido1:/# btrfstune -S 1 /dev/zram0
rapido1:/# btrfstune -s /dev/zram0
Seeding flag is currently set
rapido1:/# mount /dev/zram0 /mnt/test/
mount: /dev/zram0 is write-protected, mounting read-only
...
rapido1:/# btrfs device add -f /dev/zram1 /mnt/test/
[   77.805374] BTRFS info (device zram0): disk added /dev/zram1
rapido1:/# umount /mnt/test/
rapido1:/# btrfstune -s /dev/zram1
Seeding flag is currently unset
rapido1:/# btrfstune -s /dev/zram0
Seeding flag is currently set

Cheers, David

Re: [PATCH 1/1] btrfs-progs: btrfstune: print seeding status

[Anand Jain]

I agree btrfstune should have ability to read the parameter. But none of 
the btrsftune parameter [1] which it sets, aren't readable using 
btrfstune. So can we first design how are going to do it for rest of 
the parameters in btrfstune then later this patch can just fix for seed.

[1]
usage: btrfstune [options] device
	-S value	positive value will enable seeding, zero to disable, negative 
is not allowed
	-r 		enable extended inode refs
	-x 		enable skinny metadata extent refs
	-n 		enable no-holes feature (more efficient sparse file representation)
	-f 		force to do dangerous operation, make sure that you are aware of 
the dangers
	-u 		change fsid, use a random one
	-U UUID		change fsid to UUID


One suggestion
    btrfstune read -<S|r|x|n|u> <dev>




On 01/04/2019 06:26 PM, David Disseldorp wrote:
> On Fri, 4 Jan 2019 09:05:00 +0200, Nikolay Borisov wrote:
> 
>> btrfs inspect-internal dump-super already supports showing seeding, why
>> do we need btrfstune support for that as well?
> 
> Ah, I wasn't aware of that. IMO the logical place to look is next to
> where the user enables seeding (btrfstune -S1).
> 
>> Also the way you've
>> phrased is a bit misleading, since checking the superflag doesn't mean
>> seeding for a particular _device_ is enabled but that the filesystem
>> itself is set as seeding.
> 
> Not quite sure I follow. For a multi-dev FS (one seeding), I see:
> 
> rapido1:/# btrfstune -s /dev/zram0
> Seeding flag is currently unset
> rapido1:/# btrfstune -S 1 /dev/zram0
> rapido1:/# btrfstune -s /dev/zram0
> Seeding flag is currently set
> rapido1:/# mount /dev/zram0 /mnt/test/
> mount: /dev/zram0 is write-protected, mounting read-only
> ....
> rapido1:/# btrfs device add -f /dev/zram1 /mnt/test/
> [   77.805374] BTRFS info (device zram0): disk added /dev/zram1
> rapido1:/# umount /mnt/test/
> rapido1:/# btrfstune -s /dev/zram1
> Seeding flag is currently unset
> rapido1:/# btrfstune -s /dev/zram0
> Seeding flag is currently set
> 
> Cheers, David
> 

Re: [PATCH 1/1] btrfs-progs: btrfstune: print seeding status

[Nikolay Borisov]

On 4.01.19 г. 12:26 ч., David Disseldorp wrote:
> On Fri, 4 Jan 2019 09:05:00 +0200, Nikolay Borisov wrote:
> 
>> btrfs inspect-internal dump-super already supports showing seeding, why
>> do we need btrfstune support for that as well?
> 
> Ah, I wasn't aware of that. IMO the logical place to look is next to
> where the user enables seeding (btrfstune -S1).
> 
>> Also the way you've
>> phrased is a bit misleading, since checking the superflag doesn't mean
>> seeding for a particular _device_ is enabled but that the filesystem
>> itself is set as seeding.
> 
> Not quite sure I follow. For a multi-dev FS (one seeding), I see:

the seeding flag is a feature of the filesystem not any particular
device in it. It's set per-superblock, the superblock on every device is
(or should be) identical to the superblock on any other device.

> 
> rapido1:/# btrfstune -s /dev/zram0
> Seeding flag is currently unset
> rapido1:/# btrfstune -S 1 /dev/zram0
> rapido1:/# btrfstune -s /dev/zram0
> Seeding flag is currently set
> rapido1:/# mount /dev/zram0 /mnt/test/
> mount: /dev/zram0 is write-protected, mounting read-only
> ...
> rapido1:/# btrfs device add -f /dev/zram1 /mnt/test/
> [   77.805374] BTRFS info (device zram0): disk added /dev/zram1
> rapido1:/# umount /mnt/test/
> rapido1:/# btrfstune -s /dev/zram1
> Seeding flag is currently unset
> rapido1:/# btrfstune -s /dev/zram0
> Seeding flag is currently set

Right, i think this is a side effect rather than a deliberate behavior.
What I mean by this is if you have a raid0 filesystem and you set it to
seed then the flag will be reflected on both devices, but not because
the code said "mark device 1 and device 2 as seed devices" but because
the logic goes "mark this superblock as being a seed and write it to all
devices".

So when you actually add the new device to an existing filesystem which
is seed what btrfs does is initialize a new filesystem on the newly
added device. For reference you can check the code around
btrfs_init_new_device and btrfs_prepare_sprout. IMO seed has really been
hacked on "just because" and is lacking coherent design, I'd prefer it
didn't proliferate and rely on what we have currently.

> 
> Cheers, David
> 

Re: [PATCH 1/1] btrfs-progs: btrfstune: print seeding status

[David Disseldorp]

On Fri, 4 Jan 2019 15:18:13 +0200, Nikolay Borisov wrote:

> IMO seed has really been
> hacked on "just because" and is lacking coherent design, I'd prefer it
> didn't proliferate and rely on what we have currently.

That's unfortunate to hear. Seed devices are IMO a very cool feature for
transactional OS updates and live installers.

Cheers, David

Re: [PATCH 1/1] btrfs-progs: btrfstune: print seeding status

[Anand Jain]

On 01/04/2019 09:34 PM, David Disseldorp wrote:
> On Fri, 4 Jan 2019 15:18:13 +0200, Nikolay Borisov wrote:
> 
>> IMO seed has really been
>> hacked on "just because" and is lacking coherent design,

Can you elaborate please?

It has great potential, especially in the golden image kind of 
environment, current missing feature is ability to propagate and apply 
the golden image diff. Also it can be a choice of feature for the OS 
installation. I see a lot of potential in it.


>> I'd prefer it
>> didn't proliferate and rely on what we have currently.

> That's unfortunate to hear. Seed devices are IMO a very cool feature for
> transactional OS updates and live installers.



> Cheers, David
>