[systemd-devel] Revisiting the "ExecRestart" issue

[Brandon Black]

On Fri, Mar 28, 2014 at 12:12 PM, Brandon Black <blblack at gmail.com> wrote:
>
>   Given where things are at today, as best I can tell my best bet is to go
> down that sort of road, though, and try to clone over the cgroups
> memberships manually somehow during an ExecReload= command for this restart
> (even though it really is a restart), and leaving true reloads (SIGHUP to a
> running daemon) to be done from outside systemd.
>

I've done some experimenting this evening (on a Fedora 20 system w/
systemd-208),
playing with methods of MAINPID notification and how to coerce
ExecReloadinto letting me do an overlapped restart.  The result is
that I can make it
work, but it's hacky.  The main thing that bothers me about it is that the
mechanisms probably aren't officially supported interfaces and my methods
will randomly fail on a future version of systemd (or a
differently-configured distro).

To recap my results: there were primarily two things in the way of naively
using ExecReload to trigger gdnsd's overlapped restart:

1) gdnsd wants to use sd_notifyf() to indicate the MAINPID switch in the
new daemon, which is a descendant of the ExecReload process.  The
ExecReloadprocess doesn't get a copy of $NOTIFY_SOCKET even with
NotifyAccess=all.  So I hacked around that by having the daemon set
$NOTIFY_SOCKET for itself, to the value "@/org/freedesktop/systemd1/notify",
which seems semi-standard for the moment.

2) ExecReload control processes can't become the MAINPID even after
notification because they're not in the correct cgroup (or subgroup, or
whatever it is that's special about most control procs), unlike
Start'scontrol process, which is in the right
cgroup for its descendants to become MAINPID successfully.  This was hacked
around by grabbing the basic unit name from sd_pid_get_unit() (let's call
the result "$U") and then writing our pid to "/sys/fs/cgroup/systemd
/system.slice/$U/cgroup.procs" from the new daemon before it drops root
privs and later notifies about the MAINPID switch.

(And of course, re-purposing ExecReload isn't ideal in the first place.
 It's semantically wrong and it wastes the reload verb, forcing actual
reload actions to need to happen from outside of systemctl)

The resulting commit (which is off in a testing branch of a development
branch for now, there's plenty of time to work out alternate solutions) is
here:

https://github.com/blblack/gdnsd
/commit/17a40b0483da7d072912169e832df31d69349440

>From going through this exercise, I think I can refine my feature-plea to
this: What would be ideal (well, from the limited perspective of making
things easier for this one daemon) would be an ExecRestart (or whatever)
verb which acts almost exactly like ExecStart (correct control group for
final daemon, gets $NOTIFY_SOCKET), but has its own separate command string
and doesn't pre-check that the service is currently considered inactive.  I
don't think it would be too hard to write such a patch, but my first
concern is whether such a patch is even remotely likely to be accepted, or
whether there are better alternatives (other patches that could be made, or
perhaps better interfaces I'm unaware of in the current code can obviate
the hacky stuff above without any patching).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20140401/723d4af0/attachment.html>