search

bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
667 stars 78 forks source link

launcher: use new StartUnitWithFlags method if available #278

Closed bluca closed 2 years ago

bluca commented 2 years ago

Check if the new method from systemd is available via Introspect(), and if so use it. This allows us to receive 'skipped' as the result when the service we are trying to activate does not run because of a failed Condition check, which would return a false 'done' with the older StartUnit method, resulting in us just waiting for a unit to show up that will never arrive.

Fixes #276

Requires new systemd interface from https://github.com/systemd/systemd/pull/21609 so opening as draft for now

bluca commented 2 years ago

https://github.com/systemd/systemd/pull/21609 has been approved and will be merged soon and be part of the v250 release, so this is now ready for review. PTAL!

bluca commented 2 years ago

This is looking really good! I had some minor points about error handling inline (our error handling is a bit non-standard). Otherwise, I'm pretty happy with this, though would love to wait for systemd to hit rawhide, so we can test that this works as expected with the new interface.

I've tested this by building an image with mkosi, it's really simple if you want to give it a shot. Simply install mkosi (it's available as a package in fedora), clone the systemd git tree on the latest main branch, and from the systemd git repo directory run:

$ sudo mkosi build
$ sudo systemd-dissect --copy-to image.raw <path/to/dbus/broker/build/dir>/dbus-broker-launch /usr/bin/dbus-broker-launch
$ sudo systemd-dissect --copy-to image.raw <path/to/dbus/broker/build/dir>/dbus-broker /usr/bin/dbus-broker
$ sudo mkosi boot

This will build an image of the current running distro (so Fedora in your case) and add the systemd binaries built from the repo, and then copy the dbus broker binaries on top of it, and then boot it using nspawn. To check the failure mode, simply add a drop-in for any bus-activatable unit with:

[Unit]
ConditionPathExists=/tmp/marker

You can verify that before creating the marker file the unit doesn't activate and you get an error immediately with this patch, but it gets stuck without.

teg commented 2 years ago

This is looking great to me! Though I'll give @dvdhrm a chance to have a look before merging.

bluca commented 2 years ago

The interface is going to be reworked a bit, so closing for now