Re: [PATCH] bcachefs: fix incorrect show_options results

[David Disseldorp <ddiss@suse.de>]

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 <lihongbo22@huawei.com>
> ---
>  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.