On Tue, Oct 10, 2023 at 9:33 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > On Tue, 10 Oct 2023 at 20:15, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > It may not be me, it may be someone else, so there is a limit to my > > commitment, but kernel developers usually abide by Linus' no regressions > > rules (which do allow some slack). > > Note: the no regressions rule is about actual "out in the field" > regressions, not about potential or theoretical regressions. > > My guess is that changing the escaping rules for workdir and upperdir > would not make any difference. Look: on my laptop 0.0032% of > filenames contain a backslash. How likely is such a filename to be > used as workdir or upperdir? So yes, I think getting rid of > unescaping for these parameters on the new API is safe and will not > invoke the no regressions rule. > > The same cannot be said of lowerdir, because the incidence of colons > in filenames is much higher. But the new API also introduced an > "append mode" for lowerdirs, where the colon doesn't have the same > separator role as with the "bulk mode". Unfortunately it's not > possible to clearly differentiate the two modes, which I think is a > downside of the current design, and it's why I suggested the _noesc > variants. > We could add new keys: lowerdir_first=,lowerdir_next=,lowerdatadir_next= as explicit variants for the "[^:]",":","::" prefix detection and those don't need to be unescaped. > > > > Anyway, let's focus on what you would like best. > > If you prefer to just fix the regression, it is doable. > > If you prefer the upperdirfd, workdirfd, lowerdirfd API, I think we can > > find a volunteer to write it up. > > It's not all good: when showing these options, the result is > completely meaningless. Or is there a plan to make that work nicely? > Currently, the paths to display in mount options are stored in ofs->config.lowerdirs array. Those paths could be invalid or point to different objects or not accessible from the mount namepsace by the time someone observes them in mount options, so there are not many guarantees about those displayed paths. We could use file_path() to resolve path strings of the moment in the mount namepsace of the mounter from the lowerdirfd files and store them in ofs->config.lowerdirs array. I don't think it matters that the displayed mount options are not the actual user passed mount options as long as the paths are fairly descriptive and as long as they can be use to mount the same overlay with the same mount options in the common case (no escaped chars). BTW, it looks like we also don't display the user passed lowerdir parameter as is in the case of escaped characters in lowerdirs. Admittedly, that is another change of behavior from the new mount api param parsers. Thanks, Amir.
