On Tue, Aug 3, 2021, 11:36 Ulrich Windl <Ulrich.Windl@xxxxxxxxxxxxxxxxxxxx> wrote:
Hi!
I'm not into socket units, but maybe one one is could have a look at the unit files shown in https://serverfault.com/q/1070757/407952. Those files are from a commercial product (Backup Software)
I wonder if these look OK, or (what I think) are quite incomplete.
Looks mostly OK to me.
A dependency on network is not needed for sockets with wildcard binds (i.e. just port, no specific IP address). These will always succeed.
The service doesn't need a general network dep either, if you've received a connection then the network is usually already up.
Dependency on local-fs is implied in DefaultDependencies, *I think,* so you don't need to explicitly add it. Although maybe RequiresMountsFor=/var/opt/omni would be useful.
I wouldn't expect the vendor to include a dep on remote-fs by default. It's up to the admin to decide this – locally adding dependencies is quite a bit easier than locally removing unwanted ones (I really wish unit files had a -= operator, but.) Having things depend on NFS when they don't actually need it is annoying.
On the other hand, I would *remove* some options – that PIDFile= is redundant in general, and just outright makes no sense for a templated@ service to use a non-templated pidfile path. The KillMode=process is also somewhat dubious.
PartOf=omni.service doesn't seem like it would work, as an Accept=yes socket will indeed use omni@.service (one instance per connection, not a single long-running daemon – this very much looks like a direct conversion from inetd-based service to systemd). I'd remove it here.
Similarly, the WantedBy= in omni@.service is nonsensical – inetd per-connection services can't be started on boot, there's no connection to attach them to... This kind of service has to be started by the socket, not through `systemctl enable`.
