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

[Andreas Hasenack]

Enable uring support in samba. This builds a new vfs module, which can then optionally be enabled.

uring support first appeared in version 4.12.0. Search for "io_uring": https://www.samba.org/samba/history/samba-4.12.0.html

I also added DEP8 tests for it, basically the same tests we already had, but now on a share with this vfs module loaded. I took the opportunity to refactor these tests a bit, to avoid duplication of code. I kept the commits separate to make it easier to submit them to debian: a) the refactor; b) the new uring changes

If you are happy with these, I will then submit to debian before uploading.

Bileto, still running: https://bileto.ubuntu.com/#/ticket/4179

I did run the tests locally before, both in a VM and in LXD, and with proposed enabled, so I don't expect trouble. Bileto is slow because of the risc64 build.

[Lucas Kanashiro (lucaskanashiro)]

I am going to review this MP.

[Andreas Hasenack (ahasenack)]

Forgot to add skippable to the uring test restriction. Committed, force-pushed.

[Lucas Kanashiro (lucaskanashiro)]

I added an inline comment to suggest a refactoring change.

[Andreas Hasenack (ahasenack)]

I pushed new commits with that extra refactoring, and am waiting for the ppa to build now (due to risc64 bileto tickets are sloooooooooooooooooooooooooooooooooooooooooooooooooow)

[Andreas Hasenack (ahasenack)]

Bileto is mostly done again, and green-ish (the usual i386 and gfvs)

[Lucas Kanashiro (lucaskanashiro)]

* Changelog:
  - [ ] old content and logical tag match as expected
  - [ ] changelog entry correct version and targeted codename
  - [ ] changelog entries correct
  - [ ] update-maintainer has been run

* Actual changes:
  - [ ] no upstream changes to consider
  - [ ] no further upstream version to consider
  - [ ] debian changes look safe

* Old Delta:
  - [-] dropped changes are ok to be dropped
  - [√] nothing else to drop
  - [-] changes forwarded upstream/debian (if appropriate)

* New Delta:
  - [√] no new patches added
  - [-] patches match what was proposed upstream
  - [-] patches correctly included in debian/patches/series
  - [-] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok
  - [√] verified PPA package installs/uninstalls
  - [√] autopkgtest against the PPA package passes
  - [√] sanity checks test fine

LGTM, +1.

Thanks for implementing my suggestion Andreas. I believe it's ready to be submitted to Debian and uploaded to Ubuntu in the meantime.

review: Approve

[Lucas Kanashiro (lucaskanashiro)]

Sorry, forgot to fill all the fields in my checklist but it's all right :)

[Andreas Hasenack (ahasenack)]

Thanks, tagging and uploading bf2783b307b4ff5b14e9acc995b8c71aef629872

$ git push pkg upload/2%4.12.5+dfsg-3ubuntu2
Enumerating objects: 39, done.
Counting objects: 100% (39/39), done.
Delta compression using up to 4 threads
Compressing objects: 100% (23/23), done.
Writing objects: 100% (29/29), 4.50 KiB | 1.12 MiB/s, done.
Total 29 (delta 18), reused 14 (delta 6)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/samba
 * [new tag] upload/2%4.12.5+dfsg-3ubuntu2 -> upload/2%4.12.5+dfsg-3ubuntu2

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

I'll propose to debian after it migates

diff --git a/debian/changelog b/debian/changelog
index 019552e..406edda 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,23 @@
+samba (2:4.12.5+dfsg-3ubuntu2) groovy; urgency=medium
+
+  * d/t/smbclient-anonymous-share-list: add set -x and set -e
+  * Factor out common DEP8 test code into d/t/util and change the tests
+    to source from it:
+    - d/t/util: added
+    - d/t/cifs-share-access, d/t/smbclient-share-access: source from
+      util, use random share name and add set -x and set -u
+    - d/t/smbclient-authenticated-share-list: source from util and add
+      set -x and set -u
+  * d/control: enable the liburing vfs module, except on i386 where
+    liburing is not available
+  * Add new DEP8 tests for the uring vfs module:
+    - d/t/control: add smbclient-share-access-uring and
+      cifs-share-access-uring tests
+    - d/t/smbclient-share-access-uring: new test
+    - d/t/cifs-share-access-uring: new test
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 04 Aug 2020 17:20:30 -0300
+
 samba (2:4.12.5+dfsg-3ubuntu1) groovy; urgency=medium
 
   * Merge with Debian unstable. Remaining changes:
diff --git a/debian/control b/debian/control
index 790dc6a..088a9bb 100644
--- a/debian/control
+++ b/debian/control
@@ -45,6 +45,7 @@ Build-Depends: bison,
                libtasn1-bin,
                libtdb-dev (>= 1.4.3~),
                libtevent-dev (>= 0.10.2~),
+               liburing-dev [!i386],
                perl,
                pkg-config,
                po-debconf,
