tst_kvcmp: Add support for extra kernel versions

RFC PATCH New Cyril Hrubis 2 participants 9 messages Apr 21, 2017 – Apr 28, 2017 Library code

lore

Share & tools

lore

Cyril Hrubis

author

2017-04-21 15:52 UTC #0 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

This adds support for extra kernel versions to the tst_kvcmp shell helper.

Now we can append the extra version(s) after the generic kernel version in the expression. For example to check for older kernel than 3.0 or in case of RHEL6 older than 2.6.32 we do:

tst_kvcmp -lt '3.0 RHEL6:2.6.32'

This commit also reduces the number of uname() syscalls needed to acquire the result as a side efect.

Signed-off-by Cyril Hrubis <[email protected]>

CC Jan Stancek <[email protected]>

---

Jan can you please check that everything works fine when specifying extra version on RHEL? It seems that the only systems that are supported by extra kernel versions are RHEL5 and RHEL6 at the moment.

 include/tst_kvercmp.h     | 20 ++++++++++++++---
 lib/tst_kvercmp.c         | 54 ++++++++++++++++++++++++++++----------------
 testcases/lib/tst_kvcmp.c | 57 +++++++++++++++++++++++++++++++++++++----------
 3 files changed, 97 insertions(+), 34 deletions(-)

include/tst_kvercmp.h +17 -3

