Rework the logic supporting OverlayFS/docker

[Adam-pi3]

The original logic was hooking 'ovl_create_or_link' function but it could be inlined. This commit changes it by hooking 'ovl_dentry_is_whiteout' when possible.

Fixes #215

Description

Rework the logic supporting OverlayFS/docker

How Has This Been Tested?

By running docker (hello-world and fedora) under: Distributor ID: Ubuntu Description: Ubuntu 22.04 LTS Release: 22.04 Codename: jammy

[Adam-pi3]

It looks like that if we want to support RHEL7 (and others?) which uses very old base kernel (3.10.xxx) we would need to modify current preprocessor macros:

[pi3@localhost ~]$ cat /etc/redhat-release 
CentOS Linux release 7.9.2009 (Core)
[pi3@localhost ~]$ cat /proc/kallsyms |grep ovl_dentry_is_whiteout
0000000000000000 t ovl_dentry_is_whiteout   [overlay]
[pi3@localhost ~]$ cat /proc/kallsyms |grep ovl_create_or_link
0000000000000000 t ovl_create_or_link   [overlay]

Based on the information from that link (https://batmat.net/2015/10/05/upgrading-centos-7-kernel-to-enable-using-overlay-with-docker/) it sounds like OverlayFS is supported since RHEL 7.2. Although, I'm not sure which logic of hooking we should use for it. I'm suggesting to just live the latest one (hooking ovl_dentry_is_whiteout). @solardiz what do you think?

[solardiz]

FWIW, on older (non-updated) CentOS 7 after a modprobe overlay I have ovl_create_or_link, but no ovl_dentry_is_whiteout. There is ovl_dentry_is_opaque. This suggests they had backported older mainline code first, then (per Adam's comment above) updated it to newer.

[solardiz]

@vt-alt Two CI checks here are failing for reasons unrelated to this PR's changes. I tried rerunning them, they failed again.

[vt-alt]

Yes Impish is EOL'd July 14, we need to delete it. Hirsute is EOL'd even earlier in January. https://wiki.ubuntu.com/Releases#End_of_Life

[Adam-pi3]

@solardiz I just pushed the support for RHEL7.2+ which relies on ovl_dentry_is_whiteout. Older RHEL will be getting ovl_create_or_link hook with old logic

[solardiz]

it sounds like OverlayFS is supported since RHEL 7.2.

Ideally, we'd locate more reliable information on when this was introduced. Checking the kernel package log:

* Tue Dec 23 2014 Jarod Wilson <jarod@redhat.com> [3.10.0-220.el7]
[...]
- [fs] overlayfs: filesystem (David Howells) [985875]

Now need to map that to RHEL version. This has them:

https://www.redhat.com/en/blog/what-latest-kernel-release-my-version-red-hat-enterprise-linux

and per that table (somehow a picture), RHEL 7.1 already had -229, so had overlayfs. That's actually consistent with the blog post you found.

Edited: So if we have a need to check for "any overlayfs support", we'd need to check the minor for >= 1 or > 0, but we probably actually don't have that need? Rather, we need to check when support changed:

pushed the support for RHEL7.2+ which relies on ovl_dentry_is_whiteout. Older RHEL will be getting ovl_create_or_link hook with old logic

This looks wrong to me, in at least two ways:

1. You say 7.2+, but the code checks 7.3+ (> 2). We should have consistent commit message and code.
2. Do we know when RHEL actually upgraded their overlayfs support such that it switched from ovl_dentry_is_whiteout to ovl_create_or_link? We don't seem to have found that yet.

Maybe we should simplify the compile-time checks and instead try hooking ovl_dentry_is_whiteout and if that fails then hook ovl_create_or_link instead and adjust our expected off count accordingly (e.g., keep it on our r/o page).

If we keep the complicated compile-time checks, then we should probably more them to an .h file and define a tri-state numeric macro (like 0, 1, 2 for no, old, new overlayfs) and another for the function name. That would simplify the .c files. It would also avoid us abusing the ..._H macro.

[solardiz]

Edited the previous commit to say "So if we have a need to check for "any overlayfs support", we'd need to check the minor for >= 1 or > 0, but we probably actually don't have that need? Rather, we need to check when support changed."

The commit that's now called "Add RHEL7.2+ support in the new OverlayFS logic" actually also adds support for some 4.x kernel ranges. Perhaps that should be part of a previous commit, or maybe we don't need a separate commit for RHEL.

[solardiz]

Also, the second commit is basically a fix-up of the first one, so those changes should ideally be part of the first commit instead. This whole PR should probably be one commit.

[solardiz]

@vt-alt FWIW, curiously all the checks passed after Adam added a commit here, without rebasing, so yesterday's issues were not permanent yet (or maybe not yet propagated to all mirrors). Anyway, it's fine we no longer rely on those EOL'ed versions of Ubuntu.

[solardiz]

Found this:

* Wed Dec 13 2017 Rafael Aquini <aquini@redhat.com> [3.10.0-822.el7]
[...]
- [fs] ovl: add ovl_dentry_is_whiteout() (Miklos Szeredi) [1485392]

That's between 7.4 and 7.5. So perhaps we need to check for > 4 or >= 5 (a matter of style), and have 7.5+ in the commit message if we do have that as a separate commit at all.

[solardiz]

per that table (somehow a picture)

Here are RHEL versions to kernel revision number mappings in plain HTML: https://access.redhat.com/articles/3078

Not mentioned there, but I saw elsewhere, that apparently RHEL7 Aarch64 uses RHEL8'ish 4.18.x kernels. I don't know how they set the RHEL version macros for kernel module builds there - would we be seeing RHEL7 or RHEL8? If we see RHEL7, but the kernels are 4.18.x, we could build wrongly for those kernels. Anyway, I think that's exotic and shouldn't be our concern unless someone reports an issue. It is, however, yet another reason for us to prefer detection of actual kernel properties rather than versions.

@Adam-pi3 For now, I suggest you change your RHEL7 minor version checks to be of > 4 and squash the commits and force-push the one commit. You might also want to implement the macros like I suggested, or you can leave that for me.

[solardiz]

This check in the .h file:

#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 7, 0) || \
    (defined(RHEL_RELEASE_CODE) && RHEL_RELEASE_CODE > RHEL_RELEASE_VERSION(7, 2))

isn't yet updated to include 4.4.179+. I think it should be.

This comments is probably wrong, or the preceding check might be:

    /* Between the kernels 4.6-4.10 'ovl_dentry_is_whiteout' function does not exist */

We actually care about 4.7+ and we reach that #else for below 4.10, so under either reasoning 4.6-4.10 inclusive feels irrelevant.

#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 10, 0) || \
    (LINUX_VERSION_CODE >= KERNEL_VERSION(4, 4, 179) && LINUX_VERSION_CODE < KERNEL_VERSION(4, 5, 0)) || \
    (defined(RHEL_RELEASE_CODE) && RHEL_RELEASE_CODE > RHEL_RELEASE_VERSION(7, 2))
     "ovl_dentry_is_whiteout' function. "
#else
    /* Between the kernels 4.6-4.10 'ovl_dentry_is_whiteout' function does not exist */
     "ovl_create_or_link' function. "
#endif

[solardiz]

I think this comment:

    /* Between the kernels 4.6-4.10 'ovl_dentry_is_whiteout' function does not exist */

should say:

    /* Between the kernels 4.7 and 4.9, the 'ovl_dentry_is_whiteout' function does not exist */

or better yet simply:

    /* In kernels below 4.10, the 'ovl_dentry_is_whiteout' function does not exist */

because the 4.7+ dependency is introduced through a check that's not nearby in our source code, and because technically the function does also exist in many kernels below 4.7 - we just don't use it there.

[solardiz]

An issue I missed in my final review: the check now in the .h file now also requires RHEL 7.5+ (when we're on RHEL at all) to include any hooking at all. However, perhaps it should apply only for which function to hook, not for whether to hook. For the latter, we should probably check RHEL 7.1+, or maybe even some in-between version in case there was a time when ovl_create_or_link already existed in RHEL but didn't yet have the unbalanced override/revert (like upstream didn't before 4.7).

[solardiz]

check RHEL 7.1+, or maybe even some in-between version in case there was a time when ovl_create_or_link already existed in RHEL but didn't yet have the unbalanced override/revert

This is actually quite likely. RHEL 7.1's introduction of overlayfs is December 2014, Linux 4.7 is mid-2016. OTOH, it is also possible that RHEL jumped over the problematic revisions, in which case LKRG's current code is correct (but over-complicated in that it kind of "handles" now-compile-time-impossible hooking of ovl_create_or_link on RHEL below 7.5). It'd take further research to figure this out, and this is probably not worth our time (hopefully, there are very few or maybe even no RHEL7 installs still at below 7.5 and needing LKRG and overlayfs).