NixOS Fixes

[blitz]

I'm currently experimenting with generating live patches for NixOS. These are some tiny fixes that make the script work for me.

There are two changes here:

• The first patch fixes the case where kpatch files are installed as read-only (which is the default on NixOS).
• The second patch allows skipping the distro checks, which allows experimenting with new distros. (not necessary)

[joe-lawrence]

The only incredibly small nit I have is that we could group this together like so:

# cp preserves mode and the files might have been read-only. This would
# interfere with cleanup later, so ensure the $TEMPDIR is read/write.
cp -LR "$DATADIR/patch" "$TEMPDIR" || die
chmod -R u+rw "$TEMPDIR" || die

[blitz]

Fixed!

[joe-lawrence]

I'm a little confused, but admit to not having all ~1500 lines of kpatch-build bash code cached in my brain :)

Typical kpatch-build use cases fall into one of the following categories:

1. Building against a distro kernel, in which case this script has (too much) code to try and fetch/expand/etc. that source tree on behalf of the user. There are a few other distro-specific quirks required for this use case as well.
2. Building against an out of tree kernel module ($OOT_MODULE), in which case the distro kernel tree doesn't matter.
3. Building against a local kernel tree ($USERSRCDIR), which like (2) doesn't care about the distro tree.

With that in mind, I'm curious about your use case. Did you not have to specify $USERSRCDIR (case 3) to point kpatch-build to your kernel-tree?

[blitz]

I'm building against the NixOS distro kernel. I'm using -s to point to the checked out source. The specific call looks like this:

    export CACHEDIR=$(mktemp -d)
    kpatch-build -c ${kernel.configfile} -v ${kernel.dev}/vmlinux -s $PWD/source ${patch} \
             --skip-distro-check \
             -j$NIX_BUILD_CORES

• ${kernel.configfile} is the config file that was used to build the original kernel
• ${patch} is the diff that I build a livepatch for
• ${kernel.dev} points to the kernel binaries

As far as I understand, the script tries to install dependencies, but it shouldn't. They are all already there. The script shouldn't do anything distro-specific all.

[blitz]

If I'm holding it wrong, I'm all ears. :)

[joe-lawrence]

I'm still confused (though it is only Monday morning :)

If kpatch-build is invoked with -s|--sourcedir, it should set $USERSRCDIR in command argument parsing:

while [[ $# -gt 0 ]]; do
    case "$1" in
    -s|--sourcedir)
        [[ ! -d "$2" ]] && die "source dir '$2' not found"
        USERSRCDIR="$(readlink -f "$2")"                    # << USERSRCDIR is set to "$PWD/source"
        shift
        ;;

And then it should never take the distribution setup conditional branches later:

if [[ -n "$USERSRCDIR" ]]; then     # << USERSRCDIR is set and not null, take this branch!
    echo "Using source directory at $USERSRCDIR"
    ...
elif [[ -n "$OOT_MODULE" ]]; then
    ...
elif [[ -e "$KERNEL_SRCDIR"/.config ]] && [[ -e "$VERSIONFILE" ]] && [[ "$(cat "$VERSIONFILE")" = "$ARCHVERSION" ]]
    ...
else
    if is_supported_rpm_distro "$DISTRO"; then
        ...
    elif is_supported_deb_distro "$DISTRO"; then
        ...
    else
        [[ "$SKIPDISTROCHECK" -eq 0 ]] && die "Unsupported distribution"

so when you run with -s $PWD/source, you don't see the echo "Using source directory at $USERSRCDIR" message?

[blitz]

Thanks for being persistent here! Indeed, this was probably only an issue in my earlier experimentation. It works without this change.

I've dropped this patch from the PR.

[blitz]

@joe-lawrence I've dropped the unnecessary change. Can you have a look again?

(Sorry for the long delay, but it was 🌴 vacation 🌴 time.)

[joe-lawrence]

Thanks, @blitz !