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