Merge ~paelzer/ubuntu/+source/libvirt-dbus:lp-1802005-allow-libvirtdbus-to-access-libvirt-GROOVY into ~paelzer/ubuntu/+source/libvirt-dbus:current-groovy

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

Set back to WIP for now ...

[Christian Ehrhardt  (paelzer)]

PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4284

[Christian Ehrhardt  (paelzer)]

*sigh* test errors (unrelated to my change) that didn't happen before stall this ...

[Christian Ehrhardt  (paelzer)]

After some patches from upstream, a discussion there, and a follow on patch.
Now builds fine.

Ready for review

[Paride Legovini (paride)]

LGTM, I just spotted a typo in a comment (see inline comment).

Builds fine locally and it's lintian clean. The "usermod -a -G" bit looks good, I'm surprised by the low number of packages needing to run usermod that way in postinst, but I couldn't find a different way of doing it (e.g. directly via dh_sysusers or similar).

More worrisome is the issue with gulong on armhf: tests-vcpupininfo-32bit.patch band-aids the bug but the underlying issue is likely to still show, as gulongs are used elsewhere in the code. This is however clearly stated in the patch description.

review: Approve

[Christian Ehrhardt  (paelzer)]

complete

diff --git a/debian/changelog b/debian/changelog
index 6650114..f61e3fe 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+libvirt-dbus (1.4.0-1ubuntu1) groovy; urgency=medium
+
+  * d/postinst: add libvirtdbus to group libvirt to be able to access the
+    sockets of libvirtd (LP: #1802005)
+  * Fix FTFBS due to libvirt 6.6
+    - d/p/tests-skip-CPU-pinning-test-on-libvirt-6.6.0.patch
+    - d/p/tests-vcpupininfo-use-pytest.skip.patch
+    - d/p/tests-vcpupininfo-32bit.patch: fix tests on armhf
+
+ -- Christian Ehrhardt <[e-mail]>  Tue, 29 Sep 2020 15:16:03 +0200
+
 libvirt-dbus (1.4.0-1) unstable; urgency=medium
 
   * [47a48e5] New upstream version 1.4.0
diff --git a/debian/control b/debian/control
index 1d41361..a51385a 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: libvirt-dbus
 Section: admin
 Priority: optional
-Maintainer: Debian Libvirt Maintainers <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian Libvirt Maintainers <[e-mail]>
 Uploaders: Andrea Bolognani <[e-mail]>
 Homepage: https://libvirt.org/
 Vcs-Git: https://salsa.debian.org/libvirt-team/libvirt-dbus.git
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..16f1b67
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1,3 @@
+tests-skip-CPU-pinning-test-on-libvirt-6.6.0.patch
+tests-vcpupininfo-use-pytest.skip.patch
+tests-vcpupininfo-32bit.patch
diff --git a/debian/patches/tests-skip-CPU-pinning-test-on-libvirt-6.6.0.patch b/debian/patches/tests-skip-CPU-pinning-test-on-libvirt-6.6.0.patch
new file mode 100644
index 0000000..cbc496d
--- /dev/null
+++ b/debian/patches/tests-skip-CPU-pinning-test-on-libvirt-6.6.0.patch
@@ -0,0 +1,41 @@
+From e01a3cf5e60fa8f4d3bf9a805bfedbe5d5fa1507 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <[e-mail]>
+Date: Tue, 22 Sep 2020 16:16:02 +0100
+Subject: [PATCH] tests: skip CPU pinning test on libvirt 6.6.0
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+This release broke the test driver making it depend on host CPU topology
+instead of the fake CPU topology.
+
+Signed-off-by: Daniel P. Berrangé <[e-mail]>
+
+Origin: upstream, https://gitlab.com/libvirt/libvirt-dbus/-/commit/e01a3cf5e6
+Last-Update: 2020-10-01
+
+---
+ tests/test_domain.py | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/tests/test_domain.py b/tests/test_domain.py
+index 624d25b..7f67a3a 100755
+--- a/tests/test_domain.py
++++ b/tests/test_domain.py
+@@ -153,6 +153,13 @@ class TestDomain(libvirttest.BaseTestClass):
+         assert domain.GetVcpus(0) == dbus.Int32(vcpus_expected)
+ 
+     def test_domain_vcpu_pin_info(self):
++        # Test driver was broken in 6.6.0 accidentally depending
++        # on host CPU topology instead of its fake topology
++        obj = self.bus.get_object('org.libvirt', '/org/libvirt/Test')
++        ver = obj.Get('org.libvirt.Connect', "LibVersion", dbus_interface=dbus.PROPERTIES_IFACE)
++        if ver == 6006000:
++            return
++
+         obj, domain = self.get_test_domain()
+         pinInfo_expected = [
+             [True, True, True, True, True, True, True, True],
+-- 
+2.28.0
+
diff --git a/debian/patches/tests-vcpupininfo-32bit.patch b/debian/patches/tests-vcpupininfo-32bit.patch
new file mode 100644
index 0000000..74644aa
--- /dev/null
+++ b/debian/patches/tests-vcpupininfo-32bit.patch
@@ -0,0 +1,36 @@
+Description: fix tests on 32bit armhf
+ There seems to be an underlying issue with gulong variables, for now mask
+ the value in the test to get the correct 32bit result that it checks on.
+ I forwarded the case in below IRC discussion, but the fix is only a band aid
+ for our builds until the underlying issue is fixed.
+ IRC snippet:
+ <cpaelzer> danp: jtomko: but I wonder about the result - it works on all
+  architectures but armhf for me - the version that is returned is odd
+ <cpaelzer> it is 17830927092969874672
+ <jtomko> cpaelzer: the lower 32 bits of the number you posted are 6006000
+ <cpaelzer> hmm, so we could need a mask then on armhf
+ <jtomko> virtDBusConnectGetLibVersion sets "t" as the type (u64) but passes
+   a gulong there
+ <cpaelzer> the type auto-set for var is <class 'dbus.UInt64'>
+ <jtomko> cpaelzer: it might not be the only libvirt-dbus API broken on 32-bit.
+  a quick 'git grep gulong' shows we use it even for APIs that take long long
+  sometimes
+ <cpaelzer> jtomko: just for confirmation ver &= 0xFFFFFFFF indeed "fixes" the
+  issue
+ <cpaelzer> but yes there might be a bigger underlying issue in regard to the
+  gulong usage
+Forwarded: IRC
+Author: Christian Ehrhardt <[e-mail]>
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=971171
+Last-Update: 2019-06-30
+--- a/tests/test_domain.py
++++ b/tests/test_domain.py
+@@ -158,6 +158,8 @@ class TestDomain(libvirttest.BaseTestCla
+         # on host CPU topology instead of its fake topology
+         obj = self.bus.get_object('org.libvirt', '/org/libvirt/Test')
+         ver = obj.Get('org.libvirt.Connect', "LibVersion", dbus_interface=dbus.PROPERTIES_IFACE)
++        # There is a problem on 32bit architectures, for now mask this value
++        ver &= 0xFFFFFFFF
+         if ver == 6006000:
+             pytest.skip("CPU toploogy is broken in libvirt 6.6.0 test driver")
+ 
diff --git a/debian/patches/tests-vcpupininfo-use-pytest.skip.patch b/debian/patches/tests-vcpupininfo-use-pytest.skip.patch
new file mode 100644
index 0000000..720384a
--- /dev/null
+++ b/debian/patches/tests-vcpupininfo-use-pytest.skip.patch
@@ -0,0 +1,43 @@
+From e19bc98ff7f653dca7c38e0aeec5754a74141e16 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?J=C3=A1n=20Tomko?= <[e-mail]>
+Date: Thu, 24 Sep 2020 15:10:27 +0200
+Subject: [PATCH] tests: vcpupininfo: use pytest.skip()
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Mark the test as skipped instead of quietly passing.
+
+Signed-off-by: Ján Tomko <[e-mail]>
+
+Origin: upstream, https://gitlab.com/libvirt/libvirt-dbus/-/commit/e19bc98ff7
+Last-Update: 2020-10-01
+
+---
+ tests/test_domain.py | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/tests/test_domain.py b/tests/test_domain.py
+index 7f67a3a..605fb43 100755
+--- a/tests/test_domain.py
++++ b/tests/test_domain.py
+@@ -2,6 +2,7 @@
+ 
+ import dbus
+ import libvirttest
++import pytest
+ import xmldata
+ 
+ DBUS_EXCEPTION_MISSING_FUNCTION = 'this function is not supported by the connection driver'
+@@ -158,7 +159,7 @@ class TestDomain(libvirttest.BaseTestClass):
+         obj = self.bus.get_object('org.libvirt', '/org/libvirt/Test')
+         ver = obj.Get('org.libvirt.Connect', "LibVersion", dbus_interface=dbus.PROPERTIES_IFACE)
+         if ver == 6006000:
+-            return
++            pytest.skip("CPU toploogy is broken in libvirt 6.6.0 test driver")
+ 
+         obj, domain = self.get_test_domain()
+         pinInfo_expected = [
+-- 
+2.28.0
+
diff --git a/debian/postinst b/debian/postinst
new file mode 100644
index 0000000..4bdfa84
--- /dev/null
+++ b/debian/postinst
@@ -0,0 +1,53 @@
+#!/bin/sh
+# postinst script for libvirt-dbus
+#
+# see: dh_installdeb(1)
+
+set -e
+
+# summary of how this script can be called:
+#        * <postinst> `configure' <most-recently-configured-version>
+#        * <old-postinst> `abort-upgrade' <new version>
+#        * <conflictor's-postinst> `abort-remove' `in-favour' <package>
+#          <new-version>
+#        * <postinst> `abort-remove'
+#        * <deconfigured's-postinst> `abort-deconfigure' `in-favour'
+#          <failed-install-package> <version> `removing'
+#          <conflicting-package> <version>
+# for details, see https://www.debian.org/doc/debian-policy/ or
+# the debian-policy package
+
+
+case "$1" in
+    configure)
+    ;;
+
+    abort-upgrade|abort-remove|abort-deconfigure)
+    ;;
+
+    *)
+        echo "postinst called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+#DEBHELPER#
+
+case "$1" in
+    configure)
+        # after users are created by dh_sysuser: add user to libvirt group
+        # if policykit isn't used to manae access to libvirt sockets then it
+        # will use group based permission management with the group "libvirt"
+        # being the one to gain access.
+        # libvirt-dbus is safe via /usr/share/dbus-1/system.d/org.libvirt.conf
+        # so we can allow that by default
+        if getent group libvirt >/dev/null; then
+            usermod -a -G libvirt libvirtdbus
+        fi
+    ;;
+esac
+
+exit 0