[PATCH 02/26] remote: notify systemd when reloading config

Daniel P. Berrangé posted 26 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH 02/26] remote: notify systemd when reloading config
Posted by Daniel P. Berrangé 1 year, 1 month ago
Switch to the 'notify-reload' service type and send notifications to
systemd when reloading configuration.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/remote/libvirtd.service.in | 2 +-
 src/remote/remote_daemon.c     | 2 ++
 src/virtd.service.in           | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 250b4a6fc3..b0a062e885 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -26,7 +26,7 @@ After=xencommons.service
 Conflicts=xendomains.service
 
 [Service]
-Type=notify
+Type=notify-reload
 Environment=LIBVIRTD_ARGS="--timeout 120"
 EnvironmentFile=-@initconfdir@/libvirtd
 ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 1d079c7e4b..d44a365000 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -452,9 +452,11 @@ static void daemonReloadHandlerThread(void *opaque G_GNUC_UNUSED)
     virHookCall(VIR_HOOK_DRIVER_DAEMON, "-",
                 VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL);
 
+    virSystemdNotifyReload();
     if (virStateReload() < 0) {
         VIR_WARN("Error while reloading drivers");
     }
+    virSystemdNotifyReady();
 
     /* Drivers are initialized again. */
     g_atomic_int_set(&driversInitialized, 1);
diff --git a/src/virtd.service.in b/src/virtd.service.in
index 651a8d82d7..7ffb77e339 100644
--- a/src/virtd.service.in
+++ b/src/virtd.service.in
@@ -15,7 +15,7 @@ After=dbus.service
 After=apparmor.service
 
 [Service]
-Type=notify
+Type=notify-reload
 Environment=@SERVICE@_ARGS="--timeout 120"
 EnvironmentFile=-@initconfdir@/@service@
 ExecStart=@sbindir@/@service@ $@SERVICE@_ARGS
-- 
2.47.1
Re: [PATCH 02/26] remote: notify systemd when reloading config
Posted by Peter Krempa 1 year ago
On Wed, Jan 08, 2025 at 19:42:35 +0000, Daniel P. Berrangé wrote:
> Switch to the 'notify-reload' service type and send notifications to
> systemd when reloading configuration.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/remote/libvirtd.service.in | 2 +-
>  src/remote/remote_daemon.c     | 2 ++
>  src/virtd.service.in           | 2 +-
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
> index 250b4a6fc3..b0a062e885 100644
> --- a/src/remote/libvirtd.service.in
> +++ b/src/remote/libvirtd.service.in
> @@ -26,7 +26,7 @@ After=xencommons.service
>  Conflicts=xendomains.service
>  
>  [Service]
> -Type=notify
> +Type=notify-reload

^^^

>  Environment=LIBVIRTD_ARGS="--timeout 120"
>  EnvironmentFile=-@initconfdir@/libvirtd
>  ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index 1d079c7e4b..d44a365000 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -452,9 +452,11 @@ static void daemonReloadHandlerThread(void *opaque G_GNUC_UNUSED)
>      virHookCall(VIR_HOOK_DRIVER_DAEMON, "-",
>                  VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL);
>  
> +    virSystemdNotifyReload();

The man page says for 'notify-reload':

           •   Behavior of notify-reload is similar to notify, with one
               difference: the SIGHUP UNIX process signal is sent to the
               service's main process when the service is asked to reload and
               the manager will wait for a notification about the reload being
               finished.

               When initiating the reload process the service is expected to
               reply with a notification message via sd_notify(3) that contains
               the "RELOADING=1" field in combination with "MONOTONIC_USEC="
               set to the current monotonic time (i.e.  CLOCK_MONOTONIC in
               clock_gettime(2)) in μs, formatted as decimal string. Once
               reloading is complete another notification message must be sent,
               containing "READY=1". Using this service type and implementing
               this reload protocol is an efficient alternative to providing an
               ExecReload= command for reloading of the service's
               configuration.

So IIUC we need to be also sending MONOTONIC_USEC= in addition to
RELOADING=1 in order to do the handshake for 'notify-reload' properly.



>      if (virStateReload() < 0) {
>          VIR_WARN("Error while reloading drivers");
>      }
> +    virSystemdNotifyReady();
>  
>      /* Drivers are initialized again. */
>      g_atomic_int_set(&driversInitialized, 1);
> diff --git a/src/virtd.service.in b/src/virtd.service.in
> index 651a8d82d7..7ffb77e339 100644
> --- a/src/virtd.service.in
> +++ b/src/virtd.service.in
> @@ -15,7 +15,7 @@ After=dbus.service
>  After=apparmor.service
>  
>  [Service]
> -Type=notify
> +Type=notify-reload
>  Environment=@SERVICE@_ARGS="--timeout 120"
>  EnvironmentFile=-@initconfdir@/@service@
>  ExecStart=@sbindir@/@service@ $@SERVICE@_ARGS
> -- 
> 2.47.1
> 
Re: [PATCH 02/26] remote: notify systemd when reloading config
Posted by Daniel P. Berrangé 1 year ago
On Thu, Jan 30, 2025 at 12:22:52PM +0100, Peter Krempa wrote:
> On Wed, Jan 08, 2025 at 19:42:35 +0000, Daniel P. Berrangé wrote:
> > Switch to the 'notify-reload' service type and send notifications to
> > systemd when reloading configuration.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/remote/libvirtd.service.in | 2 +-
> >  src/remote/remote_daemon.c     | 2 ++
> >  src/virtd.service.in           | 2 +-
> >  3 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
> > index 250b4a6fc3..b0a062e885 100644
> > --- a/src/remote/libvirtd.service.in
> > +++ b/src/remote/libvirtd.service.in
> > @@ -26,7 +26,7 @@ After=xencommons.service
> >  Conflicts=xendomains.service
> >  
> >  [Service]
> > -Type=notify
> > +Type=notify-reload
> 
> ^^^
> 
> >  Environment=LIBVIRTD_ARGS="--timeout 120"
> >  EnvironmentFile=-@initconfdir@/libvirtd
> >  ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
> > diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> > index 1d079c7e4b..d44a365000 100644
> > --- a/src/remote/remote_daemon.c
> > +++ b/src/remote/remote_daemon.c
> > @@ -452,9 +452,11 @@ static void daemonReloadHandlerThread(void *opaque G_GNUC_UNUSED)
> >      virHookCall(VIR_HOOK_DRIVER_DAEMON, "-",
> >                  VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL);
> >  
> > +    virSystemdNotifyReload();
> 
> The man page says for 'notify-reload':
> 
>            •   Behavior of notify-reload is similar to notify, with one
>                difference: the SIGHUP UNIX process signal is sent to the
>                service's main process when the service is asked to reload and
>                the manager will wait for a notification about the reload being
>                finished.
> 
>                When initiating the reload process the service is expected to
>                reply with a notification message via sd_notify(3) that contains
>                the "RELOADING=1" field in combination with "MONOTONIC_USEC="
>                set to the current monotonic time (i.e.  CLOCK_MONOTONIC in
>                clock_gettime(2)) in μs, formatted as decimal string. Once
>                reloading is complete another notification message must be sent,
>                containing "READY=1". Using this service type and implementing
>                this reload protocol is an efficient alternative to providing an
>                ExecReload= command for reloading of the service's
>                configuration.
> 
> So IIUC we need to be also sending MONOTONIC_USEC= in addition to
> RELOADING=1 in order to do the handshake for 'notify-reload' properly.

Opps, yes, well spotted.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|