Merge ~paelzer/ubuntu/+source/qemu:lp-1913421-exec-run-FOCAL into ubuntu/+source/qemu:ubuntu/focal-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

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

Please review these MPs in depth as the maintscript and edge-case magic has bitten me enough on this :-/

[Dan Streetman (ddstreet)]

abstaining from review per bug 1913421 comment 41

review: Abstain

[Christian Ehrhardt  (paelzer)]

The discussion between Dan and me continued on the bug, I'd ask (and therefore assign for pre-unapproved-review) the SRU-Team to help us by adding a third opinion.

Dan and I are not even battling with strong opposing opinions - there are just multiple ways to do things and it isn't clear how to best-continue so we'd need a tie breaker.

I'd ask/recommend to read comment 41 and all that followed on the bug, if needed to go deeper also the rest of the bug comments and the related Debian PR.
Then adding a 3rd voice/opinion here would really help us.

3ea58ce... by Christian Ehrhardt  on 2021-09-09

d/qemu-block-extra.prerm.in: test for exec and prepare /var/run/qemu if needed

/run is by default mounted as noexec which breaks the use case of
/var/run/qemu - therefore if not already executable (via e.g. an admin that
has taken care) prepare a mountpoint that suffices for the intended use.

For SRUs this was considered less regression risk than the badly dh*
handled mount unit that is used in >=21.10 releases.

Signed-off-by: Christian Ehrhardt <email address hidden>

[Christian Ehrhardt  (paelzer)]

Re-worked according to the bug discussion - thanks to everyone that participated!
Thereby ready for re-review.

[Utkarsh Gupta (utkarsh)]

4d7e39d261ce5a6372b08d1b0bc841d739d96f3f
-> okay, not much to comment there except the commit
   message has a typo; one of the "postrm" should be "prerm".
   (the commit message typo also resulted in a typo in d/ch)

d0cbddde8451168ebeb5d2cfe051f6b198b71162
-> k, nothing much to comment again, seems okay.

a93de72fecece932dc3586356648dd5fd53a7820
-> like Hirsute, we split the `remove` and `purge` actions and the
   extended commit message helps in making things clearer. Whilst
   purging, we remove anything and everything that was there under
   /run/qemu; okay, +1. But quick q: do we not care about /var/run/qemu
   as we seem to be doing in the "remove" case, few lines above?
   Apologies if it's obvious and I am just lacking the context.

2925434903ab858a3cc366d19e581ca5efc02664
-> follows https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1913421/comments/47.
   +1 there.

So TL;DR 1 typo in commit message & d/ch entry and 1 question to satiate my understanding. All good otherwise, thus approving.

review: Approve

[Utkarsh Gupta (utkarsh)]

fabba88fdbe53457d08725e6af58ae03c35f4d34
-> d/ch entry has a typo; mentioned above (1st line); please
   consider fixing that before uploading.

[Christian Ehrhardt  (paelzer)]

Typo cleaned up - thanks

We could make all and everything /var/run/qemu but for the sake of not changing more I've kept it as it is in the current state (using /run/qemu) in Focal (and Hirsute).
Indeed using /var/run/qemu everywhere would have been better, but in 99.9..% /var/run == /run so that only makes a difference in a very very few environments.
OTOH you are right, we touch it we could/should maybe clean it up.
If someone made /var/run != /run it could otherwise lead to problems.

I'll update them and re-test before uploading

[Christian Ehrhardt  (paelzer)]

All re-verified and uploading to -unapproved now.
Thanks everyone involved.

c155ebb... by Christian Ehrhardt  on 2021-09-30

d/qemu-block-extra.prerm.in: make check helper is executable

Signed-off-by: Christian Ehrhardt <email address hidden>

53992e6... by Christian Ehrhardt  on 2021-09-30

Revert "d/rules: --enable-module-upgrades not needed for qemu-system-x86-xen"

This reverts commit 430ef6313e3c9dcf15486e23a2833fae6193c38b.

e4dcd6c... by Christian Ehrhardt  on 2021-09-30

changelog: dropping xen module upgrade fix

While the code really is useless and it could be removed the SRU
mindset defines that we would not want/need to touch it if there is
no impact and we are not aware of any.

Signed-off-by: Christian Ehrhardt <email address hidden>

diff --git a/debian/changelog b/debian/changelog
index 081c8f3..20c9285 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+qemu (1:4.2-3ubuntu6.18) focal; urgency=medium
+
+  * enhance loading of old modules post upgrade (LP: #1913421)
+    - d/rules: d/qemu-system-gui.{prerm,postrm}.in: do not save gui modules
+      (can't be loaded late)
+    - d/qemu-block-extra.postrm.in: clear all (current and former) modules
+      on purge
+    - d/qemu-block-extra.prerm.in: test for exec and prepare /var/run/qemu
+      if needed
+
+ -- Christian Ehrhardt <[e-mail]>  Thu, 19 Aug 2021 14:10:54 +0200
+
 qemu (1:4.2-3ubuntu6.17) focal-security; urgency=medium
 
   * SECURITY UPDATE: NULL pointer dereference in MemoryRegionOps object
diff --git a/debian/qemu-block-extra.postrm.in b/debian/qemu-block-extra.postrm.in
index ef2126a..2193801 100644
--- a/debian/qemu-block-extra.postrm.in
+++ b/debian/qemu-block-extra.postrm.in
@@ -20,12 +20,17 @@ set -e
 
 
 case "$1" in
-    purge|remove)
+    remove)
         # remove .so files for still running qemu instances in /var/run
         # for details see bug LP: #1847361
         rm -f /var/run/qemu/@PKGVERSION@/block-*.so
     ;;
 
