On Mon, Sep 23, 2024 at 02:39:31PM GMT, David Disseldorp wrote:
> On Mon, 23 Sep 2024 17:50:03 +0800, Hongbo Li wrote:
>
> > When call show_options in bcachefs, the options buffer is appeneded
> > to the seq variable. In fact, it requires an additional comma to be
> > appended first. This will affect the remount process when reading
> > existing mount options.
> >
> > Fixes: 9305cf91d05e ("bcachefs: bch2_opts_to_text()")
>
> Ah, that'd explain why I wasn't seeing this alongside the
> fail-on-unknown-opt fix. FWIW, this first appears in
> bcachefs-2024-09-21 as 3621ecc10f831f4fd27784083dfaf5b8481098b5 .
>
> > Signed-off-by: Hongbo Li <[email protected]>
> > ---
> > fs/bcachefs/fs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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);
>
> Looks okay, although it might be simpler to just drop the !first
> conditionals from bch2_opts_to_text() as things were prior to the
> regression.
>
> > int ret = buf.allocation_failure ? -ENOMEM : 0;
>
> Should the copy from buf into seq occur if allocation_failure
> is already flagged? I'd expect seq_puts() to have oopsed.
That's a good point; it might be the first allocation that fails, not a
realloc. It won't be in practice, buuuut....