diff --git a/debian/tests/cifs-share-access b/debian/tests/cifs-share-access
index acfb70a..47ce01a 100644
--- a/debian/tests/cifs-share-access
+++ b/debian/tests/cifs-share-access
@@ -1,34 +1,23 @@
-#!/bin/sh -x
+#!/bin/sh
 
-if ! testparm -s 2>&1 | grep -qE "^\[homes\]"; then
-    echo "Adding [homes] share"
-    cat >> /etc/samba/smb.conf <<EOFEOF
-[homes]
-  valid users = %S
-  read only = no
-  guest ok = no
-EOFEOF
-    systemctl reload smbd.service
-else
-    echo "No need to add [homes] share, continuing."
-fi
+set -x
+set -e
+
+. debian/tests/util
 
 username="smbtest$$"
 password="$$"
-echo "Creating a local test user called ${username}"
-useradd -m "$username"
-echo "Setting samba password for the ${username} user"
-echo "${password}\n${password}" | smbpasswd -s -a ${username}
-userhome=$(eval echo ~$username)
+add_user "${username}" "${password}"
+
+myshare="myshare$$"
+add_share "${myshare}"
+
 echo "Creating file with random data and computing its md5"
-dd if=/dev/urandom bs=1 count=128 2>/dev/null | base64 > ${userhome}/data
-chown ${username}:${username} ${userhome}/data
-cd ${userhome}
-md5sum data > data.md5
+populate_share "${myshare}" "${username}"
 
-echo "Mounting //localhost/${username} via CIFS"
+echo "Mounting //localhost/${myshare} via CIFS"
 temp_mount=$(mktemp -d)
-mount -t cifs //localhost/${username} "$temp_mount" -o user=${username},username=${username},password=${password}
+mount -t cifs //localhost/"${myshare}" "$temp_mount" -o user="${username}",username="${username}",password="${password}"
 
 echo "Verifying MD5 via cifs"
 cd "$temp_mount"
diff --git a/debian/tests/cifs-share-access-uring b/debian/tests/cifs-share-access-uring
 new file mode 100644
index 0000000..6bbd3e1
--- /dev/null
+++ b/debian/tests/cifs-share-access-uring
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+set -x
+set -e
+
+ARCH=$(dpkg --print-architecture)
+if [ "$ARCH" = "i386" ]; then
+    echo "liburing not available on $ARCH, skipping test"
+    exit 77
+fi
+
+. debian/tests/util
+
+username="smbtest$$"
+password="$$"
+add_user "${username}" "${password}"
+
+myshare="myshare$$"
+add_share "${myshare}" io_uring
+
+echo "Creating file with random data and computing its md5"
+populate_share "${myshare}" "${username}"
+
+echo "Mounting //localhost/${myshare} via CIFS"
+temp_mount=$(mktemp -d)
+mount -t cifs //localhost/"${myshare}" "$temp_mount" -o user="${username}",username="${username}",password="${password}"
+
+echo "Verifying MD5 via cifs"
+cd "$temp_mount"
+md5sum -c data.md5
+result=$?
+cd -
+umount "$temp_mount"
+rmdir "$temp_mount"
+exit "$result"
diff --git a/debian/tests/control b/debian/tests/control
index 2ebb31c..3ecb853 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -2,6 +2,10 @@ Tests: cifs-share-access
 Depends: samba, coreutils, systemd, cifs-utils, passwd
 Restrictions: needs-root, allow-stderr, isolation-machine
 
+Tests: cifs-share-access-uring
+Depends: samba, samba-vfs-modules, coreutils, systemd, cifs-utils, passwd
+Restrictions: needs-root, allow-stderr, isolation-machine, skippable
+
 Tests: python-smoke
 Depends: python3-samba
 
@@ -16,3 +20,7 @@ Restrictions: needs-root, allow-stderr, isolation-container
 Tests: smbclient-share-access
 Depends: samba, smbclient, coreutils, systemd, passwd
 Restrictions: needs-root, allow-stderr, isolation-container
+
+Tests: smbclient-share-access-uring
+Depends: samba, samba-vfs-modules, smbclient, coreutils, systemd, passwd
+Restrictions: needs-root, allow-stderr, isolation-container, skippable
diff --git a/debian/tests/smbclient-anonymous-share-list b/debian/tests/smbclient-anonymous-share-list
index a503ae7..e71ef27 100644
--- a/debian/tests/smbclient-anonymous-share-list
+++ b/debian/tests/smbclient-anonymous-share-list
@@ -1,3 +1,6 @@
-#!/bin/sh -x
+#!/bin/sh
+
+set -x
+set -e
 
 smbclient -N -L localhost
diff --git a/debian/tests/smbclient-authenticated-share-list b/debian/tests/smbclient-authenticated-share-list
index 71f0994..9fcaf43 100644
--- a/debian/tests/smbclient-authenticated-share-list
+++ b/debian/tests/smbclient-authenticated-share-list
@@ -1,17 +1,18 @@
-#!/bin/sh -x
+#!/bin/sh
+
+set -x
+set -e
+
+. debian/tests/util
 
 username="smbtest$$"
 password="$$"
 
