Re: [PATCH v5 3/7] lib: Add support for max_kver to struct tst_test and tst_fs

Hi Cyril,

> Hi!
> > +/*
> > + * Check for the maximal required kernel version.
> > + *
> > + * return: true if the kernel version is low enough, false otherwise.
> > + */
> > +static bool check_max_kver(const char *max_kver, const int brk_nosupp)
> > +{
> > +	char *msg;
> > +	int dots, i, v1, v2, v3;
> > +
> > +	tst_parse_kver(max_kver, &v1, &v2, &v3);
> > +
> > +	for (i = 0, dots = 0; max_kver[i]; i++)
> > +		dots += (max_kver[i] == '.');
> > +
> > +	/*
> > +	 * For mainline kernel release without patch level (single dot e.g. "7.1")
> > +	 * ignore v3 (the sublevel): 7.1.x is always ok.
> > +	 * Do *not* ignore v3 on stable kernel release (2 dots, e.g. 7.1.5).
> > +	 */
> > +	if (tst_kvercmp(v1, v2, v3) > (dots == 1 ? 1023 : 0)) {

> Uff, this is quite ugly. What exactly are we trying to do?

I hoped that was explained in the comment above the code:
 .max_kver = "7.0" should run the test not only kernel <= 7.0,
but also all stable kernels: 7.0.x.
OTOH if there is something backported to stable kernel and one specifies:
.max_kver = "7.0.5" it will be compared just to <= 7.0.5.

Sure, this can be avoided if .max_kver = "7.0" is not inclusive,
i.e. < 7.0 (one would have to use .max_kver = "7.1"), which is less
intuitive, because .min_kver is inclusive).

> Also we are missing wiring for the shell test executor:

I deliberately did not add it and documented my intention in
doc/developers/writing_tests.rst LTP C And Shell Test API Comparison table
+    * - .max_kver
+      - not implemented

The reason is that it's not needed now for shell tests. But sure I can add it.

Kind regards,
Petr

> iff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
> index 2778fb6d5..263a319bc 100644
> --- a/testcases/lib/tst_run_shell.c
> +++ b/testcases/lib/tst_run_shell.c
> @@ -57,6 +57,7 @@ enum test_attr_ids {
>         DEV_MIN_SIZE,
>         FILESYSTEMS,
>         FORMAT_DEVICE,
> +       MAX_KVER,
>         MIN_CPUS,
>         MIN_MEM_AVAIL,
>         MIN_KVER,
> @@ -91,6 +92,7 @@ static ujson_obj_attr test_attrs[] = {
>         UJSON_OBJ_ATTR_IDX(DEV_MIN_SIZE, "dev_min_size", UJSON_INT),
>         UJSON_OBJ_ATTR_IDX(FILESYSTEMS, "filesystems", UJSON_ARR),
>         UJSON_OBJ_ATTR_IDX(FORMAT_DEVICE, "format_device", UJSON_BOOL),
> +       UJSON_OBJ_ATTR_IDX(MAX_KVER, "max_kver", UJSON_STR),
>         UJSON_OBJ_ATTR_IDX(MIN_CPUS, "min_cpus", UJSON_INT),
>         UJSON_OBJ_ATTR_IDX(MIN_MEM_AVAIL, "min_mem_avail", UJSON_INT),
>         UJSON_OBJ_ATTR_IDX(MIN_KVER, "min_kver", UJSON_STR),
> @@ -462,6 +464,9 @@ static void parse_metadata(void)
>                 case FORMAT_DEVICE:
>                         test.format_device = val.val_bool;
>                 break;
> +               case MAX_KVER:
> +                       test.max_kver = strdup(val.val_str);
> +               break;
>                 case MIN_CPUS:
>                         if (val.val_int <= 0)
>                                 ujson_err(&reader, "Minimal number of cpus must be > 0");

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