> > >> Basically, I need to be able to continue serving the same files and > > >> paths even while the copy-up metadata process for any part of the tree > > >> is in progress. And it sounds like your idea of considering a copy-up > > >> of a merged dir as "complete" (and essentially opaque) would be the > > >> way to do that without files or dirs ever moving or losing access even > > >> momentarily. > > > > > > > > > Yes, that's the idea. > > > > > > I'll see when I get around to that demo. > > > > I found some time to write the POC patch, but not enough time > > to make it work :) - it is failing some fstests. > > > > Since I don't know when I will have time to debug the issues, > > here is the WIP if you want to debug it and point out the bugs: > > > > https://github.com/amir73il/linux/commits/ovl-finalize-dir/ > > This is very cool - many thanks! > Pushed two fixes to this branch - now it passes fstest - please re-test. > Unfortunately, I'm probably not the right person to code and identify > actual fixes, but I can test and describe results pretty well. :) > Your detailed description lead me straight to the bugs :) > So I applied the patch (cleanly) to v6.9.3 (because I had it handy) > and mounted with "metadata=on". The first oddity is that the root ovl > directory shows no results for "ls /ovl" (there are lots of dirs in > the lower layer) Was a silly bug - failure to initialize root dentry correctly. It was also observed by several fstests that now pass. > but if I do the same to a directory I know exists, it appears and > returns results just fine (e.g. ls /ovl/thing/blah). Then if I "ls > /ovl" again I see just /ovl/thing but none of the other dirs (until > also accessed by path). > > Anyway, that doesn't really block further testing as the software I > load does not need to walk or interrogate the entries. So then I did a > "chown -h -R bob /blah/thing/stuff/version" and looked at the xattrs > of the upper - all the (metadata) files and dirs were brought up with > files having a redirect, but the dirs that should have > trusted.overlay.opaque=z did not at this stage. Another followup "ls > -lR /blah/thing/stuff/version" and now I can see the > trusted.overlay.opaque=z where I would expect it to be. > > But now when I lookup random NOENT files in those directories, I can > still see the lookup going across the network to the lower filesystem? Second silly bug. > It looks like it's the same for the positive lookups - doing a stat > against a file that I know is in a trusted.overlay.opaque=z directory > still sends the lookup over NFS (which it does not if the directory is > opaque=y). > > I mean, I expect a lookup for an existing file with a metadata > redirect to it for reads but not metadata stat() lookups? Also I would ovl will lookup the lower data of a metacopy file at lookup time. Perhaps we could do lazy lookup of data files, like we do with data-only layers, but this would be more complicated and in any case, stat(2) needs the lower data file to present the correct value of st_blocks. IOW, this patch mainly helps to prevent *negative* lookups in the lower layers. > expect no lookups to the lower for negative lookups? Unless we can't > serve negative lookups from the readdir of the upper dir? Correct expectation. The bug was fixed. It should hopefully work now, although I did not test. > > I have probably misunderstood that the "finalized" directories will > only serve the contents of the readdir result and not send metadata > lookups to the lower level (ala dir=opaque). Or my v6.9.3 kernel has > some other issue unrelated to this patch.... You understood almost correctly, but you need to understand that the finalized directory is not completely opaque, it is partly opaque. You can visualize this as painting the space "between entries" opaque, so that negative lookup results will stop as well as readdir, but ovl will still look *underneath* entries. BTW, ovl will also lookup in lower dirs if entries inside an upper opaque dir have an absolute path redirect xattr. This type of upper opaque dir is also called "impure" and has an "impure" xattr. So you may say that the difference between an opaque=y dir and opaque=z dir is that in the latter, all entries are treated as if they have an absolute path redirect (to their own path), but that is a very hand wavy way of putting it. Anyway, if you are happy with the patch and want to see it upstreamed, I have few requests: 1. Please provide your Tested-by. 2. Please provide a detailed cover letter that explains the problem (*) and how the patch fixes it, which includes performance gain numbers. You may post the cover letter with my patch or ask me to do it 3. Please write an fstest to test the finalized dir feature (**) You may want to look at test overlay/031 for inspiration if also makes changes to layers offline and examines the outcome (*) Please do NOT include the story of changing lower entries under a mounted overlayfs - they are not relevant to this patch and they are very controversial. (**) Basically do chown -R and ls, unmount overlay, add lower entry re-mount overlay and verify that it is not observed. Can also sanity check that opaque=z was set. Note that adding lower entries offline is a behavior that was once allowed and some users actually expect it to work, so I made the feature depend on !ovl_allow_offline_changes(ofs). Therefore, the test should explicitly require and enable metacopy for overlayfs mount to enable the finalized dir feature. Thanks, Amir.
