bus1 / dbus-broker

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

bus activated services that fail can just get `Could not activate remote peer` errors #269

Closed cgwalters closed 2 years ago

cgwalters commented 3 years ago

See https://github.com/coreos/rpm-ostree/pull/2932

If I explicitly force rpm-ostreed to fail during startup before claiming the bus name, clients can just see errors like:

[root@cosa-devsh ~]# rpm-ostree status
error: Could not activate remote peer.

But, if a service has an associated systemd unit (and they all do now right?) then dbus-broker could at least give us the exit status from systemd, the StatusText if present. And if there's no StatusText, maybe the last line from the journal from the unit?

teg commented 3 years ago

It would make sense to forward more information in this error message, see https://github.com/bus1/dbus-broker/blob/28af5ac996c94922fc49660e0efa955f57575187/src/launch/service.c#L202

There are three error cases:

systemd passes us different metadata in each case, so not sure we can always pass on anything helpful, but makes sense to go pass on whatever useful information we receive (not sure about querying more stuff from systemd/journald though).

cgwalters commented 3 years ago

The unit enters the failed ActiveState (probably your case, where the unit fails after being started, but before grabbing the DBus name).

Yep, this case.

I don't have a strong opinon on this. If it's hard to fix, feel free to ignore/close. I'm going to work on that linked PR to skip DBus for starting the daemon.

dvdhrm commented 3 years ago

The unit enters the failed ActiveState (probably your case, where the unit fails after being started, but before grabbing the DBus name).

Yep, this case.

I don't have a strong opinon on this. If it's hard to fix, feel free to ignore/close. I'm going to work on that linked PR to skip DBus for starting the daemon.

The biggest issue is that we don't want to perform IPC from within the IPC implementation. This easily leads to deadlocks. In this case here, it is less critical, because it would be an asynchronous call in the launcher, and thus not block the broker itself. However, it can still deadlock, if the call out triggers a dbus activation on the systemd side, which then calls back and possibly waits on that stalled call. All quite unlikely, but we have run into many such problems with NSS and user-uid resolution, and hence we prefer to avoid performing any IPC from within the broker.

Anyway, we could still collect the limited information we get and somehow include this in the errors. I am unsure whether this is quite as helpful as you would expect.

bluca commented 2 years ago

I believe https://github.com/bus1/dbus-broker/pull/277 should now address this

bluca commented 2 years ago

@dvdhrm you need to close this manually since you merged the commits from #277 manually

dvdhrm commented 2 years ago

Fair enough. We do provide the limited information we get from the unit-activation jobs. I still stand by not performing any more IPC just to debug IPC-failure. But I am open to discuss this if anyone has some ideas on how this could look like!