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

powerd: init at 1.0.0 #75706

Closed
wants to merge 3 commits into from
Closed

powerd: init at 1.0.0 #75706

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 15, 2019

Motivation for this change

Add powerd, a simple daemon to handle battery power levels (execute a warning or a command depending on how low is a battery level). Add a module for it.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@ghost
Copy link
Author

ghost commented Dec 16, 2019

@GrahamcOfBorg build powerd

--exec-command "${cfg.execCommand}"
'';
serviceConfig = {
User = cfg.user;
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I don't like how this is root by default. How about this:

  • The module sets --exec-command to be echo TRIGGER > /run/powerd/triggerfile
  • In parallel to that a script that executes execCommand every time that file is written to
  • This script can then run as root or whatever, while powerd can run with DynamicUser = true without any privileges. This way we know that only execCommand can be run as root
  • This doesn't change the module interface at all, but is more secure

Copy link
Author

Choose a reason for hiding this comment

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

@infinisil Thanks! While testing I've found that dbus notifications don't work from systemd unit, so I'll remove the module. It will be better to run powerd right from user session.

Copy link
Member

@infinisil infinisil Dec 21, 2019

Choose a reason for hiding this comment

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

NixOS does support user systemd services with systemd.user.services. Want to use that instead? Can also always be done later still if needed

Copy link
Author

Choose a reason for hiding this comment

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

@infinisil Yes I know, but are they popular? I've never used any because of this issue #21460

@ghost
Copy link
Author

ghost commented Dec 22, 2019

This software doesn't consider charge mode, always executing command. So I think its not viable.

@ghost ghost closed this Dec 22, 2019
@ghost ghost deleted the powerd branch December 22, 2019 17:00
This pull request was closed.
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

1 participant