Re: [PATCH v2 0/2] livepatch: Move tests from lib/livepatch to selftests/livepatch

[Marcos Paulo de Souza <mpdesouza@suse.de>]

On Fri, Dec 02, 2022 at 01:03:25PM -0700, Shuah Khan wrote:
> On 12/2/22 02:25, Petr Mladek wrote:
> > On Thu 2022-12-01 16:58:38, Shuah Khan wrote:
> > > On 11/30/22 15:22, Joe Lawrence wrote:
> > > > On 7/15/22 10:45 AM, Petr Mladek wrote:
> > > > > On Fri 2022-07-01 16:13:50, Shuah Khan wrote:
> > > > > > On 7/1/22 1:48 AM, Miroslav Benes wrote:
> > > > > > > On Thu, 30 Jun 2022, Shuah Khan wrote:
> > > > > > > > 
> > > > > > > > Sorry Nack on this. Let's not add modules under selftests. Any usage of
> > > > > > > > module_init()
> > > > > > > > doesn't belong under selftests.
> > > > > > 
> > > > > > Yes I did and after reviewing and thinking about it some more, I decided this
> > > > > > is the right direction go down on.
> > > > > 
> > > > > Do you have some particular reason why building modules in selftests
> > > > > directory might cause problems, please?
> > > > > 
> > > 
> > > My reasons are that with this change module_init() propagates out of
> > > strictly kernel space and now is in selftests which are user-space.
> > > Any changes to this interface will be tied to user-space change.
> > 
> > I am sorry but I do not understand the meaning here. module_init() is
> > called when module is loaded. It is not called in userspace.
> > 
> > Maybe, you mean that modules under lib/ are clearly in-tree
> > modules. If we move then under tools/ then they will be build
> > like out-of-tree modules. Except that they will be maintained in-tree
> > so that it will be easy to keep them in sync.
> > 
> 
> Yes. That is what I mean.
> 
> > And I am sure that they will be actively maintained. The fixes are
> > there to make sure that livepatching still works as expected.
> > They must pass when any change is done in the livepatch subsystem.
> > And they must pass when any kernel is released.
> > 
> 
> In other words, livepatch and kernel are revision matched.
> 
> > The only concern might be how build failure is handled. IMHO, we
> > need to handle it the same way and test failure.
> > 
> > 
> 
> Yes. This is another concern - build failures. Let's experiment with
> modules under selftests and see if this becomes a problem.
> 
> > > In general, newer tests offer the best coverage, hence the recommendation
> > > to run newer tests on older kernels assuming that the tests are built
> > > on a newer kernel and backwards should run in a backwards compatible
> > > way on older kernels.
> > 
> > This works for the userspace interface that should always be backward
> > compatible. But it does not work for kABI.
> > 
> 
> This is broader than revision matching. Tests should gracefully exit
> with skip when a config option they depend on is disabled. The same
> gets extended to older kernel versions.

Agreed. I'll change functions.sh to do better checking when compiling modules.

> 
> > 
> > > Do you have a requirement that livepatch test has to be revision
> > > matched with the kernel? Even if that is the case, there is no real
> > > reason to move modules under selftests other than keeping them in
> > > one location.
> > 
> > Yes, kABI is not backward compatible. But building the tests
> > modules out-of-tree way would allow to build test modules with
> > different kABI from the same sources.
> > 
> 
> Okay. This is a solid reason for livepatch modules to live under
> sefltests. Let's capture this in README and the other updates that
> need to be made to it in v3.

I'm happy that we reached to a point where we agreed to have the modules moved
into kselftests, as the move will allow us to implement more tests that are
currently downstream.

The v3 is being prepared now. I hope to send it soon.

Thanks,
  Marcos

> 
> thanks,
> -- Shuah