Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\))
Subject: Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is
 missing in lower/upper fs
From:   =?utf-8?Q?Christian_Kohlsch=C3=BCtter?= 
        <christian@kohlschutter.com>
In-Reply-To: <CAJfpegvBSCQwkCv=5LJDx1LRCN_ztTh9VMvrTbCyt0zf7W2trw@mail.gmail.com>
Date:   Mon, 18 Jul 2022 16:25:26 +0200
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        overlayfs <linux-unionfs@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-fsdevel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <0B8DA307-7E1F-4534-B864-BC2632740C89@kohlschutter.com>
References: <4B9D76D5-C794-4A49-A76F-3D4C10385EE0@kohlschutter.com>
 <CAJfpegs1Kta-HcikDGFt4=fa_LDttCeRmffKhUjWLr=DxzXg-A@mail.gmail.com>
 <83A29F9C-1A91-4753-953A-0C98E8A9832C@kohlschutter.com>
 <CAJfpegv5W0CycWCc2-kcn4=UVqk1hP7KrvBpzXHwW-Nmkjx8zA@mail.gmail.com>
 <FFA26FD1-60EF-457E-B914-E1978CCC7B57@kohlschutter.com>
 <CAJfpeguDAJpLMABsomBFQ=w6Li0=sBW0bFyALv4EJrAmR2BkpQ@mail.gmail.com>
 <A31096BA-C128-4D0B-B27D-C34560844ED0@kohlschutter.com>
 <CAJfpegvBSCQwkCv=5LJDx1LRCN_ztTh9VMvrTbCyt0zf7W2trw@mail.gmail.com>
To:     Miklos Szeredi <miklos@szeredi.hu>
Precedence: bulk

> Am 18.07.2022 um 15:13 schrieb Miklos Szeredi <miklos@szeredi.hu>:
>=20
> On Mon, 18 Jul 2022 at 15:03, Christian Kohlsch=C3=BCtter
> <christian@kohlschutter.com> wrote:
>>=20
>> Am 18.07.2022 um 14:21 schrieb Miklos Szeredi <miklos@szeredi.hu>:
>>>=20
>>> On Mon, 18 Jul 2022 at 12:56, Christian Kohlsch=C3=BCtter
>>> <christian@kohlschutter.com> wrote:
>>>=20
>>>> However, users of fuse that have no business with overlayfs =
suddenly see their ioctl return ENOTTY instead of ENOSYS.
>>>=20
>>> And returning ENOTTY is the correct behavior.  See this comment in
>>> <asm-generic/errrno.h>:
>>>=20
>>> /*
>>> * 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 */
>>>=20
>>> Thanks,
>>> Miklos
>>=20
>> That ship is sailed since ENOSYS was returned to user-space for the =
first time.
>>=20
>> 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.
>=20
> Correct.  The question is whether any application would break in this
> case.  I think not, but you are free to prove otherwise.
>=20
> 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