Re: [LTP] [PATCH] ioctl_pidfd02-06: Add CONFIG_USER_NS and CONFIG_PID_NS to needs_kconfigs

[Petr Vorel <pvorel@suse.cz>]

Hi all,

[ Cc Martin ]

> Hi!
> > > > Because that was my question - really always prefer kconfig even there is a
> > > > simple runtime solution? I'd like to have some "rule" like conclusion we can
> > > > point during review.

> > > I think that from a long term view this is going to be simpler solution
> > > than having many different types of checks. The less diverse these
> > > checks are the easier they are to review and maintain. Hence I lean
> > > towards kernel config checks even though they are slower (mostly
> > > unmeasurable on today's harware) than the alternatives.

> > I think I lean opposite way, and rather have a check for right
> > environment to support the test.
> > You can have feature X enabled in kernel config, but still disabled
> > later at boot/runtime
> > (e.g. max_user_namespaces=0), or a module simply not being loaded.

> That is a good catch.

> Maybe the best way forward would be to add hooks for certain config
> options into the LTP kernel config parser that would do additional
> runtime checks. That way we would have both the information on which
> kernel configs should be enabled in test metadata as well as runtime
> checks.

I like the idea: having metadata doc + runtime check is nice.

FYI (I know you all watch GitHub PR but just in case) to document that for some
tests require more things to check, e.g. kconfig + min_kver:
https://github.com/linux-test-project/ltp/pull/1285

Kind regards,
Petr

> What about eventually adding something as:

> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 9bcd57721..f6abe6cc7 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -110,6 +110,18 @@ static void close_kconfig(FILE *fp)
>                 fclose(fp);
>  }

> +static void runtime_check(struct tst_kconfig_var *var)
> +{
> +       if (strstr(var->id, "CONFIG_USER_NS")) {
> +               if (!tst_user_ns_enabled()) {
> +                       tst_res(TINFO, "CONFIG_USER_NS present but runtime is disabled");
> +                       var->val = 'n';
> +               }
> +       } else if (...)
> +               ...
> +       }
> +}
> +
>  static inline int kconfig_parse_line(const char *line,
>                                       struct tst_kconfig_var *vars,
>                                       unsigned int vars_len)
> @@ -183,9 +195,11 @@ out:
>                         switch (val[0]) {
>                         case 'y':
>                                 vars[i].choice = 'y';
> +                               runtime_check(&vars[i]);
>                                 return 1;
>                         case 'm':
>                                 vars[i].choice = 'm';
> +                               runtime_check(&vars[i]);
>                                 return 1;
>                         }
>                 }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp