bus1 / dbus-broker

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

launch: do not log on destructive transactions #194

Closed teg closed 5 years ago

teg commented 5 years ago

If we try to activate a service that is in the process of being stopped, systemd refuses with an error saying the transaction is destructive.

This is expected behavior, especially on shutdown. So we should not be logging about that.

Signed-off-by: Tom Gundersen teg@jklm.no

bl33pbl0p commented 5 years ago

This should *only* happen during shutdown, it is a problem if it happens during a normally activated transaction. Does it happen otherwise?

If we try to activate a service that is in the process of being stopped ...

You are using the replace job mode, which installs your job in the unit's job slot. This error will not be observable in general, but during shutdown, the job mode used for the transaction with shutdown.target as the anchor is replace-irreversibly, which means until you do a manual "cancel" on the job, any attempt from a job to replace it and whatever was generated as part of the transaction will fail (hence the "stop" is queued and "start" requested error).

Otherwise, usually, your start job will be installed on the unit replacing the previous stop job, and it will keep waiting in the JOB_WAITING state until the action invoked from the previous stop job finishes gracefully (on every sub state change, unit_start keeps returning -EAGAIN, which keeps moving the start job in and out of the run queue for scheduling until it can run). However, this logic only holds true for the service state machine (but that's what DBus cares about anyway).

dvdhrm commented 5 years ago

@bl33pbl0p, Yeah, that's true. I now amended the comment and clarified under which circumstances this can happen. The general idea is to never log an error when we consider the problem recoverable. In this case the error is definitely recoverable and it is not a system configuration error, so we cannot unconditionally log an error.

I merged the PR and pushed a comment-fixup on top. Feel free to comment if there are still open questions.

Thanks a lot! David