Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191212145042.12694-1-labbott@redhat.com> <CAOi1vP9E2yLeFptg7o99usEi=x3kf=NnHYdURXPhX4vTXKCTCQ@mail.gmail.com>
 <fbe90a0b-cf24-8c0c-48eb-6183852dfbf1@redhat.com> <CAHk-=wh7Wuk9QCP6oH5Qc1a89_X6H1CHRK_OyB4NLmX7nRYJeA@mail.gmail.com>
 <cf4c9634-1503-d182-cb12-810fb969bc96@redhat.com> <20191212213609.GK4203@ZenIV.linux.org.uk>
In-Reply-To: <20191212213609.GK4203@ZenIV.linux.org.uk>
From:   Miklos Szeredi <miklos@szeredi.hu>
Date:   Fri, 13 Dec 2019 10:15:03 +0100
Message-ID: <CAJfpegv_zY6w6=pOL0x=sjuQmGae0ymOafZXjyAdNEHj+EKyNA@mail.gmail.com>
Subject: Re: [PATCH] vfs: Don't reject unknown parameters
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     Laura Abbott <labbott@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Ilya Dryomov <idryomov@gmail.com>,
        David Howells <dhowells@redhat.com>,
        Jeremi Piotrowski <jeremi.piotrowski@gmail.com>,
        Linux FS Devel <linux-fsdevel@vger.kernel.org>,
        Phillip Lougher <phillip@squashfs.org.uk>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Dec 12, 2019 at 10:36 PM Al Viro <viro@zeniv.linux.org.uk> wrote:

> So you could bloody well just leave recognition (and handling) of "source"
> to the caller, leaving you with just this:
>
>         if (strcmp(param->key, "source") == 0)
>                 return -ENOPARAM;
>         /* Just log an error for backwards compatibility */
>         errorf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name, param->key);
>         return 0;

Which is fine for the old mount(2) interface.

But we have a brand new API as well; do we really need to carry these
backward compatibility issues forward?  I mean checking if a
param/flag is supported or not *is* useful and lacking that check is
the source of numerous headaches in legacy interfaces (just take the
open(2) example and the introduction of O_TMPFILE).

Just need a flag in fc indicating if this option comes from the old interface:

         if (strcmp(param->key, "source") == 0)
                 return -ENOPARAM;
         /* Just log an error for backwards compatibility */
         errorf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name,
param->key);
         return fc->legacy ? 0 : -ENOPARAM;

And TBH, I think that logic applies to the common flags as well.  Some
of these simply make no sense on the new interface ("silent",
"posixacl") and some are ignored for lots of filesystems ("sync",
"dirsync", "mand", "lazytime").  It would also be nice to reject "rw"
for read-only filesystems.

I have sent patches for the above numerous times, all been ignored by
DavidH and Al.  While this seems minor now, I think getting this
interface into a better shape as early as possible may save lots more
headaches later...

Thanks,
Miklos