On Wed, Oct 11, 2023 at 1:18 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > On Wed, 11 Oct 2023 at 10:45, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > 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. > > Good idea. I'd merge "lowerdir_first" and "lowerdir_next" into > "lowerdir_one" or whatever for simplicity. I'd also consider dropping > the prefix detection, since it has only been in mainline for one > cycle. > OK. Christian, Do you know any userspace that already uses your new append prefixes? Do we have any good reason to support "lowerdir_first" so a lower dir stack could be reset before creating the sb? > > > > > > > > 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. > > Right, so the configuration would use fd's while the display would > fall back to string paths. That makes sense. > Assuming that using fds is a desired feature regardless, then possibly the _noesc options are unneeded. Not sure. > > 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. > > And it's a bug (regardless of being a regression or not) since commas > and whitespace must be escaped on this interface, and colon too for > being a separator of lower layers. OK. I think it should be easy to fix this bug. I can look into it. > > More fun: upperdir and workdir use seq_show_option() which escapes > commas and whitespace, so any escaped characters during mount will end > up being double escaped. > > Obviously this domain is severely undertested. This is all very complicated because actual users always go through escaping rules of bash and libmount. For example, the output of 'mount' command unescapes the escaping done by seq_show_option() for /proc/mounts. That's why it is scary to change the legacy behavior and better to provide the new unescaped options as you suggested and leave all the escaping in the future to userspace. Thanks, Amir.
