On Fri, Sep 8, 2023 at 11:15 AM Alexander Larsson <alexl@xxxxxxxxxx> wrote: > > > > On Thu, Sep 7, 2023 at 2:28 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> >> On Thu, Sep 7, 2023 at 11:44 AM Alexander Larsson <alexl@xxxxxxxxxx> wrote: >> > >> > We use the "overlay.whiteouts" to mark any directory in a lower dir that may >> > contain "overlay.whiteout" files (xwhiteouts). This works even if other layers >> > overlap that directory in a mount. >> > >> > For example, take these files in three layers: >> > >> > layer2/dir/ - overlay.whiteouts >> > layer2/dir/file - overlay.whiteout >> > layer1/dir/ >> > layer1/dir/file >> > >> > An overlayfs mount with -o lowerdir=layer2:layer1 would have a whiteout in >> > layer2. >> > >> > However, suppose you wanted to put this inside an overlayfs layer (say >> > "layerA"). I.e. you want to escape the whiteouts above so that when the new >> > layer is mounted using overlayfs the mount shows the above content. The natural >> > approach is to just take each layer and escape it: >> > >> > layerA/layer2/dir/ - overlay.overlay.whiteouts >> > layerA/layer2/dir/file - overlay.overlay.whiteout >> > layerA/layer1/dir/ >> > layerA/layer1/dir/file >> > >> > This initially seems to work, however if there is another lowerdir (say >> > "layerB") that overlaps the xwhiteouts dir, then this runs into problem: >> > >> > layerB/layer2/dir/ - **NO overlay.overlay.whiteouts ** >> > layerA/layer2/dir/ - overlay.overlay.whiteouts >> > layerA/layer2/dir/file - overlay.overlay.whiteout >> > layerA/layer1/dir/ >> > layerA/layer1/dir/file >> > >> > If you mount this with -o lowerdir=layerB:layerA, then in the final mount, >> > there will be no overlay.whiteouts xattrs on the "layer2/dir" merged >> > directory, because the topmost lower dir xattrs win. >> > >> > We would like this to work as is, to avoid having layer escaping depend on other >> > layers. So, to fix this up we special case the reading of escaped >> > "overlay.whiteouts" xattrs by looking in all layers for the first hit. >> > >> >> I have a few issues with this special casing: >> 1. Miklos did not speak his opinion yet >> >> 2. I don't like special casing by suffix nor special casing a single xattr > > > For the suffix part, that is because we need to do the same for multiple escaped xattrs. > > Like, if you have "overlay.overlay.overlay.whiteouts", which gets unescaped to "overlay.overlay.whiteouts" in a mount, you run into the same behaviour: > > layerB/layer2/dir/file3 > layerB/layer2/dir/ - **NO overlay.overlay.overlay.whiteout ** > layerA/layer2/dir/ - overlay.overlay.overlay.whiteout > layerA/layer2/dir/file2 > layerA/layer1/dir/ > layerA/layer1/dir/file1 > > If you were to mount B:A over the 2:1 mount, and you getxattr(overlay.overlay.whiteout) on mnt/dir you would get ENODATA, unless you also special-cased the multiple escaped xattrs. So if you wanted to use this mount to nest yet another level it would not work. > > But, that does speak to the general question that maybe even more xattrs need this kind of treatment. > >> 3. I can think of example like this: >> >> layerB/layer2/dir/file3 >> layerB/layer2/dir/ - **NO overlay.overlay.opaque ** >> layerA/layer2/dir/ - overlay.overlay.opaque >> layerA/layer2/dir/file2 >> layerA/layer1/dir/ >> layerA/layer1/dir/file1 >> >> LayerB doesn't have layer1 so it does not know that >> opaque is needed, therefore opaque needs to be merged >> as well in this case. > > > I think you may be right here. > > My original thought was when using the traditional mechanism for creating lower dirs, i.e. by make overlayfs create an upper and then reuse it as lower, we would not run into this situation. Because when overlayfs copy-ups "layer2/dir" to layerB it would copy the xattrs from the lower dir, so you wouldn't end up with NO overlay.overlay.opaque. > > However, copy_up.c treats the escaped xattrs as ovl_is_private_xattr, so they don't get copied up, even in this setup. > >> >> Can we employ the logic of ovl_xattr_get_first() for any >> escaped xattr prefix on a merge dir? > > > We could either make escaped xattrs be copied up, Seems to me that they should be copied up. A copy up should not hide xattrs that are visible before copy up. > or we could use ovl_xattr_get_first() for all escaped xattrs. > I'm not sure the latter is right though. What if you *wanted* to drop the opaque mark in the new image version (ie. layerB). If we always used ovl_xattr_get_first() then it would not be possible. > I must admit that I personally cannot follow all the user cases that we want to cover and I think that the opaque example above demonstrates that there is no one answer that fits all use cases. > Maybe it's just a fact of life that escaping xattrs like this means you just *have* to care about lower layers when escaping a layer, and we should stop trying to make it not so. However, if we do that, then we should make ovl_copy_xattr() copy up escaped xattrs. > Not sure what you are suggesting. If you are suggesting to drop this patch and fix the xattr escaping patch to copy up escaped xattr then I completely agree. Thanks, Amir.
