linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <mszeredi@redhat.com>
To: David Howells <dhowells@redhat.com>
Cc: viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-nfs@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/9] VFS: Introduce a mount context
Date: Tue, 9 May 2017 14:38:48 +0200	[thread overview]
Message-ID: <CAOssrKdJbobwX8Ns+2xq0OhdzZSOy5XmD-rKbJdWXGL6eWJkGQ@mail.gmail.com> (raw)
In-Reply-To: <15868.1494323804@warthog.procyon.org.uk>

On Tue, May 9, 2017 at 11:56 AM, David Howells <dhowells@redhat.com> wrote:
> Miklos Szeredi <mszeredi@redhat.com> wrote:
>
>> So say we have commands like
>>
>> "o+ foo"
>> "o- bar"
>
> The convention seems to be to prepend "no" to things you want to disable, so
> let's stick with that, e.g.:
>
>         "o foo"
>         "o nobar"
>
> otherwise we will have to have separate parsers for old mount() and the new sb
> config code - and not just for NFS, but at least for ext2/3/4 also.
>
> Further, we can only publish one format in /proc/mounts - and we cannot change
> that from the foo/nofoo standard we already use as it's part of the UAPI.

You're right, that this is a complicated issue and worth more
discussion.  And also you are right that we cannot change existing
UAPI, which is going to cause some headaches.

But that doesn't mean the new UAPI must follow the conventions of the
badly defined existing UAPI.

And the "no*" convention is anything but well defined, so we cannot
just stick it into generic code, because you'll find exceptions
everywhere.

And one more reason to have a new, unambiguous UAPI for retrieving
superblock options.

>
>> The generic option parser would just add or remove the option in the
>> current set of options,
>
> It sounds like you want to build up a string of "opt1,opt2,opt3" then have the
> VFS add and remove things from it and then parse it into the filesystem's
> internal structures on "commit".

That would be the default operation, if the filesystem doesn't define
its own parser.

>> and commit would just call ->remount_fs() with the new set of options.
>
> You're defining "commit" to do different things depending on the situation.
> You need a separation between "commit create" and "commit update".

It would be different, yes, at least until the superblock creation api
is completely transformed, at which point it may actually become the
same thing.  But lets not jump ahead.

>> It would probably not work for the NFS case, but that's okay, NFS can
>> implement its own option parsing.
>
> If NFS has to implement its own option parsing, we've done it wrong.

My above sentence was not clear.  What I meant to say that NFS needs
to implement the non-generic option parsing function in order to be
able to handle the case of "you can't change the server IP address".
Which it would want to do anyway, since it will result in cleaner
code.

Thanks,
Miklos

  reply	other threads:[~2017-05-09 12:38 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-03 16:04 [RFC][PATCH 0/9] VFS: Introduce mount context David Howells
2017-05-03 16:04 ` [PATCH 1/9] Provide a function to create a NUL-terminated string from unterminated data David Howells
2017-05-03 16:55   ` Jeff Layton
2017-05-03 19:26   ` Rasmus Villemoes
2017-05-03 20:13   ` David Howells
2017-05-03 16:04 ` [PATCH 2/9] Clean up whitespace in fs/namespace.c David Howells
2017-05-03 16:04 ` [PATCH 3/9] VFS: Introduce a mount context David Howells
2017-05-03 18:13   ` Jeff Layton
2017-05-03 18:26     ` Joe Perches
2017-05-03 20:38       ` Matthew Wilcox
2017-05-03 21:36         ` Joe Perches
2017-05-04  6:28           ` Julia Lawall
2017-05-03 21:17       ` David Howells
2017-05-03 18:37     ` David Howells
2017-05-03 18:43       ` Joe Perches
2017-05-03 20:11       ` David Howells
2017-05-04  9:27     ` David Howells
2017-05-04 14:34       ` Joe Perches
2017-05-03 21:43   ` Rasmus Villemoes
2017-05-04 10:22   ` David Howells
2017-05-08 15:05   ` Miklos Szeredi
2017-05-08 22:57   ` David Howells
2017-05-09  8:03     ` Miklos Szeredi
2017-05-10 12:41       ` Karel Zak
2017-05-09  9:32     ` David Howells
2017-05-09 11:04       ` Miklos Szeredi
2017-05-09  9:41     ` David Howells
2017-05-09 12:02       ` Miklos Szeredi
2017-05-09 18:51         ` Jeff Layton
2017-05-10  7:24           ` Miklos Szeredi
2017-05-10  8:05           ` David Howells
2017-05-10 13:20             ` Jeff Layton
2017-05-10 13:30               ` Miklos Szeredi
2017-05-10 13:33                 ` Miklos Szeredi
2017-05-10 13:48                 ` Jeff Layton
2017-05-12  8:15                   ` Miklos Szeredi
2017-05-10 13:31             ` David Howells
2017-05-10 13:37               ` Jeff Layton
2017-05-09  9:56     ` David Howells
2017-05-09 12:38       ` Miklos Szeredi [this message]
2017-05-03 16:05 ` [PATCH 4/9] Implement fsopen() to prepare for a mount David Howells
2017-05-03 18:37   ` Jeff Layton
2017-05-03 18:41   ` David Howells
2017-05-03 20:44   ` Rasmus Villemoes
2017-05-04 10:40   ` Karel Zak
2017-05-04 12:55   ` David Howells
2017-05-04 12:58   ` David Howells
2017-05-04 13:06   ` David Howells
2017-05-04 13:34     ` Karel Zak
2017-05-09 18:40       ` Jeff Layton
2017-05-08 15:10   ` Miklos Szeredi
2017-05-08 23:09   ` David Howells
2017-05-03 16:05 ` [PATCH 5/9] Implement fsmount() to effect a pre-configured mount David Howells
2017-05-03 16:05 ` [PATCH 6/9] Sample program for driving fsopen/fsmount David Howells
2017-05-03 16:05 ` [PATCH 7/9] procfs: Move proc_fill_super() to fs/proc/root.c David Howells
2017-05-03 16:05 ` [PATCH 8/9] proc: Support the mount context in procfs David Howells
2017-05-03 16:05 ` [PATCH 9/9] NFS: Support the mount context and fsopen() David Howells
2017-05-03 16:44 ` [RFC][PATCH 0/9] VFS: Introduce mount context Jeff Layton
2017-05-03 16:50 ` David Howells
2017-05-03 17:27   ` Jeff Layton
2017-05-05 14:35 ` Miklos Szeredi
2017-05-05 15:47 ` David Howells
2017-05-08  8:25   ` Miklos Szeredi
2017-05-08  8:35 ` David Howells
2017-05-08  8:43   ` Miklos Szeredi
2017-05-08 17:03 ` Djalal Harouni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOssrKdJbobwX8Ns+2xq0OhdzZSOy5XmD-rKbJdWXGL6eWJkGQ@mail.gmail.com \
    --to=mszeredi@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).