[Pkg-libvirt-maintainers] Bug#1064126: Bug#1064126: libvirt: install NSS modules into /usr

Andrea Bolognani eof at kiyuko.org
Tue Aug 27 23:17:05 BST 2024


On Tue, Aug 27, 2024 at 08:34:21AM +0200, Helmut Grohne wrote:
> dumat already did its work. Your ping was still helpful. Please do not
> upload libvirt to unstable as is. I'll copy the relevant yaml report
> here for reference:
> 
> libvirt-daemon-log:
>   10.6.0-2:
>     issues:
>     - files:
>       - /usr/lib/systemd/system/virtlogd-admin.socket
>       - /usr/lib/systemd/system/virtlogd.service
>       - /usr/lib/systemd/system/virtlogd.socket
>       others:
>         libvirt-daemon-system:
>           10.6.0-1: trixie|unstable
>           7.0.0-3+deb11u2: bullseye
>           7.0.0-3+deb11u3: bullseye-proposed-updates
>           8.0.0-1~bpo11+1: bullseye-backports
>           9.0.0-4: bookworm
>           9.0.0-4+deb12u1: bookworm-proposed-updates
>       what: ineffective replaces
>     source: libvirt
>     suites: experimental
>
> Reading this is not trivial. The common pattern is "what: ineffective
> replaces", which means you have DEP17 P1 problems. Refer to
> https://subdivi.de/~helmut/dep17.html for what that means precisely.
> They're all of the kind where you move a systemd unit around from one
> package to another and at the same time from /lib/systemd/system to
> /usr/lib/systemd/system.
> 
> We may choose between M7 (upgrading Replaces to Conflicts), M8
> (protective diversions) and a combination of both. Given that libvirt
> may be critical to some systems, I suggest that M7 is not sufficiently
> reliable on its own. Doing M8 only is what we tend to do for really
> critical things such as e2fsprogs, but it has the cost of persisting
> protective diversions into the actual system. By combining both, we can
> remove the protective diversions at the end of the upgrade process such
> that they are gone once the system is upgraded to trixie and you get to
> remove the mitigation code after trixie is released. I suggest that the
> last option is a good trade-off for libvirt in terms of safety vs
> effort. Do you agree? Please don't hesitate to ask if this is unclear.

I read up a bit on the various problems and mitigations documented as
part of DEP17 yesterday and figured that our changes would probably
hit P1. I hoped that M7 might be enough to take care of that, but I
trust your expertise when you say that M7+M8 is a better fit for our
specific case.

The fact that the diversions will only exist for the duration of the
upgrade, instead of sticking around afterwards, makes this approach
seem particularly attractive to me.

> Let me sketch how to apply M7+M8. For each ineffective replaces, we
> upgrade Breaks+Replaces to Conflicts in debian/control.

So this part is as simple as replacing the existing

  Breaks: libvirt-daemon-system (<< 10.6.0-2~)
  Replaces: libvirt-daemon-system (<< 10.6.0-2~)

with

  Conflicts: libvirt-daemon-system (<< 10.6.0-2~)

in the case of libvirt-daemon-log, right? Or are there additional
considerations?

> Additionally, we
> add code to preinst and postinst for each affected file. For example
> libvirt-daemon-log.preinst would dpkg-divert --divert
> /lib/systemd/system/virtlogd.socket.usr-is-merged --no-rename --add
> /lib/systemd/system/virtlogd.socket and libvirt-daemon-log.postinst and
> libvirt-daemon-log.postrm would remove the diversion. It is diverting
> the aliased path that is formerly installed by libvirt-daemon-system and
> not the canonical path installed by libvirt-daemon-log.

I don't fully understand how that would help avoid the file loss
scenario, but once again I trust your expertise here so thankfull I
don't really need to O:-)

> Hence, lintian may be annoyed.

That's fine, we can install overrides. That'd be more than warranted
in this case.

> Do you prefer receiving a patch with these mitigations against the
> experimental package or do you prefer implementing this yourself?

Let's meet halfway :)

I believe you're better positioned to get the details of the
implementation right, but I obviously have my own preferences when it
comes to what the code that gets committed to libvirt looks like.

So for now I've sketched out an implementation, which you can find
below. The calls to dpkg-divert are most likely completely wrong, and
I'd appreciate it if you could fix them.

Up to you whether you want to go all the way and prepare a MR with
the changes, or leave that last bit to me. You'll obviously be
credited in debian/changelog either way :)

> > Assuming no issues are detected, I would like to upload 10.7.0-1 with
> > these changes and the new upstream release to unstable in a week or
> > so, but I can delay that for a bit if more time is needed.
> 
> Yes, please delay. We need one more experimental upload to confirm the
> correctness of the mitigations.

Gotcha.



