Message-Id: <0B8DA307-7E1F-4534-B864-BC2632740C89@kohlschutter.com>
Date:   Mon, 18 Jul 2022 16:25:26 +0200
From:   Christian Kohlschütter 
        <christian@...lschutter.com>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        overlayfs <linux-unionfs@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is
 missing in lower/upper fs

> Am 18.07.2022 um 15:13 schrieb Miklos Szeredi <miklos@...redi.hu>:
> 
> On Mon, 18 Jul 2022 at 15:03, Christian Kohlschütter
> <christian@...lschutter.com> wrote:
>> 
>> Am 18.07.2022 um 14:21 schrieb Miklos Szeredi <miklos@...redi.hu>:
>>> 
>>> On Mon, 18 Jul 2022 at 12:56, Christian Kohlschütter
>>> <christian@...lschutter.com> wrote:
>>> 
>>>> However, users of fuse that have no business with overlayfs suddenly see their ioctl return ENOTTY instead of ENOSYS.
>>> 
>>> And returning ENOTTY is the correct behavior.  See this comment in
>>> <asm-generic/errrno.h>:
>>> 
>>> /*
>>> * This error code is special: arch syscall entry code will return
>>> * -ENOSYS if users try to call a syscall that doesn't exist.  To keep
>>> * failures of syscalls that really do exist distinguishable from
>>> * failures due to attempts to use a nonexistent syscall, syscall
>>> * implementations should refrain from returning -ENOSYS.
>>> */
>>> #define ENOSYS 38 /* Invalid system call number */
>>> 
>>> Thanks,
>>> Miklos
>> 
>> That ship is sailed since ENOSYS was returned to user-space for the first time.
>> 
>> It reminds me a bit of Linus' "we do not break userspace" email from 2012 [1, 2], where Linus wrote:
>>> Applications *do* care about error return values. There's no way in
>>> hell you can willy-nilly just change them. And if you do change them,
>>> and applications break, there is no way in hell you can then blame the
>>> application.
> 
> Correct.  The question is whether any application would break in this
> case.  I think not, but you are free to prove otherwise.
> 
> Thanks,
> Miklos

I'm not going to do that since I expect any answer I give would not change your position here. All I know is there is a non-zero chance such programs exist.

If you're willing to go ahead with the fuse change you proposed, I see no purpose in debating with you further since you're the kernel maintainer of both file systems.
That change "fixes" the problem that I had seen in my setup; I do not know the extent of side effects, but I expect some could surface eventually.

Once you're done fixing fuse, please also talk to the folks over at https://github.com/trapexit/mergerfs who explicitly return ENOSYS upon request. Who knows, maybe someone is audacious enough to try mergerfs as a lower filesystem for overlay?

Alas, I think this a clash between the philosophies of writing robust code versus writing against a personal interpretation of some specification.
You refer to "asm-generic/errno.h" as the specification and rationale for treating ENOSYS as sacrosanct. Note that the comment says "should refrain from", it doesn't say "must not", and that's why we're in this mess.

It therefore wouldn't hurt to be lenient when a lower filesystem returns an error code known to refer to "unsupported operation", and that's what my original patch to ovl does.

I thought this approach would resonate with you, since you must have been following the same logic when you added the special-case check for "EINVAL" as an exception for ntfs-3g in the commit that most likely triggered the regression ("ovl: fix filattr copy-up failure") 9 months ago.

I honestly wonder why you're risking further breakage, having introduced that regression only recently.

So long,
Christian

Powered by blists - more mailing lists