Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[Eric Sandeen via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 4/1/25 3:33 PM, Eric Sandeen wrote:
> On 3/31/25 3:31 AM, Chao Yu wrote:
>> 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.
> 
> Great, I will proceed with this. It will make the conversion simpler (but may
> make testing/validation more difficult, as behavior will change with invalid input).

FYI - I don't think I will be able to complete this conversion task myself - f2fs is
by far the most difficult conversion I've encountered, and my time for these sorts of
projects is sadly limited. I do have one more patch series that moves a lot of the
on-disk feature checking out of option parsing, and perhaps I will send it as an example
at least.

But I think it may be time to ask the f2fs experts to take over this effort, because
I'm just not getting through it on my own.

(We are down to only a small handful of filesystems left - in fact, I think only
bfs, 9p, and f2fs, that don't have patches anywhere. So it would be really great to
get some help on this.)

Thanks,
-Eric
 
> -Eric
> 
>> Thanks,



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel