| Message ID | 20190103232239.11931-2-ddiss@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: btrfstune: print seeding status | expand |
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) >
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
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
>
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 >
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
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 >
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)
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(-)