Merge ~paelzer/ubuntu/+source/qemu:lp-1913421-exec-run-BIONIC into ubuntu/+source/qemu:ubuntu/bionic-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.

[Christian Ehrhardt  (paelzer)]

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

[Utkarsh Gupta (utkarsh)]

6863f0b1c8c87175eac800835cae8980e37b4d46
-> 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.

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

ecb77f389ef403a06e1a5fc9f1d4a4b422cf7969
-> d/ch looks good. +1.

So TL;DR 1 question to satiate my understanding. All good otherwise, thus approving.

review: Approve

[Christian Ehrhardt  (paelzer)]

Indeed there is some inconsistency in Bionics version of it. Due to the different stages of /var/run == /run.
I'm tempted to say that /var/run/qemu is the better string to use and will do so in all of Bionic. In the others I'll not introduce a mess by changing them all over, although one could say it could/should be changes there as well.

I've initially kept it /run/qemu to match the later behavior, but for Bionic where a lot is /var... already I'll unify on that.

Thanks - I'll clean that up.

[Christian Ehrhardt  (paelzer)]

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

diff --git a/debian/changelog b/debian/changelog
index cdc52db..e10b766 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+qemu (1:2.11+dfsg-1ubuntu7.38) bionic; urgency=medium
+
+  * enhance loading of old modules post upgrade (LP: #1913421)
+    - d/qemu-block-extra.prerm.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:30:25 +0200
+
 qemu (1:2.11+dfsg-1ubuntu7.37) bionic-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 5c3d73e..84667da 100644
--- a/debian/qemu-block-extra.postrm.in
+++ b/debian/qemu-block-extra.postrm.in
@@ -20,12 +20,19 @@ 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)
+        # Might fail if guests run that have files loaded from there
+        # That is ok and will be cleaned on reboot
+        umount /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@/
     ;;