index 6b0414a..1349976 100644
@@ -35,13 +35,27 @@
 #define TST_KVERCMP_H__
 
 /*
- * Parse string kernel version into three integers.
- *
- * If version has only two numbers, i.e. 2.4 the third integer is set to zero.
+ * The same as tst_kvercmp() but running kernel version is passed as parameter
+ * instead of utilizing uname().
+ */
+int tst_kvcmp(const char *cur_kver, int r1, int r2, int r3);
+
+/*
+ * Parsers string into three integer version.
  */
 int tst_parse_kver(const char *str_kver, int *v1, int *v2, int *v3);
 
 /*
+ * Returns distribution name parsed from kernel version string or NULL.
+ */
+const char *tst_kvcmp_distname(const char *cur_kver);
+
+/*
+ * Compares versions up to five version numbers long.
+ */
+int tst_kvexcmp(const char *tst_exv, const char *cur_kver);
+
+/*
  * Compare given kernel version with currently running kernel.
  *
  * Returns negative if older, 0 if the smame and possitive if newer.

lib/tst_kvercmp.c +35 ~~-19~~

92 lines not on this page · raw message

testcases/lib/tst_kvcmp.c +45 ~~-12~~

96 lines not on this page · raw message

Signature · 1 line

2.10.2

Jan Stancek

2017-04-24 07:58 UTC #1 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

----- Original Message -----

Quoted 12 lines

This adds support for extra kernel versions to the tst_kvcmp shell helper.
Now we can append the extra version(s) after the generic kernel version in the expression. For example to check for older kernel than 3.0 or in case of RHEL6 older than 2.6.32 we do:
tst_kvcmp -lt '3.0 RHEL6:2.6.32'
This commit also reduces the number of uname() syscalls needed to acquire the result as a side efect.

Quoted 7 lines

Signed-off-by Cyril Hrubis <[email protected]>
CC Jan Stancek <[email protected]>
---
Jan can you please check that everything works fine when specifying extra version on RHEL? It seems that the only systems that are supported by extra kernel versions are RHEL5 and RHEL6 at the moment.

Should we print an error also if invalid distro is passed as parameter? I'd add 4th number to all examples: "RHEL6:2.6.32" -> "RHEL6:2.6.32-77", because all RHEL6 kernels are 2.6.32.

# uname -r
2.6.32-71.el6.x86_64
# ./tst_kvcmp -lt '3.11 RHEL6:2.6.32'; echo $?
1
# ./tst_kvcmp -lt '3.11 RHEL6:2.6.32-77'; echo $?
0

Otherwise it looks good to me.

Regards,
Jan

Cyril Hrubis

author

2017-04-26 15:06 UTC #2 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

Hi!

Quoted 5 lines

Quoted 3 lines
Jan can you please check that everything works fine when specifying extra version on RHEL? It seems that the only systems that are supported by extra kernel versions are RHEL5 and RHEL6 at the moment.
Should we print an error also if invalid distro is passed as parameter?

That would need a table of known distros stored into the tst_kvercmp.c and function to validate a distribution name, or do I miss something?

I guess that we can rewrite the code so that the kernel version substring and distro name are stored in an array of structures and loop over it so that we can say if it's valid or not but I'm not sure that it's worth the work.

Signature · 2 lines

Cyril Hrubis
[email protected]

Jan Stancek

2017-04-27 08:28 UTC #3 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

----- Original Message -----

Quoted 10 lines

Hi!
Quoted 6 lines
Quoted 4 lines
Jan can you please check that everything works fine when specifying extra version on RHEL? It seems that the only systems that are supported by extra
kernel versions are RHEL5 and RHEL6 at the moment.
Should we print an error also if invalid distro is passed as parameter?
That would need a table of known distros stored into the tst_kvercmp.c and function to validate a distribution name, or do I miss something?

+static int compare_kver(const char *cur_kver, char *kver) +{

+        const char *ver, *exver;
+        const char *distname = tst_kvcmp_distname(cur_kver);
+        int v1, v2, v3;

+
+ ver = strtok(kver, " ");
+
+ while ((exver = strtok(NULL, " "))) {
+ char *exkver = strchr(exver, ':');
+
+ if (!exkver) {
+ fprintf(stderr, "Invalid extra version '%s'\n", exver);
+ exit(2);
+ }
+
+ *(exkver++) = '\0';
+
+ if (!distname || strcmp(distname, exver))
+ continue;
+
+ return tst_kvexcmp(exkver, cur_kver);
+ }

If distname != NULL, that means that this is a distro we recognize. And if we reached here, after while loop, that means we didn't match any distro specified in "kver", correct?

So, is it OK to make conclusion, that there is a problem in kver?

if (distname)

    fprintf(stderr, "distname: %s didn't match any distro in kver: %s\n", distname, kver);

Regards,
Jan

Quoted 10 lines

I guess that we can rewrite the code so that the kernel version substring and distro name are stored in an array of structures and loop over it so that we can say if it's valid or not but I'm not sure that it's worth the work.
--
Cyril Hrubis
[email protected]

Cyril Hrubis

author

2017-04-27 13:01 UTC #4 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

Hi!

Quoted 24 lines

testcases/lib/tst_kvcmp.c +23 -0

+static int compare_kver(const char *cur_kver, char *kver)
+{
+        const char *ver, *exver;
+        const char *distname = tst_kvcmp_distname(cur_kver);
+        int v1, v2, v3;
+
+        ver = strtok(kver, " ");
+
+        while ((exver = strtok(NULL, " "))) {
+                char *exkver = strchr(exver, ':');
+
+                if (!exkver) {
+                        fprintf(stderr, "Invalid extra version '%s'\n", exver);
+                        exit(2);
+                }
+
+                *(exkver++) = '\0';
+
+                if (!distname || strcmp(distname, exver))
+                        continue;
+
+                return tst_kvexcmp(exkver, cur_kver);
+        }

Quoted 3 lines

If distname != NULL, that means that this is a distro we recognize. And if we reached here, after while loop, that means we didn't match any distro specified in "kver", correct?

Correct.

Quoted 1 line

So, is it OK to make conclusion, that there is a problem in kver?

Not at all. Even if we recognize the distro and the distname is non-NULL we can have any subset of distribution specific kernel versions (including empty one) in the input string. Hence if the loop does not mach any extra version against the currently detected distribution we simply fall back to the generic kernel version that should be at the start of the string.

Think about the distribution specific versions as of overrides that apply only if the distribution version is matched, otherwise we use the generic kernel version to make the decision.

The only thing that should probably be validated here are the distribution names passed in the kver string. As it is everything that is not matched is skipped including possible typos. Hence I was speaking about adding a function that would check if given distribution name is valid (known to the library code).

Signature · 2 lines

Cyril Hrubis
[email protected]

Jan Stancek

2017-04-27 13:21 UTC #5 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

----- Original Message -----

Quoted 41 lines

Hi!
Quoted 25 lines
testcases/lib/tst_kvcmp.c +13 -0
+static int compare_kver(const char *cur_kver, char *kver)
+{
+        const char *ver, *exver;
+        const char *distname = tst_kvcmp_distname(cur_kver);
+        int v1, v2, v3;
+
+        ver = strtok(kver, " ");
+
+        while ((exver = strtok(NULL, " "))) {
+                char *exkver = strchr(exver, ':');
+
+                if (!exkver) {
+                        fprintf(stderr, "Invalid extra version '%s'\n",
exver);
testcases/lib/tst_kvcmp.c +10 -0
+                        exit(2);
+                }
+
+                *(exkver++) = '\0';
+
+                if (!distname || strcmp(distname, exver))
+                        continue;
+
+                return tst_kvexcmp(exkver, cur_kver);
+        }
Quoted 3 lines
If distname != NULL, that means that this is a distro we recognize. And if we reached here, after while loop, that means we didn't match any distro specified in "kver", correct?
Correct.
Quoted 1 line
So, is it OK to make conclusion, that there is a problem in kver?
Not at all. Even if we recognize the distro and the distname is non-NULL we can have any subset of distribution specific kernel versions (including empty one) in the input string. Hence if the loop does not mach any extra version against the currently detected distribution we simply fall back to the generic kernel version that should be at the start of the string.

Quoted 3 lines

Think about the distribution specific versions as of overrides that apply only if the distribution version is matched, otherwise we use the generic kernel version to make the decision.

OK, I see where my reasoning failed.

Quoted 6 lines

The only thing that should probably be validated here are the distribution names passed in the kver string. As it is everything that is not matched is skipped including possible typos. Hence I was speaking about adding a function that would check if given distribution name is valid (known to the library code).

I'd put that on todo/wish list, since this extended version matching isn't widely used.

Regards,
Jan

Quoted 5 lines

--
Cyril Hrubis
[email protected]

Cyril Hrubis

author

2017-04-27 14:52 UTC #6 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

Hi!
I've pushed the original patch with added -77 to the example in the commit message.

Now this patch should fix the missing commands:

From da7a8f38d34659f4e02631c2e0a0f49e59edfbfa Mon Sep 17 00:00:00 2001
From: Cyril Hrubis <[email protected]>
Date: Thu, 27 Apr 2017 16:48:02 +0200
Subject: [PATCH] cpuset: Make use of tst_kvcmp

The tst_kvercmp2 was removed in:

commit 4847f1c13dabbfadd612aa812e2c99973f35f230

Author: Cyril Hrubis <[email protected]>
Date:   Mon Nov 14 13:44:45 2016 +0100

apicmd: Get rid of tst_kvercmp* binaries.

And these calls were not fixed beforehand.

Signed-off-by Cyril Hrubis <[email protected]>

---
 .../cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh          | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh +2 -4

index 63a9dc5..992b8f2 100755
@@ -128,8 +128,7 @@ test_cpus()
 		base_op_test "$CPUSET/1/cpus" "0,1-$((nr_cpus-2))," "0-$((nr_cpus-2))"
 	fi
 
-	tst_kvercmp2 3 0 0 "RHEL6:2.6.32"
-	if [ $? -eq 0 ]; then
+	if tst_kvcmp -lt "3.0 RHEL6:2.6.32"; then
 		base_op_test "$CPUSET/1/cpus" "0-" "WRITE_ERROR"
 	else
 		base_op_test "$CPUSET/1/cpus" "0-" "0"

@@ -164,8 +163,7 @@ test_mems()
 		base_op_test "$CPUSET/1/mems" "0,1-$((nr_mems-2))," "0-$((nr_mems-2))"
 	fi
 
-	tst_kvercmp2 3 0 0 "RHEL6:2.6.32"
-	if [ $? -eq 0 ]; then
+	if tst_kvcmp -lt "3.0 RHEL6:2.6.32"; then
 		base_op_test "$CPUSET/1/mems" "0-" "WRITE_ERROR"
 	else
 		base_op_test "$CPUSET/1/mems" "0-" "0"

Signature · 8 lines

2.10.2


OK to commit?

-- 
Cyril Hrubis
[email protected]

Jan Stancek

2017-04-28 06:58 UTC #7 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

----- Original Message -----

Quoted 59 lines

Hi!
I've pushed the original patch with added -77 to the example in the commit message.
Now this patch should fix the missing commands:
From da7a8f38d34659f4e02631c2e0a0f49e59edfbfa Mon Sep 17 00:00:00 2001
From: Cyril Hrubis <[email protected]>
Date: Thu, 27 Apr 2017 16:48:02 +0200
Subject: [PATCH] cpuset: Make use of tst_kvcmp
The tst_kvercmp2 was removed in:
commit 4847f1c13dabbfadd612aa812e2c99973f35f230
Author: Cyril Hrubis <[email protected]>
Date:   Mon Nov 14 13:44:45 2016 +0100
apicmd: Get rid of tst_kvercmp* binaries.
And these calls were not fixed beforehand.
Signed-off-by Cyril Hrubis <[email protected]>
---
 .../cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh          | 6
 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh index 63a9dc5..992b8f2 100755
---
a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
+++
b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh +2 -5
@@ -128,8 +128,7 @@ test_cpus()
 		base_op_test "$CPUSET/1/cpus" "0,1-$((nr_cpus-2))," "0-$((nr_cpus-2))"
 	fi
 
-	tst_kvercmp2 3 0 0 "RHEL6:2.6.32"
-	if [ $? -eq 0 ]; then
+	if tst_kvcmp -lt "3.0 RHEL6:2.6.32"; then
 		base_op_test "$CPUSET/1/cpus" "0-" "WRITE_ERROR"
 	else
 		base_op_test "$CPUSET/1/cpus" "0-" "0"
@@ -164,8 +163,7 @@ test_mems()
 		base_op_test "$CPUSET/1/mems" "0,1-$((nr_mems-2))," "0-$((nr_mems-2))"
 	fi
 
-	tst_kvercmp2 3 0 0 "RHEL6:2.6.32"
-	if [ $? -eq 0 ]; then
+	if tst_kvcmp -lt "3.0 RHEL6:2.6.32"; then
 		base_op_test "$CPUSET/1/mems" "0-" "WRITE_ERROR"
 	else
 		base_op_test "$CPUSET/1/mems" "0-" "0"
--
2.10.2

Quoted 1 line

OK to commit?

Yes.

Thanks,
Jan

Cyril Hrubis

author

2017-04-28 15:10 UTC #8 lore raw

RFC PATCH 1/1 tst_kvcmp: Add support for extra kernel versions

Hi!

Quoted 3 lines

Quoted 1 line
OK to commit?
Yes.

Pushed.

Signature · 2 lines

Cyril Hrubis
[email protected]