On 3/29/25 12:18, Eric Sandeen wrote:
> I was working on next steps for this, and I have a followup question.
>
> Today, several mount options are simply ignored if the on-disk format
> does not support them. For example:
>
> case Opt_compress_mode:
> if (!f2fs_sb_has_compression(sbi)) {
> f2fs_info(sbi, "Image doesn't support
> compression");
> break;
> }
> name = match_strdup(&args[0]);
> if (!name)
> return -ENOMEM;
> if (!strcmp(name, "fs")) {
> F2FS_OPTION(sbi).compress_mode =
> COMPR_MODE_FS;
> } else if (!strcmp(name, "user")) {
> F2FS_OPTION(sbi).compress_mode =
> COMPR_MODE_USER;
> } else {
> kfree(name);
> return -EINVAL;
> }
> kfree(name);
> break;
>
> so if f2fs_sb_has_compression() is not true, then the option is ignored
> without
> any validation.
>
> in other words, "mount -o compress_mode=nope ..." will succeed if the feature
> is disabled on the filesystem.
>
> If I move the f2fs_sb_has_compression() check to later for the new mount API,
> then "mount -o compress_mode=nope ..." will start failing for all images. Is
> this acceptable? It seems wise to reject invalid options rather than ignore
> them,
> even if they are incompatible with the format, but this would be a behavior
> change.
I'm fine w/ this change. IIRC, I haven't saw above use case, otherwise user
should stop passing invalid mount option to f2fs.
Thanks,
>
> The above would be simple enough to defer (maybe set to COMPR_MODE_INVAL and
> reject it later) but I think other options such as compress/nocompress
> extensions
> would be very messy to approach as "accept all options given during parsing,
> and validate them later only if the corresponding feature is present."
>
> So I wonder if a behavior change (stricter option validation) would be
> acceptable here?
>
> Thanks,
> -Eric
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
