kpatch-build: building OOT livepatch modules for kernels built from source #1234
Conversation
| if [[ -z "$OOT_MODULE" ]]; then | ||
| KPATCH_BUILD="$SRCDIR" | ||
| else |
There was a problem hiding this comment.
$(dirname "$CONFIGFILE") doesn't sound great, I think we'll be better off using $SRCDIR instead.
| if [[ -z "$OOT_MODULE" ]]; then | |
| KPATCH_BUILD="$SRCDIR" | |
| else | |
| if [[ -z "$OOT_MODULE" || ! -e "/lib/modules/$ARCHVERSION/build" ]]; then | |
| KPATCH_BUILD="$SRCDIR" | |
| else | |
| KPATCH_BUILD="/lib/modules/$ARCHVERSION/build" | |
| fi |
There was a problem hiding this comment.
I tried $SRCDIR and it didn't work because if script is invoked from directory with OOT module, it does invoke make -C $SRCDIR, which does nothing, because -C must have kernel source directory as its argument.
There was a problem hiding this comment.
SRCDIR should contain the path to kernel source directory here. Here where it gets assigned: https://github.com/dynup/kpatch/blob/master/kpatch-build/kpatch-build#L675. And here we make sure USERSRCDIR is set when we use OOT_MODULE: https://github.com/dynup/kpatch/blob/master/kpatch-build/kpatch-build#L660.
There was a problem hiding this comment.
But this repo's README has the invocation example, and it states that SRCDIR is assigned module's source directory:
Line 588 in 61fc3f1
There was a problem hiding this comment.
Yes, you are right. This semantic change is rather confusing. So are you relying on user-specified config then (through --config option)? Because if it is not specified that way it will be set to /boot/config-xxx (https://github.com/dynup/kpatch/blob/master/kpatch-build/kpatch-build#L738).
It seems to me that instead of changing --source semantics it'll be better to add another option for OOT_MODULE case that will define OOT_MODULE_PATH or something in that vein. We then'll be able to keep using SRCDIR normally
There was a problem hiding this comment.
Yes, I thought about that. Will be working on it next week and remove confusing semantics. Thanks for reviewing!
There was a problem hiding this comment.
Added -p | --oot-module-src option, allowing to specify module source and use -s in conventional way, only for kernel. Should I change the README of the repo too?
| KPATCH_BUILD="$SRCDIR" | ||
|
|
||
| if [[ -z "$OOT_MODULE" || ! -e "/lib/modules/$ARCHVERSION/build" ]]; then | ||
| KPATCH_BUILD="$USERSRCDIR" |
There was a problem hiding this comment.
Hi, thanks for the update.
Unfortunately this change breaks the builds that don't specify $USERSRCDIR.
There was a problem hiding this comment.
But, if I understood correctly, that was the whole point of
It seems to me that instead of changing
--sourcesemantics it'll be better to add another option forOOT_MODULEcase that will defineOOT_MODULE_PATHor something in that vein. We then'll be able to keep usingSRCDIRnormally
In what cases the build will break?
There was a problem hiding this comment.
In what cases the build will break?
Sorry, I should have been more clear. This breaks the normal build (without oot_module) without USERSRCDIR In that case SRCDIR should be used for kpatch-build. I think ideally we would keep SRCDIR untouched by OOT_MODULE code and only use OOT_SOURCE directly when needed. Can't tell if that will be easy to do though, we don't normaly build out-of-tree modules so I am not very familiar with that code.
There was a problem hiding this comment.
Unfortunately, if we do not set SRCDIR to OOT_SOURCE and use it directly, that will add to many if's and that is not good. I tried to mitigate the issue in easier way, please check my last update.
There was a problem hiding this comment.
At what line does OOT_MODULE depend on USERSRCDIR?
There was a problem hiding this comment.
At what line does OOT_MODULE depend on USERSRCDIR?
Line#666 is the main place but there are also others like Line#692 which is inside if [[ -n "$USERSRCDIR" ]]; then clause.
There was a problem hiding this comment.
Well, i don't think it shouldn't depend on USERSRCDIR. Where else can we get working Kbuild?
There was a problem hiding this comment.
Normally (for non-oot cases) if USERSRCDIR is not specified kpatch-build would download running distro kernel's sources and prepare them for build in SRCDIR (~/.kpatch/src by default). There is this whole section handling it. I think that can work for OOT modules as well.
…ource Ability to build out-of-tree modules for non-host kernel is currently missing. This commit introduces that ability for any kernel built from source, including cross-compiled ones. The workaround checks for existence of /lib/modules/$ARCHVERSION/build, and if nonexistent, assumes the kernel source directory to be a build directory.
Previously OOT-module builds used `--sourcedir` to specify oot-module source directory which was a bit confusing and also denied users ability to build kpatches agains oot modules built against non-distro kernels. This patch adds `-p/--oot-module-src` option to specify source dir for oot module while keeping `--sourcedir` for kernel source directory specification. I also tried to disambiguate `SRCDIR` in kpatch-build a bit. Now there are 3 variables: - `KERNEL_SRCDIR` - contains path to kernel source directory - `OOT_MODULEL_SRCDIR` - contains path to out-of-tree module source directory - `SRCDIR` - can be set to either of the above and is used for patch-related actions Another attempt at this was done by @omatiusha in dynup#1234 Signed-off-by: Artem Savkov <asavkov@redhat.com>
|
Hi, Since there haven't been any updates for a while I took a stab at it in #1242. Hopefully that solution works for your usecase, please take a look. I am closing this PR as duplicate for now, but it can be reopened if needed. |
Previously OOT-module builds used `--sourcedir` to specify oot-module source directory which was a bit confusing and also denied users ability to build kpatches agains oot modules built against non-distro kernels. This patch adds `-p/--oot-module-src` option to specify source dir for oot module while keeping `--sourcedir` for kernel source directory specification. I also tried to disambiguate `SRCDIR` in kpatch-build a bit. Now there are 3 variables: - `KERNEL_SRCDIR` - contains path to kernel source directory - `OOT_MODULEL_SRCDIR` - contains path to out-of-tree module source directory - `SRCDIR` - can be set to either of the above and is used for patch-related actions Another attempt at this was done by @omatiusha in dynup#1234 Signed-off-by: Artem Savkov <asavkov@redhat.com>
|
Hi, I was trying to get it to work but you did it first, thank you so much! |
Previously OOT-module builds used `--sourcedir` to specify oot-module source directory which was a bit confusing and also denied users ability to build kpatches agains oot modules built against non-distro kernels. This patch adds `-p/--oot-module-src` option to specify source dir for oot module while keeping `--sourcedir` for kernel source directory specification. I also tried to disambiguate `SRCDIR` in kpatch-build a bit. Now there are 3 variables: - `KERNEL_SRCDIR` - contains path to kernel source directory - `OOT_MODULEL_SRCDIR` - contains path to out-of-tree module source directory - `BUILDDIR` - can be set to either of the above and is used for patch-related actions Another attempt at this was done by @omatiusha in dynup#1234 Signed-off-by: Artem Savkov <asavkov@redhat.com>
2 participants
Ability to build out-of-tree modules for non-host kernel is currently
missing. This commit introduces that ability for any kernel built
from source, including cross-compiled ones. The workaround checks for
existence of /lib/modules/$ARCHVERSION/build, and if nonexistent,
assumes the kernel source directory to be a build directory.
This PR is subsequent to #1233