-echo "Creating a local test user called ${username}"
-useradd -m "$username"
-
-echo "Setting samba password for the ${username} user"
-echo "${password}\n${password}" | smbpasswd -s -a ${username}
+add_user "${username}" "${password}"
 
 echo "Testing with incorrect password: must fail"
-smbclient -L localhost -U ${username}%wrongpass && exit 1
+smbclient -L localhost -U "${username}"%wrongpass && exit 1
 
 echo "Testing with correct password: must work"
-smbclient -L localhost -U ${username}%${password}
+smbclient -L localhost -U "${username}"%"${password}"
 
diff --git a/debian/tests/smbclient-share-access b/debian/tests/smbclient-share-access
index 22546f3..b124c0c 100644
--- a/debian/tests/smbclient-share-access
+++ b/debian/tests/smbclient-share-access
@@ -1,34 +1,23 @@
-#!/bin/sh -x
+#!/bin/sh
 
-if ! testparm -s 2>&1 | grep -qE "^\[homes\]"; then
-    echo "Adding [homes] share"
-    cat >> /etc/samba/smb.conf <<EOFEOF
-[homes]
-  valid users = %S
-  read only = no
-  guest ok = no
-EOFEOF
-    systemctl reload smbd.service
-else
-    echo "No need to add [homes] share, continuing."
-fi
+set -x
+set -e
+
+. debian/tests/util
 
 username="smbtest$$"
 password="$$"
-echo "Creating a local test user called ${username}"
-useradd -m "$username"
-echo "Setting samba password for the ${username} user"
-echo "${password}\n${password}" | smbpasswd -s -a ${username}
-userhome=$(eval echo ~$username)
+add_user "${username}" "${password}"
+
+myshare="myshare$$"
+add_share "${myshare}"
+
 echo "Creating file with random data and computing its md5"
-dd if=/dev/urandom bs=1 count=128 2>/dev/null | base64 > ${userhome}/data
-chown ${username}:${username} ${userhome}/data
-cd ${userhome}
-md5sum data > data.md5
+populate_share "${myshare}" "${username}"
 
 rm -f downloaded-data
 echo "Downloading file and comparing its md5"
-smbclient //localhost/${username} -U ${username}%${password} -c "get data downloaded-data"
+smbclient //localhost/"${myshare}" -U "${username}"%"${password}" -c "get data downloaded-data"
 
 mv -f downloaded-data data
 md5sum -c data.md5
diff --git a/debian/tests/smbclient-share-access-uring b/debian/tests/smbclient-share-access-uring
 new file mode 100644
index 0000000..e6dc017
--- /dev/null
+++ b/debian/tests/smbclient-share-access-uring
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+set -x
+set -e
+
+ARCH=$(dpkg --print-architecture)
+if [ "$ARCH" = "i386" ]; then
+    echo "liburing not available on $ARCH, skipping test"
+    exit 77
+fi
+
+. debian/tests/util
+
+username="smbtest$$"
+password="$$"
+add_user "${username}" "${password}"
+
+myshare="myshare$$"
+add_share "${myshare}" io_uring
+
+echo "Creating file with random data and computing its md5"
+populate_share "${myshare}" "${username}"
+
+rm -f downloaded-data
+echo "Downloading file and comparing its md5"
+smbclient //localhost/"${myshare}" -U "${username}"%"${password}" -c "get data downloaded-data"
+
+mv -f downloaded-data data
+md5sum -c data.md5
diff --git a/debian/tests/util b/debian/tests/util
 new file mode 100644
index 0000000..8c5d452
--- /dev/null
+++ b/debian/tests/util
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+# $1: share name
+# $2: comma separated list of vfs_objects to use, if any
+add_share() {
+    local share="$1"
+    local vfs="$2"
+    if ! testparm -s 2>&1 | grep -E "^\[${share}\]"; then
+        echo "Adding [${share}] share"
+        cat >> /etc/samba/smb.conf <<EOFEOF
+[${share}]
+  read only = no
+  guest ok = no
+  path = /${share}
+EOFEOF
+        if [ -n "${vfs}" ]; then
+            echo "vfs objects = ${vfs}" >> /etc/samba/smb.conf
+        fi
+        systemctl restart smbd.service
+    else
+        echo "Share [${share}] already exists, continuing"
+    fi
+}
+
+# $1: username
+# $2: password
+add_user() {
+    local username="$1"
+    local password="$2"
+
+    echo "Creating a local and samba user called ${username}"
+    useradd -m "${username}"
+    echo "Setting samba password for the ${username} user"
+    echo "${password}\n${password}" | smbpasswd -s -a ${username}
+}
+
+# $1: share name
+populate_share() {
+    local sharename="$1"
+    local usergroup="$2"
+    local sharepath="/${sharename}"
+
+    mkdir -p "${sharepath}"
+    dd if=/dev/urandom bs=4096 count=1000 2>/dev/null | base64 > "${sharepath}/data"
+    cd "${sharepath}"
+    md5sum data > data.md5
+    chown -R "${usergroup}:${usergroup}" "${sharepath}"
+}
+