Er, I did post it to -devel, and I don't see any discussion of it on -users...I just double-checked the archives, and it's all in -devel...(why is this being discussed on -users? Taking it over to -devel)
--daemon and --syslog already control syslog's program name. Since we would have to support the existing usages, your suggestion to add that to --syslog-facility would add complexity, not simplify things.--syslog-facility should be more clean, like --sysloc-facility facility [progname]
You really don't want that.
Sorry that you don't like my accommodation for init scripts. It helps OpenVPN and its USERS.intermixing option argument "A" and "B" in the same string is not going to help anyone.
The fact is that I'm not going to get all the distributions to change their init scripts, and neither are you. It's a major effort to do that and get it coordinated across all the distributions. No one -- not me, not you, and not the distro maintainers -- has time for that.Init scripts can be adjusted.
Asking our users to do it piecemeal - and maintain it or lobby their distributions is not good engineering. It's a problem that we can solve on this end.
Even if one could get distro cycles to change their init scripts - why would we waste them on something like this? We'll
want them when we want something that can't be handled without their help...
the last one really makes me want to throw up.
Yes, it's ugly. And my stomach rumbles too.If OpenVPN were engineered from scratch, --daemon would just daemonize, --syslog would control the syslog program name and facility, and the two could be used together. And there wouldn't be silly ordering constraints among the options. But that's not the way the existing code -- and customers' config and distros' init files -- work.
This is the real world, and sometimes we have to live with ugly. Even if it makes us a little queasy.
Using [] as part of an option string is a bad idea, because it is commonly used for optional arguments (as you saw in David's question), and
As for the choice of square brackets - the delimiter had to be something easy to parse that is uncommon in program and file names.
The init file usage does not make it easy. Facility can not be a prefix, because the Debian usage I cite generates the --daemon argument from the config file name -- adding a prefix. It can't be a suffix because of the way systemd-based distros auto-generate multiple instances.
So it has to be something that can be embedded anywhere in the string, And that means delimiters on both ends.
I was aware that [ ] is used for 'optional' in syntax descriptions when I picked them. But I provided examples to clarify the point. And the other choices were worse, as they have other meanings to shells and filesystems. At least [ ] is visually distinctive and
easy to remember.It could be something else. Do you have a better idea that doesn't add complexity?
Actually, the first pass didn't - the facility was ONLY in the progname argument. See the patch in Trac, which was the minimalIt brings it's own config statement (--syslog-facility)
change required to solve the problems.However, the additional code is minimal, and for cases where it is possible to avoid the ugly overloading of progname, it is a cleaner way to specify facility. I thought it worth the effort so as not to force ugliness on users when it isn't required.
someone else considered "a clever hack"
None of this was the result of random choices or "cleverness". I've been in the software business long enough to have dealt with more of that than most. Try adding a major feature to a 30-year old OS that's written in assembler...or to microcode.
I thought carefully about the issues, the requirement and how to meet it in a way that minimizes the internal and external impacts.
I'm busy too - but I took the time to understand the problem and do a proper job. I checked multiple distributions, consulted standards, localized the changes and did careful testing in the environments that I have. And tried to give back to the community.
I could have just compiled with -DLOG_OPENVPN=LOG_LOCAL5 and gone on with my life... Or thrown an 'elegant' solution over
the wall and left customers to deal with the init file issues.I didn't ignore it for 2 years, and I didn't let aesthetics get in the way of the best solution that I could come up with.
The scope is hardly large. If you look at the actual code - the size of the patch is dominated by documentation and the lookup table. There are a couple of lines to parse the option, a subroutine to lookup the names, and local block of code in open to handle the syntax.If we take this at all - and I have reservations, because the amount of code it brings in is quite significant
Although the init-script compatibility is ugly and adds a few lines of code, it meets the requirements with the least total impact to all concerned.
Feature-NAK
If you don't want the feature, why did you write in Trac #188:
If someone does a patch, I think 2.4 is perfectly fine.For 2.3, I'm more sceptical - translating syslog strings to the syslog(3) facility values needs a translation table, lookup, some systems might not have all the facilities the rest has, so it's not a one-line change and needs cross-platform testing.
?You clearly understood what was requested and the scope of the change, aside from the init file issue that you didn't anticipate. (And neither did I, until I dug in to the problem.)
As soon as I tried to configure OpenVPN, I saw that it used syslog, and the first thing I looked for was how to send output to a facility of my choice. I saw the request in Trac - and several times in the mailing lists/forums. I'm hardly the only person requesting the feature.
Given that the standard specifies the facility names, the local scope of the code and the fact that to trigger any of the new code paths one has to take positive action, I think this is reasonably safe for 2.3.
As I noted in the Trac ticket and in the previous notes, what I did was carefully portable and standard-compliant. It's not a one-line change; it does
need testing, but it's not as bad as you thought. Standards help.I don't expect issues - but if there are any, they will happen the first time it's compiled on some (probably obscure) platform that doesn't have a required facility code. It's unlikely, but possible. It won't be something subtle in the field -- like TLS1.2 :-(
But if you want to hold it for 2.4, it's your project.And if you have a solution that is a lot prettier AND addresses the init-file issues, I'm open to considering it.
At the moment, I'm still convinced that what I posted is the best solution available.
Timothe Litt ACM Distinguished Engineer -------------------------- This communication may not represent the ACM or my employer's views, if any, on the matters discussed. On 02-May-14 03:23, Gert Doering wrote:
Hi, (why is this being discussed on -users? Taking it over to -devel) On Thu, May 01, 2014 at 09:52:45PM -0400, Timothe Litt wrote:e.g. the following are valid: --daemon Daemonize, use the default progname (from PACKAGE) for syslog --daemon my-vpn-daemon Daemonize, use "my-vpn-daemon" for the syslog progname. --daemon [local1] Daemonize, use the default progname, use the local1 syslog facility (unless --syslog-facility already specified one) --daemon my[local1]-vpn-daemon --daemon [local1]my-vpn-daemon --daemon my-vpn-daemon[local1] Daemonize, use "my-vpn-daemon" for the syslog progname. Use the local1 syslog facility (unless --syslog-facility already specified one)Uh. Feature-NAK - especially the last one really makes me want to throw up. Using [] as part of an option string is a bad idea, because it is commonly used for optional arguments (as you saw in David's question), and intermixing option argument "A" and "B" in the same string is not going to help anyone. If we take this at all - and I have reservations, because the amount of code it brings in is quite significant - it should not touch the --daemon command line. It brings it's own config statement (--syslog-facility), so there really is no reason to mess up the syntax for --daemon. Also, --syslog-facility should be more clean, like --sysloc-facility facility [progname] with "progname" actually being *optional*, not "marked by using rectangular brackets". [..]Init scripts can be adjusted. Obscure code inside OpenVPN stays foreverAs I explain above, this is to make it possible to specify a facility name in cases where the system init script uses --daemon before the user can put a --syslog-facility. --syslog-facility must precede --daemon or --syslog because the latter two immediately switch logging to syslog; they are what consume the facility code when they call openlog().(guess why the developers have so little time? right, maintaining stuff that someone else considered "a clever hack" 5 years ago). gert
smime.p7s
Description: S/MIME Cryptographic Signature
