[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dddab8dd-6d36-4899-ae8c-d284c4c0306c@igalia.com>
Date: Wed, 9 Apr 2025 14:02:32 -0300
From: André Almeida <andrealmeid@...lia.com>
To: Gabriel Krisman Bertazi <gabriel@...sman.be>
Cc: Miklos Szeredi <miklos@...redi.hu>, Amir Goldstein <amir73il@...il.com>,
Theodore Tso <tytso@....edu>, linux-unionfs@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
kernel-dev@...lia.com
Subject: Re: [PATCH 0/3] ovl: Enable support for casefold filesystems
Hi Gabriel!
Em 09/04/2025 13:52, Gabriel Krisman Bertazi escreveu:
> André Almeida <andrealmeid@...lia.com> writes:
>
>> Hi all,
>>
>> We would like to support the usage of casefold filesystems with
>> overlayfs. This patchset do some of the work needed for that, but I'm
>> sure there are more places that need to be tweaked so please share your
>> feedback for this work.
>
> I didn't look the patches yet, but this is going to be quite tricky.
> For a start, consider the semantics when mixing volumes with different
> case settings for lower/upper/work directories. And that could be any
> setting, such as whether the directory has +F, the encoding version and
> the encoding flags (strict mode). Any mismatch will look bonkers and
> you want to forbid the mount.
>
> Consider upperdir is case-sensitive but lowerdir is not. In this case,
> I suspect the case-exact name would be hidden by the upper, but the
> inexact-case would still resolve from the lower when it shouldn't, and
> can't be raised again. If we have the other way around, the upper
> will hide more than one file from the lower and it is a matter of luck
> which file we are getting.
>
> In addition, if we have a case-insensitive on top of a case-sensitive,
> there is no way we can do the case-insensitive lookup on the lower
> without doing a sequential search across the entire directory. Then it
> is again a matter of luck which file we are getting.
>
> The issue can appear even on the same volume, since case-insensitiveness
> is actually per-directory and can be flipped when a directory is empty.
> If something like the below is possible, we are in the same situation
> again:
>
> mkdir lower/ci
> chattr +F lower/ci
> touch lower/ci/BLA
> mount -o overlay none upperdir=upper,lowerdir=lower,workdir=work merged
> rm -r merged/ci/BLA // creates a whiteout in upper
> // merged looks empty and should be allowed to drop +F
> chattr -F merged/ci
>
> So we'd also need to always forbid clearing the +F attribute and perhaps
> forbid it from ever being set on the merged directory. We also want to
> require the encoding version and flags to match.
>
Thank you for the prompt response. I agree with you, for the next
version I will implement such restrictions. I think it will be better to
start with very restrict possibilities and then slowly dropping then, if
they prove to be not problematic.
>> * Implementation
>>
>> The most obvious place that required change was the strncmp() inside of
>> ovl_cache_entry_find(), that I managed to convert to use d_same_name(),
>> that will then call the generic_ci_d_compare function if it's set for
>> the dentry. There are more strncmp() around ovl, but I would rather hear
>> feedback about this approach first than already implementing this around
>> the code.
>
> I'd suggest marking it as an RFC since it is not a functional
> implementation yet, IIUC.
>
Ops, you are right, I forgot to tag it as such.
>>> * Testing
>> sudo mount -t tmpfs -o casefold tmpfs mnt/
>> cd mnt/
>> mkdir dir
>> chattr +F dir
>> cd dir/
>> mkdir upper lower
>> mkdir lower/A lower/b lower/c
>> mkdir upper/a upper/b upper/d
>> mkdir merged work
>> sudo mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work, merged
>> ls /tmp/mnt/dir/merged/
>> a b c d
>>
>> And ovl is respecting the equivalent names. `a` points to a merged dir
>> between `A` and `a`, but giving that upperdir has a lowercase `a`, this
>> is the name displayed here.
>
> Did you try fstests generic/556? It might require some work to make it
> run over ovl, but it will exercise several cases that are quite
> hard to spot.
>
>
I haven't tried, I'm not sure which directory should I point to fstests
to use for the test, the merged one?
Powered by blists - more mailing lists