Merge ~ahasenack/ubuntu/+source/samba:groovy-samba-dep8-uring-old-kernel into ubuntu/+source/samba:ubuntu/devel

[Andreas Hasenack]

Skip uring test if the running kernel is too old.

This happens in our testers for armhf, which runs inside a lxd container on a bionic host. The bionic kernel does not have support for uring, and since the lxd container sees the host kernel, even though the container itself is groovy, the test fails.

Bileto ticket: https://bileto.ubuntu.com/#/ticket/4193

If you go to the armhf test, and search for "kernel", you will see it's 4.15.something. Scroll down to the bottom and you will see the uring tests skipped. The cifs ones because it's lxd and not a vm, and the smbclient uring one because of the old kernel.

[Andreas Hasenack (ahasenack)]

Bileto is green. i386 is the usual red, and gvfs on s390x always needs a couple of retries.

[Lucas Kanashiro (lucaskanashiro)]

I am on it.

[Lucas Kanashiro (lucaskanashiro)]

I added an inline comment with a suggestion to the check_kernel_version function.

review: Needs Information

[Andreas Hasenack (ahasenack)]

Replied

[Andreas Hasenack (ahasenack)]

Hm, looks like my 6.0.2 example doesn't pass

[Andreas Hasenack (ahasenack)]

How about:

    # uring is supported starting with kernel 5.1.x
    if [ ${k_major} -lt 5 ]; then
        return 1
    elif [ ${k_major} -eq 5 ] && [ ${k_minor} -ge 1 ]; then
       return 0
    elif [ ${k_major} -ge 6 ]; then
       return 0
    else
        return 1
    fi

Example test script:
#!/bin/bash

check_kernel_version() {
    local k_ver=$1
    local k_major=$(echo ${k_ver} | cut -d . -f 1)
    local k_minor=$(echo ${k_ver} | cut -d . -f 2)

    # uring is supported starting with kernel 5.1.x
    if [ ${k_major} -lt 5 ]; then
        return 1
    elif [ ${k_major} -eq 5 ] && [ ${k_minor} -ge 1 ]; then
       return 0
    elif [ ${k_major} -ge 6 ]; then
       return 0
    else
        return 1
    fi
}

for k_ver in 5.10.0 5.4.0 5.1.0 4.10.0 4.4.0 6.0.2; do
    result=0
    echo -n "Checking kernel ${k_ver}... "
    check_kernel_version ${k_ver} || result=$?
    if [ ${result} -ne 0 ]; then
        echo "FAIL"
    else
        echo "OK"
    fi
done

$ ./check-kernel.sh
Checking kernel 5.10.0... OK
Checking kernel 5.4.0... OK
Checking kernel 5.1.0... OK
Checking kernel 4.10.0... FAIL
Checking kernel 4.4.0... FAIL
Checking kernel 6.0.2... OK

[Lucas Kanashiro (lucaskanashiro)]

In your latest implementation the first if statement is still needed? I did this small change to your test script and I got the same result:

- if [ ${k_major} -lt 5 ]; then
- return 1
- elif [ ${k_major} -eq 5 ] && [ ${k_minor} -ge 1 ]; then
+ if [ ${k_major} -eq 5 ] && [ ${k_minor} -ge 1 ]; then

$ ./check-kernel.sh
Checking kernel 5.10.0... OK
Checking kernel 5.4.0... OK
Checking kernel 5.1.0... OK
Checking kernel 4.10.0... FAIL
Checking kernel 4.4.0... FAIL
Checking kernel 6.0.2... OK

[Andreas Hasenack (ahasenack)]

I added 5.0.0 to the test script to be sure:
#!/bin/bash

check_kernel_version() {
    local k_ver=$1
    local k_major=$(echo ${k_ver} | cut -d . -f 1)
    local k_minor=$(echo ${k_ver} | cut -d . -f 2)

    # uring is supported starting with kernel 5.1.x
    if [ ${k_major} -eq 5 ] && [ ${k_minor} -ge 1 ]; then
       return 0
    elif [ ${k_major} -ge 6 ]; then
       return 0
    else
       return 1
    fi
}

for k_ver in 5.0.0 5.10.0 5.4.0 5.1.0 4.10.0 4.4.0 6.0.2 5.2.0; do
    result=0
    echo -n "Checking kernel ${k_ver}... "
    check_kernel_version ${k_ver} || result=$?
    if [ ${result} -ne 0 ]; then
        echo "FAIL"
    else
        echo "OK"
    fi
done

$ ./check-kernel.sh
Checking kernel 5.0.0... FAIL
Checking kernel 5.10.0... OK
Checking kernel 5.4.0... OK
Checking kernel 5.1.0... OK
Checking kernel 4.10.0... FAIL
Checking kernel 4.4.0... FAIL
Checking kernel 6.0.2... OK
Checking kernel 5.2.0... OK

[Lucas Kanashiro (lucaskanashiro)]

