* [PATCH resend v2] bcachefs: reject unknown mount options
@ 2024-09-17 1:39 David Disseldorp
2024-09-21 23:13 ` Kent Overstreet
2024-10-10 18:33 ` Janpieter Sollie
0 siblings, 2 replies; 6+ messages in thread
From: David Disseldorp @ 2024-09-17 1:39 UTC (permalink / raw)
To: linux-bcachefs; +Cc: David Disseldorp, Martin Doucha, Hongbo Li
This is effectively a revert of commit 03ef80b469d5 ("bcachefs: Ignore
unknown mount options"), rebased atop commit 929d954330142 ("bcachefs:
use new mount API").
Contrary to the 03ef80b469d5 commit message, unknown mount options do
result in failure on other filesystems such as xfs, btrfs, ext4, etc.
Ignoring unknown options may result in unexpected behaviour: e.g. if
an application requires future-workload-specific-mount-option to work
and is run on a system without future-workload-specific-mount-option
kernel support, then it may fail without indication of the missing
kernel functionality.
This reversion may cause failures for existing users reliant on the
ignore-unknown behaviour, but it should be bearable given bcachefs is
still flagged experimental.
Fixes: 03ef80b469d5 ("bcachefs: Ignore unknown mount options")
Reported-by: Martin Doucha <martin.doucha@suse.com>
Closes: https://bugzilla.opensuse.org/show_bug.cgi?id=1230065
Signed-off-by: David Disseldorp <ddiss@suse.de>
Reviewed-by: Hongbo Li <lihongbo22@huawei.com>
---
fs/bcachefs/opts.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c
index e10fc1da71b19..bc5c180878212 100644
--- a/fs/bcachefs/opts.c
+++ b/fs/bcachefs/opts.c
@@ -482,9 +482,8 @@ int bch2_parse_one_mount_opt(struct bch_fs *c, struct bch_opts *opts,
val = "0";
}
- /* Unknown options are ignored: */
if (id < 0)
- return 0;
+ goto bad_opt;
if (!(bch2_opt_table[id].flags & OPT_MOUNT))
goto bad_opt;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH resend v2] bcachefs: reject unknown mount options 2024-09-17 1:39 [PATCH resend v2] bcachefs: reject unknown mount options David Disseldorp @ 2024-09-21 23:13 ` Kent Overstreet 2024-09-23 3:59 ` David Disseldorp 2024-09-23 9:05 ` Hongbo Li 2024-10-10 18:33 ` Janpieter Sollie 1 sibling, 2 replies; 6+ messages in thread From: Kent Overstreet @ 2024-09-21 23:13 UTC (permalink / raw) To: David Disseldorp; +Cc: linux-bcachefs, Martin Doucha, Hongbo Li On Tue, Sep 17, 2024 at 01:39:57AM GMT, David Disseldorp wrote: > This is effectively a revert of commit 03ef80b469d5 ("bcachefs: Ignore > unknown mount options"), rebased atop commit 929d954330142 ("bcachefs: > use new mount API"). > > Contrary to the 03ef80b469d5 commit message, unknown mount options do > result in failure on other filesystems such as xfs, btrfs, ext4, etc. > > Ignoring unknown options may result in unexpected behaviour: e.g. if > an application requires future-workload-specific-mount-option to work > and is run on a system without future-workload-specific-mount-option > kernel support, then it may fail without indication of the missing > kernel functionality. > > This reversion may cause failures for existing users reliant on the > ignore-unknown behaviour, but it should be bearable given bcachefs is > still flagged experimental. > > Fixes: 03ef80b469d5 ("bcachefs: Ignore unknown mount options") > Reported-by: Martin Doucha <martin.doucha@suse.com> > Closes: https://bugzilla.opensuse.org/show_bug.cgi?id=1230065 > Signed-off-by: David Disseldorp <ddiss@suse.de> > Reviewed-by: Hongbo Li <lihongbo22@huawei.com> I applied this and got some new failures in the CI - it seems something is broken in remount. 00162 ========= TEST ec_small 00162 00162 WATCHDOG 60 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): starting version 1.12: rebalance_work_acct_fix opts=errors=panic,metadata_replicas=2,data_replicas=2,erasure_code 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): initializing new filesystem 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): going read-write 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): marking superblocks 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): initializing freespace 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): done initializing freespace 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): reading snapshots table 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): reading snapshots done 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): done starting filesystem 00163 1+0 records in 00163 1+0 records out 00163 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0115145 s, 91.1 MB/s 00163 bcachefs: bch2_parse_one_mount_opt() Bad mount option rwerrors 00163 Error: Invalid argument 00163 Error 1 at /host//home/testdashboard/ktest/tests/fs/bcachefs/ec.ktest 32 from: mount -o remount,ro /mnt, exiting 00163 Error 1 at /host/home/testdashboard/ktest/tests/prelude.sh 269 from: ( set -e; run_test $i ), exiting 00163 00163 ========= FAILED ec_small in 2s The test is just doing mount -o remount,ro /mnt Thomas, this is likely something you introduced, think you could take a look? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend v2] bcachefs: reject unknown mount options 2024-09-21 23:13 ` Kent Overstreet @ 2024-09-23 3:59 ` David Disseldorp 2024-09-23 6:07 ` Hongbo Li 2024-09-23 9:05 ` Hongbo Li 1 sibling, 1 reply; 6+ messages in thread From: David Disseldorp @ 2024-09-23 3:59 UTC (permalink / raw) To: Kent Overstreet; +Cc: linux-bcachefs, Martin Doucha, Hongbo Li On Sat, 21 Sep 2024 19:13:31 -0400, Kent Overstreet wrote: > I applied this and got some new failures in the CI - it seems something > is broken in remount. ... > 00163 bcachefs: bch2_parse_one_mount_opt() Bad mount option rwerrors > 00163 Error: Invalid argument > 00163 Error 1 at /host//home/testdashboard/ktest/tests/fs/bcachefs/ec.ktest 32 from: mount -o remount,ro /mnt, exiting > 00163 Error 1 at /host/home/testdashboard/ktest/tests/prelude.sh 269 from: ( set -e; run_test $i ), exiting > 00163 > 00163 ========= FAILED ec_small in 2s > > The test is just doing > mount -o remount,ro /mnt > > Thomas, this is likely something you introduced, think you could take a > look? Thanks for the details. "mount -o remount,ro <mnt>" works fine for me, and I'm not aware of anything that could be providing the "rwerrors" parameter. Which version of mount / util-linux is used here? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend v2] bcachefs: reject unknown mount options 2024-09-23 3:59 ` David Disseldorp @ 2024-09-23 6:07 ` Hongbo Li 0 siblings, 0 replies; 6+ messages in thread From: Hongbo Li @ 2024-09-23 6:07 UTC (permalink / raw) To: David Disseldorp, Kent Overstreet; +Cc: linux-bcachefs, Martin Doucha On 2024/9/23 11:59, David Disseldorp wrote: > On Sat, 21 Sep 2024 19:13:31 -0400, Kent Overstreet wrote: > >> I applied this and got some new failures in the CI - it seems something >> is broken in remount. > ... >> 00163 bcachefs: bch2_parse_one_mount_opt() Bad mount option rwerrors >> 00163 Error: Invalid argument >> 00163 Error 1 at /host//home/testdashboard/ktest/tests/fs/bcachefs/ec.ktest 32 from: mount -o remount,ro /mnt, exiting >> 00163 Error 1 at /host/home/testdashboard/ktest/tests/prelude.sh 269 from: ( set -e; run_test $i ), exiting >> 00163 >> 00163 ========= FAILED ec_small in 2s >> >> The test is just doing >> mount -o remount,ro /mnt >> >> Thomas, this is likely something you introduced, think you could take a >> look? > > Thanks for the details. "mount -o remount,ro <mnt>" works fine for me, > and I'm not aware of anything that could be providing the "rwerrors" It seems rw, errors=xxx which misses a comma. > parameter. Which version of mount / util-linux is used here? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend v2] bcachefs: reject unknown mount options 2024-09-21 23:13 ` Kent Overstreet 2024-09-23 3:59 ` David Disseldorp @ 2024-09-23 9:05 ` Hongbo Li 1 sibling, 0 replies; 6+ messages in thread From: Hongbo Li @ 2024-09-23 9:05 UTC (permalink / raw) To: Kent Overstreet, David Disseldorp; +Cc: linux-bcachefs, Martin Doucha On 2024/9/22 7:13, Kent Overstreet wrote: > On Tue, Sep 17, 2024 at 01:39:57AM GMT, David Disseldorp wrote: >> This is effectively a revert of commit 03ef80b469d5 ("bcachefs: Ignore >> unknown mount options"), rebased atop commit 929d954330142 ("bcachefs: >> use new mount API"). >> >> Contrary to the 03ef80b469d5 commit message, unknown mount options do >> result in failure on other filesystems such as xfs, btrfs, ext4, etc. >> >> Ignoring unknown options may result in unexpected behaviour: e.g. if >> an application requires future-workload-specific-mount-option to work >> and is run on a system without future-workload-specific-mount-option >> kernel support, then it may fail without indication of the missing >> kernel functionality. >> >> This reversion may cause failures for existing users reliant on the >> ignore-unknown behaviour, but it should be bearable given bcachefs is >> still flagged experimental. >> >> Fixes: 03ef80b469d5 ("bcachefs: Ignore unknown mount options") >> Reported-by: Martin Doucha <martin.doucha@suse.com> >> Closes: https://bugzilla.opensuse.org/show_bug.cgi?id=1230065 >> Signed-off-by: David Disseldorp <ddiss@suse.de> >> Reviewed-by: Hongbo Li <lihongbo22@huawei.com> > > I applied this and got some new failures in the CI - it seems something > is broken in remount. > > 00162 ========= TEST ec_small > 00162 > 00162 WATCHDOG 60 > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): starting version 1.12: rebalance_work_acct_fix opts=errors=panic,metadata_replicas=2,data_replicas=2,erasure_code > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): initializing new filesystem > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): going read-write > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): marking superblocks > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): initializing freespace > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): done initializing freespace > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): reading snapshots table > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): reading snapshots done > 00163 bcachefs (857683e6-ef4f-41f5-abdd-dca191c38fff): done starting filesystem > 00163 1+0 records in > 00163 1+0 records out > 00163 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0115145 s, 91.1 MB/s > 00163 bcachefs: bch2_parse_one_mount_opt() Bad mount option rwerrors > 00163 Error: Invalid argument > 00163 Error 1 at /host//home/testdashboard/ktest/tests/fs/bcachefs/ec.ktest 32 from: mount -o remount,ro /mnt, exiting > 00163 Error 1 at /host/home/testdashboard/ktest/tests/prelude.sh 269 from: ( set -e; run_test $i ), exiting > 00163 > 00163 ========= FAILED ec_small in 2s > > The test is just doing > mount -o remount,ro /mnt ah, this is indeed a bug in show_options. It misses a comma when fill options buffer to seq. The following code should be added. diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c index e774cb3e6b1a..79c3a650b714 100644 --- a/fs/bcachefs/fs.c +++ b/fs/bcachefs/fs.c @@ -1927,7 +1927,7 @@ static int bch2_show_options(struct seq_file *seq, struct dentry *root) bch2_opts_to_text(&buf, c->opts, c, c->disk_sb.sb, OPT_MOUNT, OPT_HIDDEN, OPT_SHOW_MOUNT_STYLE); printbuf_nul_terminate(&buf); - seq_puts(seq, buf.buf); + seq_printf(seq, ",%s", buf.buf); int ret = buf.allocation_failure ? -ENOMEM : 0; printbuf_exit(&buf); I will send this later. The test passed in my environment with this v2 patch. Thanks, Hongbo > > Thomas, this is likely something you introduced, think you could take a > look? ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH resend v2] bcachefs: reject unknown mount options 2024-09-17 1:39 [PATCH resend v2] bcachefs: reject unknown mount options David Disseldorp 2024-09-21 23:13 ` Kent Overstreet @ 2024-10-10 18:33 ` Janpieter Sollie 1 sibling, 0 replies; 6+ messages in thread From: Janpieter Sollie @ 2024-10-10 18:33 UTC (permalink / raw) To: David Disseldorp, linux-bcachefs Cc: Martin Doucha, Hongbo Li, Kent Overstreet Op 17/09/2024 om 03:39 schreef David Disseldorp: > This is effectively a revert of commit 03ef80b469d5 ("bcachefs: Ignore > unknown mount options"), rebased atop commit 929d954330142 ("bcachefs: > use new mount API"). > > Contrary to the 03ef80b469d5 commit message, unknown mount options do > result in failure on other filesystems such as xfs, btrfs, ext4, etc. > > Ignoring unknown options may result in unexpected behaviour: e.g. if > an application requires future-workload-specific-mount-option to work > and is run on a system without future-workload-specific-mount-option > kernel support, then it may fail without indication of the missing > kernel functionality. > > This reversion may cause failures for existing users reliant on the > ignore-unknown behaviour, but it should be bearable given bcachefs is > still flagged experimental. > > Fixes: 03ef80b469d5 ("bcachefs: Ignore unknown mount options") > Reported-by: Martin Doucha <martin.doucha@suse.com> > Closes: https://bugzilla.opensuse.org/show_bug.cgi?id=1230065 > Signed-off-by: David Disseldorp <ddiss@suse.de> > Reviewed-by: Hongbo Li <lihongbo22@huawei.com> > --- > fs/bcachefs/opts.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c > index e10fc1da71b19..bc5c180878212 100644 > --- a/fs/bcachefs/opts.c > +++ b/fs/bcachefs/opts.c > @@ -482,9 +482,8 @@ int bch2_parse_one_mount_opt(struct bch_fs *c, struct bch_opts *opts, > val = "0"; > } > > - /* Unknown options are ignored: */ > if (id < 0) > - return 0; > + goto bad_opt; > > if (!(bch2_opt_table[id].flags & OPT_MOUNT)) > goto bad_opt; Testing this patch caused my bcachefs mount command to fail: the "user" and "nofail" attributes in /etc/fstab are not recognised. I'd love the "nofail" attribute on a "experimental" filesystem. IMO it is even a soft requirement Janpieter Sollie ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-10 18:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-17 1:39 [PATCH resend v2] bcachefs: reject unknown mount options David Disseldorp 2024-09-21 23:13 ` Kent Overstreet 2024-09-23 3:59 ` David Disseldorp 2024-09-23 6:07 ` Hongbo Li 2024-09-23 9:05 ` Hongbo Li 2024-10-10 18:33 ` Janpieter Sollie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox