On Fri, Jan 5, 2018 at 10:20 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Fri, Jan 5, 2018 at 9:07 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: [...] >>> Those are good questions to address to Miklos. >>> In V1, the mount option was called 'index=all' for indexing every file/dir >>> on copy up and the verify origin functionality was implemented with >>> a different option (verify_dir). >>> Then I realized that full indexing could actually be a feature on its own, >>> so I named this feature and documented what it does in Kconfig. >> >> index=all was much more intuitive to me. I could understand that it will >> create index for all kind of objects (and not just hardlinks). >> > > Yes, it was straight forward to understand technically, but what does it > say to users? absolutely nothing. OTOH redirect_dir is not called > dir_rename, so yeh, index=all can work. > Whatever Miklos prefers is fine by me. > >>> >>> Another option name I was contemplating is 'redirect_dir=verify' >>> this name makes a lot more sense for the part of detecting multiple >>> redirects. >> >> Or verify_redirect_dir=on/off. Advantage of fine grained options is >> that user can opt-in. But at the same time too many options will be >> confusing. > > The reason I like 'redirect_dir=verify' is because redirect_dir already has > modes now and those modes are not going to be a complex matrix of > options, just a linear scale of options: off,follow,on,verify > which are relatively easy to document. > > So maybe the most logical thing to do is to add redirect_dir=verify to > control the new behavior of verifying merge dir and detecting multiple redirect > and add an option index=all to control full indexing and detecting multiple > origin of non-dir. And either let redirect_dir=verify imply and turn on > index=all or let redirect_dir=verify require index=all and fail mount. > > How do you feel about this option? > >> >>> The reason I went for the more generic name 'verify=on' is because the >>> feature also detects multiple files with same origin (which are not hardlinks). >> >> So one problem with verify=on is that say it checks for 3 inconsistencies >> today. What will happen when we check for 4th inconsistency tomorrow. Say >> we want to enforce ORIGIN validity on non-dir objects. If we combine it >> with verify=on, then it becomes a backward copatibility issue. If we >> come up with a separate option, then we kind of split it badly. That is >> non-dir origin verification has separate mount option while dir origin >> verification is part of verify=on. >> >> Atleast a normal user will find it very hard to figure it out. >> >>> I have no strong feelings about the verify=on feature name. >> >> I don't have strong feelings either. I am just trying to think from a >> user's point of view and also thinking what will happen when more >> inconsistency checks are added in future. Will these be part of verify=on >> or we will create new options due to backward compatibility concerns. >> > > I understand your concern. > I agree that verify=on sounds a bit too broad and inviting trouble ;-) > But I just remembered why I switched from 2 opt-in mount options in V1 to a single config/module option in V2. consistency verification as well NFS export don't play well with overlays that are mounted without full index, so I wanted to have a config option that would make NFS export "just work" with overlayfs if default mount options are used. I guess I could have OVERLAY_FS_INDEX_ALL as a config option that depends on OVERLAY_FS_INDEX and redirect_dir=verify as a mount option. Or, I could follow you advise and just rename OVERLAY_FS_VERIFY to OVERLAY_FS_VERIFY_ORIGIN, which implies both full index and verification of lower dir. That last option is my favorite at the moment. How about it? Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