That's the expected behavior, right? 5.0.0 does not support uring according to your comment in the code, >= 5.1.0 is what we want.

[Andreas Hasenack (ahasenack)]

Yes, sorry, I meant to say it works :)

[Andreas Hasenack (ahasenack)]

Pushed your suggestion

[Lucas Kanashiro (lucaskanashiro)]

Thanks Andreas! LGTM, +1.

Do not forget to drop the '~ppa1' from the version string ;-)

review: Approve

[Andreas Hasenack (ahasenack)]

Good call :)

Squashed and force pushed

[Andreas Hasenack (ahasenack)]

Tagging and uploading 124bdc164f3e78e61abdcd44f2e9f42afb176a97

$ git push pkg upload/2%4.12.5+dfsg-3ubuntu3
Enumerating objects: 18, done.
Counting objects: 100% (18/18), done.
Delta compression using up to 4 threads
Compressing objects: 100% (12/12), done.
Writing objects: 100% (12/12), 1.78 KiB | 365.00 KiB/s, done.
Total 12 (delta 9), reused 0 (delta 0)
remote: Checking connectivity: 12, done.
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/samba
 * [new tag] upload/2%4.12.5+dfsg-3ubuntu3 -> upload/2%4.12.5+dfsg-3ubuntu3

[Andreas Hasenack (ahasenack)]

$ dput ubuntu ../samba_4.12.5+dfsg-3ubuntu3_source.changes
Checking signature on .changes
gpg: ../samba_4.12.5+dfsg-3ubuntu3_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../samba_4.12.5+dfsg-3ubuntu3.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading samba_4.12.5+dfsg-3ubuntu3.dsc: done.
  Uploading samba_4.12.5+dfsg-3ubuntu3.debian.tar.xz: done.
  Uploading samba_4.12.5+dfsg-3ubuntu3_source.buildinfo: done.
  Uploading samba_4.12.5+dfsg-3ubuntu3_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 406edda..fd28297 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+samba (2:4.12.5+dfsg-3ubuntu3) groovy; urgency=medium
+
+  * d/t/{util, smbclient-share-access-uring, cifs-share-access-uring}:
+    guard uring tests with a kernel version check and skip if it's too old
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 11 Aug 2020 11:00:35 -0300
+
 samba (2:4.12.5+dfsg-3ubuntu2) groovy; urgency=medium
 
   * d/t/smbclient-anonymous-share-list: add set -x and set -e
diff --git a/debian/tests/cifs-share-access-uring b/debian/tests/cifs-share-access-uring
index 6bbd3e1..22253ca 100644
--- a/debian/tests/cifs-share-access-uring
+++ b/debian/tests/cifs-share-access-uring
@@ -3,13 +3,19 @@
 set -x
 set -e
 
+. debian/tests/util
+
 ARCH=$(dpkg --print-architecture)
 if [ "$ARCH" = "i386" ]; then
     echo "liburing not available on $ARCH, skipping test"
     exit 77
 fi
 
-. debian/tests/util
+k_ver=$(uname -r | cut -d - -f 1)
+if ! check_kernel_version ${k_ver}; then
+    echo "uring not available in kernel version ${k_ver}, skipping test"
+    exit 77
+fi
 
 username="smbtest$$"
 password="$$"
diff --git a/debian/tests/smbclient-share-access-uring b/debian/tests/smbclient-share-access-uring
index e6dc017..e7915c3 100644
--- a/debian/tests/smbclient-share-access-uring
+++ b/debian/tests/smbclient-share-access-uring
@@ -3,13 +3,19 @@
 set -x
 set -e
 
+. debian/tests/util
+
 ARCH=$(dpkg --print-architecture)
 if [ "$ARCH" = "i386" ]; then
     echo "liburing not available on $ARCH, skipping test"
     exit 77
 fi
 
-. debian/tests/util
+k_ver=$(uname -r | cut -d - -f 1)
+if ! check_kernel_version ${k_ver}; then
+    echo "uring not available in kernel version ${k_ver}, skipping test"
+    exit 77
+fi
 
 username="smbtest$$"
 password="$$"
diff --git a/debian/tests/util b/debian/tests/util
index 8c5d452..47d58b9 100644
--- a/debian/tests/util
+++ b/debian/tests/util
@@ -47,3 +47,19 @@ populate_share() {
     chown -R "${usergroup}:${usergroup}" "${sharepath}"
 }
 
+
+# $1: kernel version in the form major.minor.patch
+check_kernel_version() {
+    local k_ver=$1
+    local k_major=$(echo ${k_ver} | cut -d . -f 1)
+    local k_minor=$(echo ${k_ver} | cut -d . -f 2)
+
+    # uring is supported starting with kernel 5.1.x
+    if [ ${k_major} -eq 5 ] && [ ${k_minor} -ge 1 ]; then
+       return 0
+    elif [ ${k_major} -ge 6 ]; then
+       return 0
+    else
+        return 1
+    fi
+}