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 <e...@kiyuko.org> Resistance is futile, you will be garbage collected.
signature.asc
Description: PGP signature
