On Thu, Sep 24, 2020 at 4:18 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > On Thu, Sep 24, 2020 at 05:44:22AM +0300, Amir Goldstein wrote: > > On Wed, Sep 23, 2020 at 10:47 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > > > > On Wed, Sep 23, 2020 at 06:23:08PM +0300, Pavel Tikhomirov wrote: > > > > This relaxes uuid checks for overlay index feature. It is only possible > > > > in case there is only one filesystem for all the work/upper/lower > > > > directories and bare file handles from this backing filesystem are uniq. > > > > > > Hi Pavel, > > > > > > Wondering why upper/work has to be on same filesystem as lower for this to > > > work? > > > > > > > I reckon that's because I asked for this constraint, so I will answer. > > > > You are right that the important thing is that all lower layers are > > on the same fs, but because of > > a888db310195 ovl: fix regression with re-formatted lower squashfs > > Hi Amir, > > So with "upper on same as lower fs" contstraint we are just making it > little harder so that people don't use recreated lower with existing > upper? Is that the intention behind this constraint. > > On a side note, I have a question about above commit. > > So this is basically the issue of upper stored file handle resolving to > a different file (in recreated lower). And we are considering this to > be a corner case. But the very fact a user was running into it, it > probably is not that hard to reproduce. So with the fix a888db310195, > we avoided the problem for simple configurations (no-index, no-metacopy, > and no xino). But if same user runs with index=on, with recreatd lower, > they can still run into similar issues? > > > > > I preferred to keep the rules simpler. > > > > Pavel's use case is clone of disk and change of its UUID. > > This is a real use case and I don't think it is unique to Virtuozzo, > > so I wanted index=nouuid to address that use case only and > > I prefer that it is documented that way too. > > Sure. I understand that. I am only harping on this to make sure > we tell people to not use this "recreated lower with existing upper". > In Pavel's use case, it is more of a cloned use case and not > re-created use case. > > Otherwise people will re-create lower layers with regular filesystems and > use index=nouuid and then run into squashfs like issue one day. > > Or we could document what Miklos had said. Using existing upper > with recreated lower will likely run into issues with advanced > overlay features like (index, metacopy, xino etc). > I am perfectly fine with saying that and with allowing the special case of cloning disk with index=nouuid. There was a "patch" floating around for improving the doc, I was assuming you will pick it up add your own proposed changes and make it into a proper patch. > > > > Ironically, one of the justifications for index=nouuid is virtiofs - > > because fuse is now allowed as upper (or as same fs), > > one can already use fuse passthough (or one could use fuse > > passthrough when nfs export works correctly) as a "uuid anonymizer" > > for any fs, so in practice, index=nouuid cannot do any more harm > > then one can already do when enabling index over virtiofs. > > Interesing. Using virtiofs or a fuse passthrough filesystem on top > just to avoid uuid check will be lot of work. > > But keeping upper/ on same fs as lower fs constraint does not help with this. > No, it does not. I am only saying index=nouuid is "just" as bad as what people can already do with virtiofs. Not much worse. > > > > That is why I prefer the interpretation that index=nouuid means > > "use null uuid instead of s_uuid for ovl_fh" over the interpretation > > "relax comparison of uuid in ovl_fh". > > So bottom line is that there are many ways where users can recreate > lower layers and run into issues. > > - squashfs with index > - use a fuse passthrough filesystem > - use index=nouuid > > So to me documenting that don't use existig upper with recreated lower > should help with all. > > And putting a constraint of "lower and upper being on same fs" seems fine > for now but I am not sure it helps a lot. Anyway, I am fine with this > constratint. Just wanted to understand the rationale behind it. > Only rational is - it is intended for cloned disk - don't make it easy to use this for anything else. Thanks, Amir.
