Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure dbus detects new services #20871

Merged
merged 1 commit into from Jan 27, 2017
Merged

Ensure dbus detects new services #20871

merged 1 commit into from Jan 27, 2017

Conversation

layus
Copy link
Member

@layus layus commented Dec 2, 2016

From the dbus introduction:

The service files go into a directory indicated in a
block in the bus' configuration file; the default location is
/usr/share/dbus-1/services/. If you add service files while the bus is
running, the bus daemon will notice and read them without any further
prodding.

This patch sets 's to /run/current-system/sw/... and bundles
all the provided dbus schemas in that location. System updates are
automatically catched by dbus.

Fixes #19034.

@mention-bot
Copy link

@layus, thanks for your PR! By analyzing the history of the files in this pull request, we identified @wkennington, @edolstra and @peterhoeg to be potential reviewers.

@tomberek
Copy link
Contributor

@layus Tested this patch but I still get the security policy error:


webserver.> ● avahi-daemon.service - Avahi mDNS/DNS-SD Stack
webserver.>    Loaded: loaded (/nix/store/f9ddz8k0rfzdyv5bk6awdflpvf2kfv8w-unit-avahi-daemon.service/avahi-daemon.service; bad; vendor preset: enabled)
webserver.>    Active: failed (Result: exit-code) since Mon 2016-12-12 15:36:25 UTC; 107ms ago
webserver.>   Process: 2003 ExecStart=/nix/store/b4lr10q8v3jcr0d6gc8yfhk8r1673aa6-unit-script/bin/avahi-daemon-start (code=exited, status=255)
webserver.>   Process: 1989 ExecStartPre=/nix/store/fhspsv0alf2xkrv2799jbd5q50203nhc-unit-script/bin/avahi-daemon-pre-start (code=exited, status=0/SUCCESS)
webserver.>  Main PID: 2003 (code=exited, status=255)
webserver.>    Status: "avahi-daemon 0.6.32 exiting."
webserver.>
webserver.> Dec 12 15:36:25 webserver avahi-daemon[2003]: Found user 'avahi' (UID 10) and group 'avahi' (GID 10).
webserver.> Dec 12 15:36:25 webserver avahi-daemon[2003]: Successfully dropped root privileges.
webserver.> Dec 12 15:36:25 webserver avahi-daemon[2003]: avahi-daemon 0.6.32 starting up.
webserver.> Dec 12 15:36:25 webserver avahi-daemon[2003]: dbus_bus_request_name(): Connection ":1.14" is not allowed to own the service "org.freedesktop.Avahi" due to security policies in the configuration file
webserver.> Dec 12 15:36:25 webserver avahi-daemon[2003]: WARNING: Failed to contact D-Bus daemon.
webserver.> Dec 12 15:36:25 webserver avahi-daemon[2003]: avahi-daemon 0.6.32 exiting.
webserver.> Dec 12 15:36:25 webserver systemd[1]: avahi-daemon.service: Main process exited, code=exited, status=255/n/a
webserver.> Dec 12 15:36:25 webserver systemd[1]: Failed to start Avahi mDNS/DNS-SD Stack.
webserver.> Dec 12 15:36:25 webserver systemd[1]: avahi-daemon.service: Unit entered failed state.
webserver.> Dec 12 15:36:25 webserver systemd[1]: avahi-daemon.service: Failed with result 'exit-code'.

@layus
Copy link
Member Author

layus commented Dec 12, 2016

@tomberek Just to be sure, you applied the patch and rebooted before testing ?
The current dbus config is irremediably broken and will require a reboot anyway.

To test the patch, you need first to apply the patch, disable avahi and reboot. On a machine booted with the patch (and avahi disabled), you can now enable avahi with nixos-rebuild switch and it should load correctly.
*replace avahi with any previously failing dbus client.

@tomberek
Copy link
Contributor

@layus Yes. I did a fresh re-creation of the environment and images. The above is the result of a nixops create followed by a nixops deploy.

nixos-version
17.03.git.9b473fb (Gorilla)
commit 9b473fb2e67f0ac11c56b6f1c2fab17fca2f4b25
Merge: 09fecd18b2 9ee476c0e3
Author: Thomas Bereknyei <tomberek@gmail.com>
Date:   Mon Dec 12 10:24:45 2016 -0500

    Merge branch 'dbus-reload' of github.com:layus/nixpkgs into dbus_reload

commit 09fecd18b2ebf7d309478b90965110b1f02f4650
Merge: 7c8d4cd9a9 570708183b
Author: Peter Simons <simons@cryp.to>
Date:   Mon Dec 12 14:46:27 2016 +0100

    Merge pull request #21002 from Profpatsch/hoogle-local

    haskellPackages: add hoogleLocal

@layus
Copy link
Member Author

layus commented Dec 12, 2016

I will need more time than I currently have to debug this. Maybe tomorrow morning.
The strange thing is that it worked for me.
Could it be that dbus is started even before /run/current-system is created ?

@FRidh
Copy link
Member

FRidh commented Jan 8, 2017

Status update?

DBus daemon now loads its config from /run/current-system/dbus.
Reloading the daemon makes it re-read that file and catch the updates
after a system upgrade.
@layus
Copy link
Member Author

layus commented Jan 9, 2017

@tomberek I just found a way to make it work correctly. Do you mind to try the new patch ?

@layus
Copy link
Member Author

layus commented Jan 18, 2017

@peterhoeg @domenkozar Would you mind testing and/or reviewing this PR ? It's working for me, but dbus is a core component. Better safe than sorry. You both worked on dbus in the past.

@peterhoeg
Copy link
Member

I will definitely have a look but realistically it will only be this weekend.

@fpletz fpletz added this to the 17.03 milestone Jan 20, 2017
@fpletz fpletz self-assigned this Jan 20, 2017
Copy link
Member

@fpletz fpletz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine, going to test this in the next few days.

@peterhoeg
Copy link
Member

@layus, I'm sorry, I haven't had the time to try this out... So 👎 to me (not your changes)...

@globin
Copy link
Member

globin commented Jan 25, 2017

ping @fpletz, have you tested this, yet?

@fpletz fpletz merged commit 29667f6 into NixOS:master Jan 27, 2017
@cstrahan
Copy link
Contributor

This looks great! With that said, would somewhere under /etc perhaps be a better place for this?

@layus
Copy link
Member Author

layus commented Jan 28, 2017

@cstrahan
/run/current system seemed the right place. Services started with a known config location need not use /etc. /etc should be used for config files managed by NixOS but not passed to the services directly. For example, /etc contains zshrc, zprofile, bashrc, groups, passwd and co, because NixOS manages them, but there is no way to pass a specific path to the applications using them. (You do not want to pass a nix-store path to every bash -l command.)
On the contrary, most systemd services are started with a specific store path as explicit configuration. (just look at the --config option in systemctl cat sshd).
In this case, we used to pass a store path to dbus, but it is not good enough as we need to pass a mutable path. /run/current-system is the perfect place for this as dbus services need always be in sync with the current-system.
Besides, dbus will still load dbus "modules" present in /etc. It seems better to me if these are user managed, without having NixOS putting some symlinks to uneditable readonly directories in there.

@cstrahan cstrahan mentioned this pull request Feb 1, 2017
@cstrahan
Copy link
Contributor

cstrahan commented Feb 1, 2017

I guess I'm just surprised that we didn't constrain the changes in this PR to something like:

-        "${lib.getBin pkgs.dbus}/bin/dbus-daemon --config-file=${configDir}/system.conf ${daemonArgs}"
+        "${lib.getBin pkgs.dbus}/bin/dbus-daemon --config-file=/etc/dbus-1/system.conf ${daemonArgs}"

It would seem that'd have the same effect.

@layus layus deleted the dbus-reload branch February 1, 2017 09:57
@abbradar
Copy link
Member

abbradar commented Feb 1, 2017

I feel that /etc is a good enough place for this -- I have an impression that we use it not only for files that we can't change path to but also for situations just like that when we want to reload service without a restart. However I don't have any specific service example to back this off the top of my head and it may be that my impression is faulty.

Anyway, in this case /etc/dbus-1 seems better to be retained because Xfce starts dbus daemon by itself (#22302). There may be other places where it's started like this -- /etc/dbus-1 should help because it's a standard path and D-Bus uses it without any command line flags. I can make the patch if it's okay -- what do you think?

@layus
Copy link
Member Author

layus commented Feb 1, 2017

@abbradar, @cstrahan I explained above why /etc is not a good default choice. Now, I have overlooked the importance of session instances in this issue. If session dbus instances can be started by anyone, then /etc becomes the right place to use.
Now, I am still unable to understand why it worked before. I will investigate and report in #22302

@vcunat
Copy link
Member

vcunat commented Feb 1, 2017

The /etc thing is a more general question. I was thinking of it lately (when adding knot-resolver service) and in the end I put the generated file into /etc/, so that people can more easily discover the current configuration. I prefer to conform to what most distros use unless I see some clear advantage in doing things differently (least surprise); and I can't see any here ATM, as /etc/ is supposed to be switched at once with /run/current-system.

@layus
Copy link
Member Author

layus commented Feb 1, 2017

@vcunat I too feel that discoverability is important. But I would rather teach NixOS users the systemctl cat xxx.service than putting in /etc files that are not read by the service.

@abbradar
Copy link
Member

abbradar commented Feb 1, 2017

I don't have any evidence but I think this follows a pattern same to for example systemd -- /usr/share/systemd for configuration files that are installed via package managers, /etc/systemd for user overrides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants