On Tue, Jun 29, 2021 at 11:10:02AM -0400, Jeff Layton wrote:
> On Tue, 2021-06-29 at 19:24 +0530, Venky Shankar wrote:
> > On Tue, Jun 29, 2021 at 5:02 PM Luis Henriques <lhenriques@xxxxxxx> wrote:
> > >
> > > [ As I said, I didn't fully reviewed this patch. Just sending out a few
> > > comments. ]
> > >
> > > On Mon, Jun 28, 2021 at 01:25:42PM +0530, Venky Shankar wrote:
> > > > Old mount device syntax (source) has the following problems:
> > > >
> > > > - mounts to the same cluster but with different fsnames
> > > > and/or creds have identical device string which can
> > > > confuse xfstests.
> > > >
> > > > - Userspace mount helper tool resolves monitor addresses
> > > > and fill in mon addrs automatically, but that means the
> > > > device shown in /proc/mounts is different than what was
> > > > used for mounting.
> > > >
> > > > New device syntax is as follows:
> > > >
> > > > cephuser@fsid.mycephfs2=/path
> > > >
> > > > Note, there is no "monitor address" in the device string.
> > > > That gets passed in as mount option. This keeps the device
> > > > string same when monitor addresses change (on remounts).
> > > >
> > > > Also note that the userspace mount helper tool is backward
> > > > compatible. I.e., the mount helper will fallback to using
> > > > old syntax after trying to mount with the new syntax.
> > > >
> > > > Signed-off-by: Venky Shankar <vshankar@xxxxxxxxxx>
> > > > ---
> > > > fs/ceph/super.c | 117 +++++++++++++++++++++++++++++++++++++++++++-----
> > > > fs/ceph/super.h | 3 ++
> > > > 2 files changed, 110 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> > > > index 9b1b7f4cfdd4..950a28ad9c59 100644
> > > > --- a/fs/ceph/super.c
> > > > +++ b/fs/ceph/super.c
> > > > @@ -145,6 +145,7 @@ enum {
> > > > Opt_mds_namespace,
> > > > Opt_recover_session,
> > > > Opt_source,
> > > > + Opt_mon_addr,
> > > > /* string args above */
> > > > Opt_dirstat,
> > > > Opt_rbytes,
> > > > @@ -196,6 +197,7 @@ static const struct fs_parameter_spec ceph_mount_parameters[] = {
> > > > fsparam_u32 ("rsize", Opt_rsize),
> > > > fsparam_string ("snapdirname", Opt_snapdirname),
> > > > fsparam_string ("source", Opt_source),
> > > > + fsparam_string ("mon_addr", Opt_mon_addr),
> > > > fsparam_u32 ("wsize", Opt_wsize),
> > > > fsparam_flag_no ("wsync", Opt_wsync),
> > > > {}
> > > > @@ -226,10 +228,68 @@ static void canonicalize_path(char *path)
> > > > path[j] = '\0';
> > > > }
> > > >
> > > > +static int ceph_parse_old_source(const char *dev_name, const char *dev_name_end,
> > > > + struct fs_context *fc)
> > > > +{
> > > > + int r;
> > > > + struct ceph_parse_opts_ctx *pctx = fc->fs_private;
> > > > + struct ceph_mount_options *fsopt = pctx->opts;
> > > > +
> > > > + if (*dev_name_end != ':')
> > > > + return invalfc(fc, "separator ':' missing in source");
> > > > +
> > > > + r = ceph_parse_mon_ips(dev_name, dev_name_end - dev_name,
> > > > + pctx->copts, fc->log.log);
> > > > + if (r)
> > > > + return r;
> > > > +
> > > > + fsopt->new_dev_syntax = false;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int ceph_parse_new_source(const char *dev_name, const char *dev_name_end,
> > > > + struct fs_context *fc)
> > > > +{
> > > > + struct ceph_parse_opts_ctx *pctx = fc->fs_private;
> > > > + struct ceph_mount_options *fsopt = pctx->opts;
> > > > + char *fsid_start, *fs_name_start;
> > > > +
> > > > + if (*dev_name_end != '=')
> > > > + return invalfc(fc, "separator '=' missing in source");
> > >
> > > An annoying thing is that we'll always see this error message when falling
> > > back to the old_source method.
> > >
>
> True. I'd rather this fallback be silent.
>
> > > (Also, is there a good reason for using '=' instead of ':'? I probably
> > > missed this discussion somewhere else already...)
> > >
>
> Yes, we needed a way for the kernel to distinguish between the old and
> new syntax. Old kernels already reject any mount string without ":/" in
> it, so we needed the new format to _not_ have that to ensure a clean
> fallback procedure.
>
> It's not as pretty as I would have liked, but it's starting to grow on
> me. :)
Heh. And gets even worst with using '/' for separating the mons IPs. (I
understand why it can't be ',' and there's probably a reason for not using
';' either.) But yeah, that ship has sailed, I'm sure you guys discussed
all this already ;-)
Cheers,
--
Luís
