[PATCH i-g-t v3] lib/igt_kmod: Allow to load kunit from alias

Lucas De Marchi lucas.demarchi at intel.com
Sat Mar 16 00:47:24 UTC 2024


On Fri, Mar 15, 2024 at 07:15:51AM +0100, Mauro Carvalho Chehab wrote:
>On Fri,  8 Mar 2024 11:31:49 -0800
>Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>
>> Instead of only allowing to load the from the module name, also allow it
>> from alias by using kmod_module_new_from_lookup(). With that a user can
>> have their on aliases and the kernel can also export a compat alias for
>> future module renames.
>>
>> There was a bug in libkmod <= 29 that would make
>> kmod_module_new_from_lookup() return prematurely after a failed alias
>> lookup when we don't have a modules.alias file in the modules dir. So
>> add a workaround to check for -ENOSYS return code too. It should still
>> be compatible with what we are doing here.
>>
>> v2: Fallback to kmod_module_new_from_name() since the kmod ctx is also
>>     used on builder machines to determine the test lists.
>> v3: Add workaround to bug in libkmod
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  lib/igt_kmod.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>> index cc242838f..d581f9073 100644
>> --- a/lib/igt_kmod.c
>> +++ b/lib/igt_kmod.c
>> @@ -1443,6 +1443,7 @@ void igt_kunit(const char *module_name, const char *suite, const char *opts)
>>  int igt_ktest_init(struct igt_ktest *tst,
>>  		   const char *module_name)
>>  {
>> +	struct kmod_list *l = NULL;
>>  	int err;
>>
>>  	memset(tst, 0, sizeof(*tst));
>> @@ -1453,9 +1454,25 @@ int igt_ktest_init(struct igt_ktest *tst,
>>
>>  	tst->kmsg = -1;
>>
>> -	err = kmod_module_new_from_name(kmod_ctx(), module_name, &tst->kmod);
>> -	if (err)
>> +	err = kmod_module_new_from_lookup(kmod_ctx(), module_name, &l);
>> +
>> +	/*
>> +	 * Check for -ENOSYS to workaround bug in kmod_module_new_from_lookup()
>> +	 * from libkmod <= 29
>> +	 */
>
>Hmm... if the bug is not present anymore on more modern libkmod versions,
>shouldn't the code be verifying the libkmod version?

unless we block the build with libkmod <= 30, no. But that would break a
lot of the enterprise distros still stuck on older libkmod and upstream
CI - I saw some using versions as old as 26.

I don't think it's a good practice to check version number in runtime as
the fixes could had been made available by distros themselves. We could
check the *behavior* and adapt, but in this case I don't think it's
worth.

>
>Also, better to add a link to the bug inside the comment, in case the
>workaround would require maintenance in the future.
>
>With that, feel free to add my:
>
>Reviewed-by: Mauro Carvalho Chehab <mchehab at kernel.org>

patch has already been applied a few days ago, but thanks.

Lucas De Marchi


More information about the igt-dev mailing list