diff --git a/debian/libvirt-daemon-log.postinst.in b/debian/libvirt-daemon-log.postinst.in
index 21723d4307..d75b30d930 100644
--- a/debian/libvirt-daemon-log.postinst.in
+++ b/debian/libvirt-daemon-log.postinst.in
@@ -15,6 +15,7 @@ set -e
 # the debian-policy package
 
 #FINISH_CONFFILE_TRANSFER#
+#DELETE_PROTECTIVE_DIVERSION#
 
 DAEMON_SYSTEM_TO_DAEMON_LOG="
     /etc/default/virtlogd
@@ -24,6 +25,12 @@ DAEMON_SYSTEM_SYSV_TO_DAEMON_LOG="
     /etc/init.d/virtlogd
 "
 
+VIRTLOGD_UNITS="
+    virtlogd-admin.socket
+    virtlogd.service
+    virtlogd.socket
+"
+
 case "$1" in
     configure)
         for conf in $DAEMON_SYSTEM_TO_DAEMON_LOG; do
@@ -46,6 +53,13 @@ case "$1" in
                 -- \
                 "$@"
         done
+        for unit in $VIRTLOGD_UNITS; do
+            delete_protective_diversion \
+                "/lib/systemd/system/$unit" \
+                "10.6.0-3~" \
+                -- \
+                "$@"
+        done
     ;;
 
     abort-upgrade|abort-remove|abort-deconfigure)
diff --git a/debian/libvirt-daemon-log.preinst.in b/debian/libvirt-daemon-log.preinst.in
new file mode 100644
index 0000000000..f5dbf73de9
--- /dev/null
+++ b/debian/libvirt-daemon-log.preinst.in
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+set -e
+
+# summary of how this script can be called:
+#
+#   * <new-preinst> `install'
+#   * <new-preinst> `install' <old-version> <new-version>
+#   * <new-preinst> `upgrade' <old-version> <new-version>
+#   * <old-preinst> `abort-upgrade' <new-version>
+#
+# for details, see https://www.debian.org/doc/debian-policy/ or
+# the debian-policy package
+
+#CREATE_PROTECTIVE_DIVERSION#
+
+VIRTLOGD_UNITS="
+    virtlogd-admin.socket
+    virtlogd.service
+    virtlogd.socket
+"
+
+case "$1" in
+    install|upgrade)
+        for unit in $VIRTLOGD_UNITS; do
+            create_protective_diversion \
+                "/lib/systemd/system/$unit" \
+                "10.6.0-3~" \
+                -- \
+                "$@"
+        done
+    ;;
+
+    abort-upgrade)
+    ;;
+
+    *)
+        echo "preinst called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+#DEBHELPER#
+
+exit 0
diff --git a/debian/snippets.sh b/debian/snippets.sh
index 6b03f242d6..ad86432332 100644
--- a/debian/snippets.sh
+++ b/debian/snippets.sh
@@ -194,6 +194,54 @@ remove_config_from_template() {
 }
 #END REMOVE_CONFIG_FROM_TEMPLATE
 
+#BEGIN CREATE_PROTECTIVE_DIVERSION
+create_protective_diversion() {
+    local usrfile="$1"
+    local firstver="$2"
+
+    if [ "$2" != "--" ]; then
+        echo "create_protective_diversion called with the wrong number of arguments" >&2
+        return 1
+    fi
+    for _ in $(seq 1 2); do
+        shift
+    done
+
+    if [ -z "$2" ] || dpkg --compare-versions -- "$2" gt "$lastver"; then
+        return 0
+    fi
+
+    dpkg-divert \
+        --no-rename \
+        --divert "$usrfile.usr-is-merged" \
+        --add "$usrfile"
+}
+#END CREATE_PROTECTIVE_DIVERSION
+
+#BEGIN DELETE_PROTECTIVE_DIVERSION
+delete_protective_diversion() {
+    local usrfile="$1"
+    local firstver="$2"
+
+    if [ "$2" != "--" ]; then
+        echo "delete_protective_diversion called with the wrong number of arguments" >&2
+        return 1
+    fi
+    for _ in $(seq 1 2); do
+        shift
+    done
+
+    if [ ! -e "$usrfile.usr-is-merged" ]; then
+        return 0
+    fi
+
+    dpkg-divert \
+        --no-rename \
+        --divert "$usrfile.usr-is-merged" \
+        --remove "$usrfile"
+}
+#END DELETE_PROTECTIVE_DIVERSION
+
 #BEGIN SYSTEMD_DAEMON_RELOAD
 systemd_daemon_reload() {
     if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then
-- 
Andrea Bolognani <eof at kiyuko.org>
Resistance is futile, you will be garbage collected.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-libvirt-maintainers/attachments/20240828/6295d0a3/attachment.sig>


More information about the Pkg-libvirt-maintainers mailing list