[Pkg-libvirt-maintainers] Bug#1082939: /usr/bin/deb-systemd-helper: error: unable to read virtlockd.socket

[Andrea Bolognani]

On Sun, Oct 06, 2024 at 01:12:57PM +0200, Michael Biebl wrote:
> Am 05.10.24 um 19:51 schrieb Andrea Bolognani:
> > On Fri, Oct 04, 2024 at 08:57:36PM +0200, Michael Biebl wrote:
> > > My recommendation would be to drop the "Also=virtlockd.socket" line from
> > > libvirt.service (and "Also=virtlogd.socket" as well), or upgrade the weak
> > > recommends to a depends for both packages.
> > 
> > The current setup seem to match intentions, with one caveat that I'll
> > address below.
> > 
> > virtlogd and virtlockd are secondary daemons that the primary
> > libvirtd daemon might need to delegate some operations to, so it
> > makes sense that they'll be enabled/started together with it; at the
> > same time, there are scenarios in which it's valid to have just the
> > primary daemon, hence why weak dependencies are used both between
> > unit files and packages.
> > 
> > The caveat is that libvirtd.service contains Requires=virtlogd.socket
> > without a matching Depends from libvirt-daemon to libvirt-daemon-log.
> > This is an obvious bug that needs to be addressed.
> > 
> > Turning the Recommends from libvirt-daemon to libvirt-daemon-lock
> > into a Depends is undesirable, as it would force people to have
> > virtlockd installed on their systems even though the vast majority of
> > setups don't actually need it. The whole point of moving the various
> > drivers and daemons to separate packages was to allow for
> > minimalistic, carefully curated installations, so this would
> > represent an unwanted step backwards.
> > 
> > Dropping the Also= lines is undesirable too, as it would require
> > users to explicitly care about enabling/disabling virtlogd and
> > virtlockd when enabling/disabling libvirtd. With the way things
> > currently work, it's enough for users to operate on the primary
> > daemon and the secondary ones will come along for the ride. Plus it
> > would mean having to diverge from upstream.
> > 
> > It should be noted that systemd itself is perfectly happy with this
> > arrangement: since Wants= and Also= are weak dependencies, the
> > corresponding units being missing is not considered a problem. As far
> > as I can tell, that doesn't even cause a warning to be logged
> > anywhere, much less print out an error message.
> > 
> > I don't know the internals of deb-systemd-helper very well, but is
> > there a chance that we could simply make it as tolerant to this
> > scenario as systemd itself is?
> 
> /usr/lib/systemd/system/virtlockd-admin.socket:Also=virtlockd.socket
> 
> Yet debian/rules uses:
> 
> dh_installsystemd -p libvirt-daemon --no-also libvirtd.service
> dh_installsystemd -p libvirt-daemon --no-stop-on-upgrade libvirtd.socket
> libvirtd-ro.socket libvirtd-admin.socket
> 
> I.e. virtlockd.socket is also referenced from virtlockd-admin.socket but the
> dh_installsystemd line for libvirtd-admin.socket does not contain
> "--no-also"

Sorry, I'm very confused.

These are the Also= lines for libvirtd sockets:

  $ grep -h Also= libvirtd*.socket | sort -u
  Also=libvirtd-admin.socket
  Also=libvirtd-ro.socket
  Also=libvirtd.socket

and these are the ones for virtlockd sockets:

  $ grep -h Also= virtlockd*.socket | sort -u
  Also=virtlockd-admin.socket
  Also=virtlockd.socket

So the libvirtd sockets only reference other libvirtd sockets, and
the virtlockd sockets only reference other virtlockd sockets.

The only cross-package references that exist are the ones we've
already identified:

  $ grep Also= libvirtd.service | sort -u | grep -v libvirtd
  Also=virtlockd.socket
  Also=virtlogd.socket

But as you've noted above the relevant dh_installsystemd invocation
uses --no-also:

  dh_installsystemd -p libvirt-daemon --no-also libvirtd.service

So it doesn't look like this is the problem? Unless I'm missing
something.

> so dh_installsystem will generate maintscript code to enable
> virtlockd.socket in libvirt-daemon.postinst (you can check the generated
> code after a successful build to verify that).

I'm not seeing any reference to virtlockd (or virtlogd) in
libvirt-daemon.postinst, is it supposed to be called out explicitly?
Can you please show me the snippet you're referring to?

> You probably need to more closely inspect which unit file references which
> other unit via Also= and if they are shipped in the same package or if not,
> what packaging relationship they have.

This is a good idea in general :) I'm not surprised to see some
fallout from the recent package split.

> Keep in mind that "--no-also" is currently an undocumented option of
> dh_installsystemd, i.e. not an officially supported feature.

Use of --no-also was introduced in [1] to prevent sockets from being
restarted together with the daemon, which caused the latter to crash.

I'm honestly not sure whether it's still relevant these days. I
should try ripping it out and seeing whether the generated postinst
snippet changes.


[1] https://salsa.debian.org/libvirt-team/libvirt/-/commit/ff981d56ff2ff261e10b85003b35d892e6832dcf
-- 
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/20241006/b3e52d5b/attachment.sig>