+    purge)
+        umount --quiet /var/run/qemu 1>/dev/null 2>&1 || true
+        rm -rf "/var/run/qemu" 1>/dev/null 2>&1 || true
+    ;;
+
     upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
     ;;
 
diff --git a/debian/qemu-block-extra.prerm.in b/debian/qemu-block-extra.prerm.in
index dee25a8..09f2a10 100644
--- a/debian/qemu-block-extra.prerm.in
+++ b/debian/qemu-block-extra.prerm.in
@@ -24,6 +24,14 @@ case "$1" in
     upgrade|deconfigure)
         # retain .so files for still running qemu instances in /var/run
         # for details see bug LP: #1847361
+        mkdir -p /var/run/qemu
+        printf "#!/bin/sh\nexit 0\n" > /var/run/qemu/exec
+        chmod +x /var/run/qemu/exec || true
+        if ! /var/run/qemu/exec 1>/dev/null 2>&1 && ! findmnt --mountpoint /var/run/qemu 1>/dev/null; then
+             # not executable and not yet a mount point on its own - create a tmpfs mount
+             mount -t tmpfs --source none --target /var/run/qemu --options defaults,nosuid,nodev,exec,mode=755 || true
+        fi
+        echo "/var/run/qemu is a mountpoint to allow still running qemu binaries of former builds (after package upgrades) to fallback-load modules from there" > /var/run/qemu/README || true
         mkdir -p /var/run/qemu/@PKGVERSION@
         cp /usr/lib/@ARCH@/qemu/block-*.so /var/run/qemu/@PKGVERSION@/
     ;;
diff --git a/debian/qemu-system-gui.postrm.in b/debian/qemu-system-gui.postrm.in
deleted file mode 100644
index 48c740a..0000000
--- a/debian/qemu-system-gui.postrm.in
+++ /dev/null
@@ -1,44 +0,0 @@
-#!/bin/sh
-# postrm script for brrr
-#
-# see: dh_installdeb(1)
-
-set -e
-
-# summary of how this script can be called:
-#        * <postrm> `remove'
-#        * <postrm> `purge'
-#        * <old-postrm> `upgrade' <new-version>
-#        * <new-postrm> `failed-upgrade' <old-version>
-#        * <new-postrm> `abort-install'
-#        * <new-postrm> `abort-install' <old-version>
-#        * <new-postrm> `abort-upgrade' <old-version>
-#        * <disappearer's-postrm> `disappear' <overwriter>
-#          <overwriter-version>
-# for details, see https://www.debian.org/doc/debian-policy/ or
-# the debian-policy package
-
-
-case "$1" in
-    purge|remove)
-        # remove .so files for still running qemu instances in /var/run
-        # for details see bug LP: #1847361
-        rm -f /var/run/qemu/@PKGVERSION@/ui-gtk.so
-        rm -f /var/run/qemu/@PKGVERSION@/audio-*.so
-    ;;
-
-    upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
-    ;;
-
-    *)
-        echo "postrm called with unknown argument \`$1'" >&2
-        exit 1
-    ;;
-esac
-
-# dh_installdeb will replace this with shell code automatically
-# generated by other debhelper scripts.
-
-#DEBHELPER#
-
-exit 0
diff --git a/debian/qemu-system-gui.prerm.in b/debian/qemu-system-gui.prerm.in
deleted file mode 100644
index 3624362..0000000
--- a/debian/qemu-system-gui.prerm.in
+++ /dev/null
@@ -1,46 +0,0 @@
-#!/bin/sh
-# prerm script for qemu-system-gui
-#
-# see: dh_installdeb(1)
-
-set -e
-
-# summary of how this script can be called:
-#        * <prerm> `remove'
-#        * <old-prerm> `upgrade' <new-version>
-#        * <new-prerm> `failed-upgrade' <old-version>
-#        * <conflictor's-prerm> `remove' `in-favour' <package> <new-version>
-#        * <deconfigured's-prerm> `deconfigure' `in-favour'
-#          <package-being-installed> <version> `removing'
-#          <conflicting-package> <version>
-# for details, see https://www.debian.org/doc/debian-policy/ or
-# the debian-policy package
-
-
-case "$1" in
-    remove)
-    ;;
-
-    upgrade|deconfigure)
-        # retain .so files for still running qemu instances in /var/run
-        # for details see bug LP: #1847361
-        mkdir -p /var/run/qemu/@PKGVERSION@
-        cp /usr/lib/@ARCH@/qemu/ui-gtk.so /var/run/qemu/@PKGVERSION@/
-        cp /usr/lib/@ARCH@/qemu/audio-*.so /var/run/qemu/@PKGVERSION@/
-    ;;
-
-    failed-upgrade)
-    ;;
-
-    *)
-        echo "prerm called with unknown argument \`$1'" >&2
-        exit 1
-    ;;
-esac
-
-# dh_installdeb will replace this with shell code automatically
-# generated by other debhelper scripts.
-
-#DEBHELPER#
-
-exit 0
diff --git a/debian/rules b/debian/rules
index 6a55d5a..ea64a7e 100755
--- a/debian/rules
+++ b/debian/rules
@@ -17,7 +17,7 @@ else
 VENDOR := DEBIAN
 endif
 
-AUTOGENERATED:= qemu-block-extra.prerm qemu-block-extra.postrm qemu-system-gui.prerm qemu-system-gui.postrm
+AUTOGENERATED:= qemu-block-extra.prerm qemu-block-extra.postrm
 PKGVERSION := $(shell printf "Debian ${DEB_VERSION}" | tr --complement '[:alnum:]+-.~' '_')
 
 # support parallel build using DEB_BUILD_OPTIONS=parallel=N