[usebeforefree]

At the moment kpatch-build doesn't support kernel variants (e.g., +64k , +rt_64k), this PR aims to add support for kpatch-build and testing for them. Tested on kernel-64k-aarch64.

Cc: @joe-lawrence

[joe-lawrence]

@usebeforefree - thanks for the PR. The parsing and code looks mostly good, I have a few cosmetic suggestions:

• Naming: I'd prefer the term used in the kernel specfile: VARIANT / variant (not FLAVOR / flavor)
• Tabs vs. spaces: use tab convention in lib.sh
• Commit message:
  • add prefix to subject line: "kpatch-build, lib.sh: "
  • note that these variants are for RHEL-derived distributions
  • since I assume this is work related, we usually Signed-off-by with our work email

Also, at some point would you later update the testing farm / packit CI to run aarch64-64k ?

[usebeforefree]

Thanks for the review @joe-lawrence !

Applied all the suggestions.

(not FLAVOR / flavor)

I used the flavor term because I couldn't find the proper term, and saw this one used in kpatch-build:

kpatch/kpatch-build/kpatch-build

Lines 981 to 988 in 74ac9ff

	# handle flavor extension on Photon ex) -rt, -esx
	if [[ "$DISTRO" = photon ]]; then
	KREL="${ARCHVERSION#*-}"
	# strip rt patchset version if present
	# remove trailing -flavor and starting -rt### patchset
	KREL="${KREL%-*}"
	KREL="${KREL#*-}"

I can find-and-replace them if we'd like to use the proper term.

Also, at some point would you later update the testing farm / packit CI to run aarch64-64k ?

I will do this in a separate PR, otherwise we'd create circular dependency within them.

[joe-lawrence]

(not FLAVOR / flavor)

I used the flavor term because I couldn't find the proper term, and saw this one used in kpatch-build:

kpatch/kpatch-build/kpatch-build

Lines 981 to 988 in 74ac9ff

	# handle flavor extension on Photon ex) -rt, -esx
	if [[ "$DISTRO" = photon ]]; then
	KREL="${ARCHVERSION#*-}"
	# strip rt patchset version if present
	# remove trailing -flavor and starting -rt### patchset
	KREL="${KREL%-*}"
	KREL="${KREL#*-}"

I can find-and-replace them if we'd like to use the proper term.

You can leave the Photon "flavor" references in place. I have no idea if that's what that distribution uses that naming convention, I just thought for RHEL-based releases we should adopt what its kernel specfile is already using. (Naming things is already hard enough :)

Also, at some point would you later update the testing farm / packit CI to run aarch64-64k ?

I will do this in a separate PR, otherwise we'd create circular dependency within them.

Sounds good.

[joe-lawrence]

@Tomcat-42 @ykopkova : I have no idea how to add @usebeforefree, or anybody for that matter, to the testing farm access. The current results say, "usebeforefree can't run tests internally". How can we add him? Thanks.

[usebeforefree]

Update: I would actually like to enable the packit tests in this PR, working on it in the meantime.

[ykopkova]

@Tomcat-42 @ykopkova : I have no idea how to add @usebeforefree, or anybody for that matter, to the testing farm access. The current results say, "usebeforefree can't run tests internally". How can we add him? Thanks.

I "think" only project maintainers or members can trigger packit ci testing.

[usebeforefree]

Update:

I came up with this solution for adding the kernel-64k-aarch64 test case. I've added a commit that adds the case and targets a test to be added inside of kernel-tests that is defined here:

https://gitlab.com/redhat/centos-stream/tests/kernel/test-plans/-/merge_requests/233/diffs?commit_id=37b4f38573c391ea535934e5a66782253ef20122

I see the test case is called in this PR, someone just needs to figure out a way to make the pipeline accept it. Though we firstly need to target the test-plans branch above mentioned, so we can test it out before merging. What's a good way to do this?

The easiest certainly would be:

• force pushing changes to make packit target the new kernel-tests branch:

diff --git a/.packit.yml b/.packit.yml
index 1939f6f..905a82c 100644
--- a/.packit.yml
+++ b/.packit.yml
@@ -61,8 +61,8 @@ _:
     skip_build: true
     manual_trigger: false
     tmt_plan: kpatch-integration-rhel/kernel-64k
-    fmf_url: https://gitlab.com/redhat/centos-stream/tests/kernel/test-plans.git
-    fmf_ref: main
+    fmf_url: https://gitlab.com/mkostic1/test-plans.git
+    fmf_ref: kernel-64k-aarch64-integration

• run the packit pipeline, make sure it passes
• merge the kernel-tests MR
• force push back the packit target to main
• merge this PR

Note to self: the same branch shenanigans could be done with test-plans as well (targeting a specific kernel-tests branch), so we don't have to merge anything before making sure things actually work.

Side question:

Should we update other tests to target rhel-10.3?

[joe-lawrence]

Update:

I came up with this solution for adding the kernel-64k-aarch64 test case. I've added a commit that adds the case and targets a test to be added inside of kernel-tests that is defined here:

https://gitlab.com/redhat/centos-stream/tests/kernel/test-plans/-/merge_requests/233/diffs?commit_id=37b4f38573c391ea535934e5a66782253ef20122

I see the test case is called in this PR, someone just needs to figure out a way to make the pipeline accept it. Though we firstly need to target the test-plans branch above mentioned, so we can test it out before merging. What's a good way to do this?

The easiest certainly would be:

• force pushing changes to make packit target the new kernel-tests branch:

diff --git a/.packit.yml b/.packit.yml
index 1939f6f..905a82c 100644
--- a/.packit.yml
+++ b/.packit.yml
@@ -61,8 +61,8 @@ _:
     skip_build: true
     manual_trigger: false
     tmt_plan: kpatch-integration-rhel/kernel-64k
-    fmf_url: https://gitlab.com/redhat/centos-stream/tests/kernel/test-plans.git
-    fmf_ref: main
+    fmf_url: https://gitlab.com/mkostic1/test-plans.git
+    fmf_ref: kernel-64k-aarch64-integration

• run the packit pipeline, make sure it passes
• merge the kernel-tests MR
• force push back the packit target to main
• merge this PR

Note to self: the same branch shenanigans could be done with test-plans as well (targeting a specific kernel-tests branch), so we don't have to merge anything before making sure things actually work.

You know more than me about the testing :D, so I'd defer to Yulia / Pablo about this.

Side question:

Should we update other tests to target rhel-10.3?

The next sprint we should be updating/pushing out final integration tests for the next y-stream releases. I think that's a pre-req for what you're asking here?

[ykopkova]

/packit test

[usebeforefree]

/packit test

[Tomcat-42]

/packit test

[usebeforefree]

not ready for merging, the 64k test is pointed to a custom branch

[usebeforefree]

/packit test

[usebeforefree]

/packit test

[usebeforefree]

The tests for all arches except kernel-64k-aarch64 are passing, so no regressions. The failing test I introduced in this PR is due to the master branch of kpatch not including integration-rhel10.3 tests. Which I will add in a new PR.

The only thing left really here is an issue I discovered along the way. The TLDR is that with this proposed fix you can't use the kpatch-build -r aka --sourcerpm argument with a kernel variant. Not only you cannot use it, it may introduce issues as it can produce bad results, because it will default to the non variant kernel. That's because the --sourcerpm does not describe the kernel variant, so we can't derive it.

kpatch/kpatch-build/kpatch-build

Lines 916 to 930 in 74ac9ff

	if [[ -n "$SRCRPM" ]]; then
	if [[ -n "$ARCHVERSION" ]]; then
	warn "--archversion is incompatible with --sourcerpm"
	exit 1
	fi
	rpmname="$(basename "$SRCRPM")"
	if [[ "$DISTRO" = photon ]]; then
	ARCHVERSION="${rpmname%.src.rpm}"
	else
	ARCHVERSION="${rpmname%.src.rpm}.$(uname -m)"
	fi
	ARCHVERSION="${ARCHVERSION#kernel-}"
	ARCHVERSION="${ARCHVERSION#alt-}"
	ARCHVERSION="${ARCHVERSION#linux-}"
	fi

[usebeforefree]

My proposal for how to fix the issue when calling -r is either:

1. Detecting kernel variant and exiting

	if  [[  "$(uname -r)" =~ \+(.*)$ ]]; then
		warn "kernel variants are incompatible with --sourcerpm"
		exit 1
	fi

2. Detecting kernel variant and appending it to the $SRCRPM derived ARCHVERSION:

   if [[ "$(uname -r)" =~ \+(.*)$ ]]; then
   	ARCHVERSION="${ARCHVERSION}+${BASH_REMATCH[1]}"
   fi

No idea what would be the better option. Exiting seems safer in any case, but I don't have the experience of how using kpatch-build to say if 2. solution would be viable.

[joe-lawrence]

My proposal for how to fix the issue when calling -r is either:

1. Detecting kernel variant and exiting

	if  [[  "$(uname -r)" =~ \+(.*)$ ]]; then
		warn "kernel variants are incompatible with --sourcerpm"
		exit 1
	fi

2. Detecting kernel variant and appending it to the $SRCRPM derived ARCHVERSION:

   if [[ "$(uname -r)" =~ \+(.*)$ ]]; then
   	ARCHVERSION="${ARCHVERSION}+${BASH_REMATCH[1]}"
   fi

No idea what would be the better option. Exiting seems safer in any case, but I don't have the experience of how using kpatch-build to say if 2. solution would be viable.

Here is how the kpatch-patch.spec runs kpatch-build:

# ... close your eyes, kids, it's .specfile voodoo ...
kpatch-build/kpatch-build -n %{patchmodname} -r %{SOURCE10} -v $vmlinux --skip-cleanup $patch_sources

I see, that's an interesting problem: the user initiates a kpatch-build and specifies the source rpm. Up until now, we could assume that the user was interested in the same-arch (non-variant) configuration from the source rpm. With this arm64 64k variant, that assumption may not always hold true. (BTW, I think this may be a similar problem if one wanted to build for modern RHEL realtime variant as well.)

I think the "right" answer is probably an additional --variant option that complements --sourcerpm. That way if one wants to build kpatches for both variants from a kernel-1234.aarch64 boot, they can run one w/o and one w/--variant 64k or something like that. (Or similarly for -rt if that ever needed to be supported.)

Does that capture the last hurdle and work as a solution?

[usebeforefree]

I think the proposal would work, but only if running kpatch-build on ex. kernel-64k without specifying --variant 64k doesn't cause potential major failures.

I don't have the expertise to know this, what I do have are logs of tests that failed due to this exact situation, and an LLM analysis:

A mismatch can at least cause a user to waste time on building the wrong kernel (see :: [ 14:09:02 ] :: timestamp in the logs), and also poison the cache with the wrong kernel config. Worst case scenario a buggy module that may cause a kernel panic?

If the above is true, maybe we can just add an internal check for mismatch between running system variant and (non)passed variant?

edit: I am confused about the usage of kpatch-build across different host and target systems, so feel free to just say if all this above is not really relevant, and I'll proceed to implement --variant.