Willy, Am 19.11.2017 um 07:32 schrieb Willy Tarreau: > Tim, I'll discuss this with William. I'm a bit embarrassed with taking > this change right now while everyone is still working on fixing existing > issues.
Understood. I see that my timing is unfortunate, but I just got my feet wet with the `execvp` bugfix and wanted to try a bit more. :-) > We may possibly end up postponing it for 1.9 and then backporting > it to existing versions once properly validated, or merging it anyway but > reverting it at the first problem report. I consider the patch to the C source code to be safe (as outlined below). The patch to the unit file is up for discussion, because there is no single good value (outlined below as well). > In the mean time, since you mentionned that building without USE_SYSTEMD > could cause problems, I think the unit file needs to at least get some > comments to indicate the values to use depending on the build options. It really is an unfortunate situation, because the correct value depends on the haproxy.cfg: Type=simple (the default and what's provided for haproxy < 1.8): This is safe iff `daemon` is *not* set. Type=forking (what's provided for haproxy 1.8+): This is safe iff `daemon` is set. This is a possible breakage with haproxy 1.8+: See my initial email (Add systemd `Type=notify` support) in this thread. Type=notify (what my patch proposes): This is safe iff my patch is compiled in. If `daemon` is set then `NotifyAccess=all` will need to be configured in the unit file as well. We can put `NotifyAccess=all` by default, but that has some security implications as any process in haproxy's control group could send messages to systemd. My patch will not cause issues if `Type` is not equal to `notify`. Old unit files will continue to work, even if it is compiled in. Quoting from sd_notify(3): > RETURN VALUE > On failure, these calls return a negative errno-style error code. If > $NOTIFY_SOCKET was not set and hence no status data could be sent, 0 is > returned. If the status was sent, these functions return with a > positive return value. In order to support both, init systems that > implement this scheme and those which do not, it is generally > recommended to ignore the return value of this call. So there is no value that is universally good. In my opinion the best would be Type=notify, because it is not dependent on haproxy.cfg. I don't know how large the percentage of users is that don't use their distro provided haproxy. Personally I trust the distributions to add the `USE_SYSTEMD` build option if they support systemd and then the "end user" would not be able to break startup using haproxy.cfg alone. > Also I don't know what systemd versions are found in field, but to be > honnest I don't feel much comfortable with the possibility that users > easily get the wrong unit file for their build options or the wrong > build options for their system, I already predict some reports here. See above on why there is no single good value. The `sd_notify()` API is supported as of systemd v1, though. The commit documenting it is from 2010: https://github.com/systemd/systemd/commit/f9378423b9758861850748aeb49ae0d3300e56e6 On Ubuntu I only had to install the `libsystemd-dev` package to get the necessary headers. > Maybe two distinct unit files should be provided, and instead of naming > the option "USE_SYSTEMD", it should be named "USE_SD_NOTIFY" to make it > clear what it implies ? I opted for USE_SYSTEMD, because the functionality is provided by libsystemd. It also provides the `sd_listen_fds()` function if one wants to build support for systemd's socket passing in the future. So USE_SD_NOTIFY would be a bit of a misnomer. The build flag is only required, because of the non-Linux or non-systemd systems. The function calls themselves won't cause issues as outlined above. I hope I was able to give a good overview so that you can decide on the proper course going forward. Thanks! Tim Düsterhus
