On Thu, Oct 26, 2017 at 4:15 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > On Thu, Oct 26, 2017 at 08:39:28AM +0300, Amir Goldstein wrote: >> On Wed, Oct 25, 2017 at 10:09 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: >> > By default metadata only copy up is disabled. Provide a mount option so >> > that users can choose one way or other. >> > >> > Also provide a kernel config and module option to enable/disable >> > metacopy feature. >> > >> > Like index feature, when overlay is mounted, on root upper directory we >> > set ORIGIN which points to lower. And at later mount time it is verified >> > again. This hopes to get the configuration right. But this does only so >> > much as we don't verify all the lowers. So it is possible that a lower is >> > missing and later data copy up fails. >> >> Since you bother to specify the motivation for verifying lower root dir, >> FYI, the main motivation was to detect the case of copied layers. > > What are copied layers? You mean I remove original layer and bring in > another one with same top dir name? > I mean admin is out of disk space so she cp -a some folders between volumes or maybe copying all layers a side to backup a machine. There is a section in Documentation/filesystems/overlayfs.txt "Sharing and copying layers" where I wrote: "It is quite a common practice to copy overlay layers to a different directory tree on the same or different underlying filesystem, and even to a different machine" This was my impression. Please fix me if you think this statement is not accurate. > But this tests only one layer, right. So if there are multiple lower > layers, I can still copy/remove or do something else with other layers > and it will not detect. > > Also I should be able to retain top dir name same and replace anything > underneath and that will not be detected either. > > So it provides only so much of protection, IIUC. It provides merely a big red sign if user used to practice "copying layers", user cannot do that anymore with index=on and she cannot do that with metacopy=on either. the test is there as a heuristic to detect that use case and alert the user. I would rephrase commit message to something like: "Like index feature, we verify on mount that upper root is not being reused with a different lower root. This hopes to get the configuration right and detect the copied layers use case. But this does only..." Less 'what' more 'why'. > >> I still hold the opinion that new incompat features should enforce >> exclusive dir lock, so withholding Reviewed-by on this one for now. > > I am really not happy about enforcing exlusive dir lock because > this breaks userspace. > > I like the idea of making it hard to get configuration wrong, but overlay > was like this since the beginning and how many issues have we faced > that users ended up sharing upper. Atleast I have not come across > even a single one. > > If that's the case, I would rahter wait that a proper fix comes along > (finding existing super block and returning that) and then harden it. > > If I enforce it now, I most likely can't even use it with current > container run time because there is a possibility they can leak > mount points. So what's the point of introducing this feature. > > Miklos, what do you think? > > Vivek -- 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
