Open YehezkelShB opened 6 years ago
Assuming the ACL location and format was aligned, a potential problem could be that boltd hasn't flushed data and the database is out of sync when tbtadm/tbtacl write to it.
boltd would probably need to set up an inotify in that location in that instance and refresh the database.
Another way to solve race conditions this could be to add optional dbus support to tbtadm/tbtacl to allow them to talk to boltd if it's installed rather than to write sysfs files itself.
Oh, so bolt doesn't write the database to disk immediately? I wouldn't expect a lot of changes to the ACL, so this looks like a minor optimization in the cost of risking data loss in case of crash, for example.
Actually I don't know. I haven't looked at the codebase that closely, but it's a general statement for "daemon" applications. @gicmo should confirm.
Let me quickly address @superm1 question first: bolt writes out changes to the database immediately, but although not hard to implement, it does not yet watch the database directories for changes. That was indeed on my todo but it is not of high-prio so it has not been done yet.
For bolt the database location is already configurable and I actually changed the default for the 0.2 release from /var/lib/thunderbolt
to /var/lib/boltd
to a) be more consistent with other daemons (like fwupd, UPower, NetworkManager) and more importantly b) to not step on each others foot. Both tools can basically co-exist, but of course work in isolation currently. I think we are also ok in terms of interaction, if someone uses tbtadm
to authorize a device that is already in the bolt database, tbtadm
, as per udev rule, would authorize it first and it would show up as authorized for bolt (and it would do nothing about it).
As for aligning the actual formats, I am totally open to it. bolt is storing a bit more information not just the ACL though. Mostly to fulfil the needs of the design our UX designers come up with (see here). For example, bolt can store a label so a user can rename a device. One big thing: if we were to share the database we probably should also share other settings, most importantly the ability to temporarily disable authorization of devices (see the "Paranoid Mode" section in the design wiki). If we were to share the database but not the these kind of settings it would lead to confusing results when both tools are installed.
I was thinking of just disabling boltd
as the authorization agent when the presence of thunderbolt-tools is detected (i.e. tbtacl is present), but then of course you lose integration with GNOME (i.e. auto-device authorization and managing, i.e. renaming & removal of, devices via gnome-control-center).
Opinions?
I don't really like the idea of disabling boltd
for authorization just based on the presence of other tools. People install the world from apt and yum all the time so it can become pretty easy to gimp the GUI control just by installing another package that you preferred the command line interface or some other feature.
I think I would rather see either the alignment of formats and locations (maybe thunderbolt-tools just ignores the extra information in the ACL but writes it out into that format).
If you guys do align ACL format, then I think a good idea would be for thunderbolt-tools to detect bolt is installed and prepare symlinks to share ACL database to it.
Following this discussion, it seems to be better for the users if ACL for bolt and for tbtadm/tbtacl will be the same.
Points to discuss:
After agreeing on the previous points, the next set of considerations is:
(Cross-posting it from bolt project: https://gitlab.freedesktop.org/bolt/bolt/issues/78)