Re: [PATCH v3 0/2] ceph: add debugfs entries signifying new mount syntax support

From: Venky Shankar <vshankar@redhat.com>
To: Jeff Layton <jlayton@redhat.com>
Cc: Patrick Donnelly <pdonnell@redhat.com>,
	ceph-devel <ceph-devel@vger.kernel.org>
Subject: Re: [PATCH v3 0/2] ceph: add debugfs entries signifying new mount syntax support
Date: Thu, 30 Sep 2021 18:52:06 +0530	[thread overview]
Message-ID: <CACPzV1=4fPeJhWkazv+_=0FyKY4Ou=QJ19kRvymkGp5a1taeHQ@mail.gmail.com> (raw)
In-Reply-To: <54238e8d192057c5ea9dc393d9974d7bdf09bf40.camel@redhat.com>

On Thu, Sep 30, 2021 at 4:58 PM Jeff Layton <jlayton@redhat.com> wrote:
>
> On Tue, 2021-09-28 at 11:36 +0530, Venky Shankar wrote:
> > v3:
> >  - create mount syntax debugfs entries under /<>/ceph/meta/client_features directory
> >  - mount syntax debugfs file names are v1, v2,... (were v1_mount_sytnax,... earlier)
> >
> > [This is based on top of new mount syntax series]
> >
> > Patrick proposed the idea of having debugfs entries to signify if
> > kernel supports the new (v2) mount syntax. The primary use of this
> > information is to catch any bugs in the new syntax implementation.
> >
> > This would be done as follows::
> >
> > The userspace mount helper tries to mount using the new mount syntax
> > and fallsback to using old syntax if the mount using new syntax fails.
> > However, a bug in the new mount syntax implementation can silently
> > result in the mount helper switching to old syntax.
> >
> > So, the debugfs entries can be relied upon by the mount helper to
> > check if the kernel supports the new mount syntax. Cases when the
> > mount using the new syntax fails, but the kernel does support the
> > new mount syntax, the mount helper could probably log before switching
> > to the old syntax (or fail the mount altogether when run in test mode).
> >
> > Debugfs entries are as follows::
> >
> >     /sys/kernel/debug/ceph/
> >     ....
> >     ....
> >     /sys/kernel/debug/ceph/meta
> >     /sys/kernel/debug/ceph/meta/client_features
> >     /sys/kernel/debug/ceph/meta/client_features/v2
> >     /sys/kernel/debug/ceph/meta/client_features/v1
> >     ....
> >     ....
> >
>
> The patches look fine, technically, so I think we're down to the
> bikeshedding here.
>
> My minor gripe is that "v1" and "v2" are not really client features.
> Perhaps we should call these "mount_format_v1" or maybe
> "mount_syntax_v1" ? I could forsee is advertising other features in this
> dir in the future, and at that point "v1" and "v2" are somewhat
> ambiguous for names.
>
> Make sense?

Doh! I changed this from "v1_mount_syntax" to just "v1" ;P

Let's call it mount_syntax_v1,..

>
>
> > Venky Shankar (2):
> >   libceph: export ceph_debugfs_dir for use in ceph.ko
> >   ceph: add debugfs entries for mount syntax support
> >
> >  fs/ceph/debugfs.c            | 41 ++++++++++++++++++++++++++++++++++++
> >  fs/ceph/super.c              |  3 +++
> >  fs/ceph/super.h              |  2 ++
> >  include/linux/ceph/debugfs.h |  2 ++
> >  net/ceph/debugfs.c           |  3 ++-
> >  5 files changed, 50 insertions(+), 1 deletion(-)
> >
>
> --
> Jeff Layton <jlayton@redhat.com>
>

-- 
Cheers,
Venky