Hi Simon,
On Thu, Nov 9, 2017 at 3:33 AM, Simon Rozman <si...@rozman.si> wrote:
> Hi,
>
> > But then making the variable static just to keep a valid pointer beyond
> the
> > current block local looks like a kludge. For me seeing static applied to
> a
> > variable scoped to a block is just confusing and unusual style. Think of
> this: if
> > you remove that static the code may still build and even work on some
> > compilers depending on optimization level etc. and mysteriously fail on
> some
> > occasions. From that one could either conclude a static qualifier is
> required
> > her or the variable is wrongly scoped. I think the latter conclusion is
> the 'right'
> > one and static is a misuse.
> >
> > As for the combination 'static const ...', it has a place and that is
> for constants
> > defined outside functions to limit the scope of an otherwise global
> const to
> > that of the compilation unit.
> >
> > It may be just me.
>
> Not necessarily. It may be just *me*. :)
>
> Anyway, you got me convinced and I shall move those structs from data
> segment to stack in the next version of the patch.
Glad that I don't have to invent an Acked-with-reservations: tag :)
>
> > I like it: using "OpenVPN" as a kind of namespace also avoids name
> collision
> > with any other 'foobar' installed in HKLM\Software\foobar
>
> Exactly, introducing instances is all about avoiding namespace collisions.
>
> > Not sure I follow, but will wait for the patch. I suppose naming
> additional
> > instances as, say, 'foo' using the official package and have service
> pipe, exit
> > event etc named after it and registry keys taken from HKLM\Software\foo
> > will still work.
>
> In the v1 and v2 patches, yes. But in v3 patch, the PACKAGE/PACKAGE_NAME
> is always used as the prefix/namespace. This way we impose the "OpenVPN"
> branding to all instances by compile-time default. In v3, people can't take
> the interactive service and make it disguise as the
> "Microsoft\Office\Automatic Update".
>
> The v3 made instancing less flexible on purpose.
>
> If you disagree, I can revert to v1/2 behaviour.
>
Looked through v3 changes --- either approach (v2 or v3) is fine.
>
> > In fact I like the current patch in that it allows installing even the
> official
> > instance as a named one with name = OpenVPN and thus decouples
> > automatic and interactive services. That makes it easy to avoid
> installing the
> > legacy service, which I think no one uses anymore.
>
> Ouch. *We* would use the automatic service if it was coded slightly
> different. I didn't know the legacy automatic service is on the obsolete
> list yet.
>
The automatic service per-se will continue to be there. But now its served
by the C#/.NET version (https://github.com/OpenVPN/openvpnserv2) that
is included in the distribution as openvpnserv2.exe We kept the old service
renamed from openvpnservice to openvpnservicelegacy as a precaution.
> Perhaps, a true replacement for the automatic service could be to make the
> openvpn.exe itself to be able to install and run as a Windows service
> (--daemon for Windows):
> - The SCM would restart it, should openvpn.exe terminate or crash.
> - Separate openvpn.exe service for each ovpn configuration, allowing
> admins to manage them individually.
> - Linux like behaviour, where you run each tunnel as a separate Systemd
> daemon.
>
Essentially that's what the new automatic service does except for the
ability to
selectively control (start/stop) individual configs. I too would like to
see that
functionality but .NET puts me to sleep.
>
> (That is exactly what I'd need for our office. Right now we are wrapping
> openvpn.exe inside a NSSM service helper. Would be nice to get that feature
> out-of-the-box.)
>
Right now using nssm is somewhat more versatile than openvpnserv2 but the
latter
does watch the process, restarts if needed etc., which the original
automatic
service was not designed to do. At that time we had debated about nssm vs
openvpnserv2.exe and the latter won because of some packaging convenience..
Personally I would like an automatic service that runs with limited
privileges
(e.g., as LocalService) which uses the interactive service to start
instances,
watches over the process and handles individual configs independently.
Refactoring the code in automatic.c may be a way to achieve that.
> Back to the interactive service... According to the latest [PATCH( v3
> missing)], one would need to start the default instance of the interactive
> service only using the following service command line:
>
> openvpnserv.exe -instance interactive ""
>
Yes that would work in a pinch and, once installed, the user need not worry
about the oddity of that command line. But ignore my comment about running
the default instance as a named one. That's not really necessary.
All said, v4 could be based on v3 or v2. Now try to convince Gert that this
belongs to 2.4 :)
Regards,
Selva
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
