Hi Carlo, Carlo Zancanaro skribis: > This is a relatively simple patch adding a replacement slot to > services in the Shepherd. When stopping a service, the replacement > slot is checked and, if it has a value, is used to upgrade the current > service. > > I've chosen to modify the existing service, rather than creating a new > one, but that was mostly because it was easier for me to implement > quickly, and I didn't have a huge amount of time. > > I'm hopeful that this, or something like it, can be used by GuixSD to > allow people to restart services after reconfiguring without rebooting > (or remembering to stop, reconfigure, start). Awesome! This is exactly what we need to address the problem. We’ll still want to be able to special-case things like nginx that can be “hot-replaced”, though. So perhaps, in addition to this patch on the Shepherd side, we’ll need extra stuff in (gnu services shepherd). For instance, the ‘actions’ field of could, by default, include an “upgrade” action that simply sets the ‘replacement’ slot. For nginx, we’d provide a custom “upgrade” action that does “nginx -s restart” or whatever it is that needs to be done. ‘guix system reconfigure’ would automatically invoke the ‘upgrade’ action for each new service. WDYT? > From e03290041c91813f1a301c7e9c4dbb9ee768b400 Mon Sep 17 00:00:00 2001 > From: Carlo Zancanaro > Date: Thu, 9 Aug 2018 22:30:38 +1000 > Subject: [PATCH] service: Add a replacement slot for delayed service > replacement. > > * modules/shepherd/service.scm (): Add replacement slot > (replace-service): New procedure. > (stop): Call replace-service after stopping a service. > * tests/replacement.sh: Add a test for it. > * Makefile.am (TESTS): Add the new test. > * doc/shepherd.texi (Slots of services): Document it. Overall LGTM. Some comments: > +(define (replace-service service) Please add a docstring. > + (let ((replacement (slot-ref service 'replacement))) > + (define (copy-slot! slot) > + (slot-set! service slot (slot-ref replacement slot))) > + (when replacement > + (copy-slot! 'provides) > + (copy-slot! 'requires) > + (copy-slot! 'respawn?) > + (copy-slot! 'start) > + (copy-slot! 'stop) > + (copy-slot! 'actions) > + (copy-slot! 'running) > + (copy-slot! 'docstring)) > + service)) Having a hardcoded list of slots sounds error-prone—surely we’ll forget to update it down the road. I wonder what else could be done. One option would be to grab the block asyncs and atomically replace the service in the ‘%services’ hash table. Then we only need to copy the ‘last-respawns’ slot to the new service, I believe. (This changes the object identity of the service but I think its OK.) Another option would be to use GOOPS tricks to iterate over the list of slots and have a list of slots *not* to copy. I’m not a big fan of this option, though. > +cat - > "$rconf"< +(let ((service (lookup-running 'test))) > + (slot-set! service 'replacement > + (make I wonder if we should let users fiddle with ‘replacement’ directly, or if we should provide a higher-level construct. For instance, ‘register-services’ could transparently set the ‘replacement’ field for services already registered instead of doing: (assert (null? (lookup-services (canonical-name new)))) Not sure if there are cases where this behavior would be undesirable, though. Thoughts? > +if test `cat $stamp` != "Goodbye"; then Nitpick: "`cat $stamp`". Thanks! Ludo’.