Re: [PATCH 3/5] libceph: crush_location infrastructure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 29, 2020 at 9:10 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Fri, 2020-05-29 at 20:38 +0200, Ilya Dryomov wrote:
> > On Fri, May 29, 2020 at 7:27 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > > On Fri, 2020-05-29 at 17:19 +0200, Ilya Dryomov wrote:
> > > > Allow expressing client's location in terms of CRUSH hierarchy as
> > > > a set of (bucket type name, bucket name) pairs.  The userspace syntax
> > > > "crush_location = key1=value1 key2=value2" is incompatible with mount
> > > > options and needed adaptation:
> > > >
> > > > - ':' separator
> > > > - one key:value pair per crush_location option
> > > > - crush_location options are combined together
> > > >
> > > > So for:
> > > >
> > > >   crush_location = host=foo rack=bar
> > > >
> > > > one would write:
> > > >
> > > >   crush_location=host:foo,crush_location=rack:bar
> > > >
> > > > As in userspace, "multipath" locations are supported, so indicating
> > > > locality for parallel hierarchies is possible:
> > > >
> > > >   crush_location=rack:foo1,crush_location=rack:foo2,crush_location=datacenter:bar
> > > >
> > >
> > > Blech, that syntax is hideous. It's also problematic in that the options
> > > are additive -- you can't override an option that was given earlier
> > > (e.g. in fstab), or in a shell script.
> > >
> > > Is it not possible to do something with a single crush_location= option?
> > > Maybe:
> > >
> > >     crush_location=rack:foo1/rack:foo2/datacenter:bar
> > >
> > > It's still ugly with the embedded '=' signs, but it would at least make
> > > it so that the options aren't additive.
> >
> > I suppose we could do something like that at the cost of more
> > parsing boilerplate, but I'm not sure additive options are that
> > hideous.  I don't think additive options are unprecedented and
> > more importantly I think many simple boolean and integer options
> > are not properly overridable even in major filesystems.
> >
>
> That is the long-standing convention though. There are reasons to
> deviate from it, but I don't see it here. Plus, I think the syntax I
> proposed above is more readable (and compact) as well.
>
> It would mean a bit more parsing code though, granted.
>
> > What embedded '=' signs are you referring to?  I see ':' and '/'
> > in your suggested syntax.
> >
>
> Sorry, yeah... I had originally done one that had '=' chars in it, but
> converted it to the above. Please disregard that paragraph.

One of the reasons I did it this way is that crush_location is
inherently additive.  I don't have a strong opinion on this though
so let's adhere to the convention.

I'll implement the suggested syntax and repost.

Thanks,

                Ilya



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux