firehol: init at 3.1.4, iprange: init at 1.0.3 #29125
Conversation
|
@geistesk, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @bjornfor and @offlinehacker to be potential reviewers. |
|
Can you make one commit per application? |
| + #fi | ||
| ]) | ||
|
|
||
| AS_IF([test "x$ac_cv_ping_6_opt" = "xyes"],[ |
There was a problem hiding this comment.
Is this ping command used at any point without root? If, yes we have to point at /run/wrappers/bin/ping to run the setuid version of it.
There was a problem hiding this comment.
Could you point me to an example for this?
And is this even required? I think we can assume that the newer ping is available.
Thanks for your feedback!
There was a problem hiding this comment.
what I mean to say, if you just let autotools detect ping it will take the ping from the nix store. This ping has no setuid bit set, so it will be only usable by root. Now the question is, if this ever a problem from this application.
There was a problem hiding this comment.
Well, afaik fireqos won't start unless you run it as root. So I don't see a problem there.
There was a problem hiding this comment.
I would hard-code ping6 path here, because otherwise the service will not find the binary later in the service.
There was a problem hiding this comment.
Why should the old ping6 be preferred over the newer ping-version with the -6-flag? Fire{HOL,QOS} seems to work fine with the given ping-command because it must be run as root.
Edit: I'll change the ping-executable with the one at /run/wrappers/bin/ping.
| default = false; | ||
| description = '' | ||
| If enabled, FireQOS will be launched with the specified | ||
| configuration given in `qosConfig`; |
There was a problem hiding this comment.
- configuration given in `qosConfig`;
+ configuration given in `qosConfig`.
| Type = "oneshot"; | ||
| RemainAfterExit = true; | ||
| ExecStart = "${pkgs.firehol}/bin/fireqos start ${fireqosConfig}"; | ||
| ExecStop = "${pkgs.firehol}/bin/fireqos stop; ${pkgs.firehol}/bin/fireqos clear_all_qos"; |
There was a problem hiding this comment.
This is not a shell. you can not run two commands by using ;.
Try something like:
ExecStop = [
"${pkgs.firehol}/bin/fireqos stop"
"${pkgs.firehol}/bin/fireqos clear_all_qos"
];
|
Thanks! |
Motivation for this change
This commit adds the FireHOL-software (v3.1.4) and it's dependency iprange (v1.0.3). The FireHOL-package also contains FireQOS, which is a human-friendly
tc-wrapper. A NixOS-module to interact with FireQOS was also provided.Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)