[PATCH 1/2] remote_daemon: Set shutdown callbacks only after init is done

[Peter Krempa]

On Thu, Dec 09, 2021 at 15:43:55 +0100, Michal Privoznik wrote:
> The initialization of drivers happens in a separate thread.
> However, the main thread continues initialization and sets
> shutdown callbacks (virStateShutdownPrepare() and
> virStateShutdownWait()) even though the driver init thread is
> still running. This is dangerous because if the daemon decides to
> quit early (e.g. because SIGINT was delivered) the
> shutdownPrepare and shutdownWait callback are called over
> partially init drivers.
> 
> Set callbacks only after all drivers were initialized.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/218
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2027400
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/remote/remote_daemon.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index de43a54c2e..4e10f3ad23 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -626,6 +626,10 @@ static void daemonRunStateInit(void *opaque)
>  
>      driversInitialized = true;
>  
> +    virNetDaemonSetShutdownCallbacks(dmn,
> +                                     virStateShutdownPrepare,
> +                                     virStateShutdownWait);
> +

Okay so this placement ensures that the state shutdown code will only
ever be executed if the state was already initialized ...

>      /* Tie the non-privileged daemons to the session/shutdown lifecycle */
>      if (!virNetDaemonIsPrivileged(dmn)) {
>  
> @@ -1214,9 +1218,6 @@ int main(int argc, char **argv) {
>  #endif
>  
>      /* Run event loop. */
> -    virNetDaemonSetShutdownCallbacks(dmn,
> -                                     virStateShutdownPrepare,
> -                                     virStateShutdownWait);

... which wasn't always true here.

>      virNetDaemonRun(dmn);
>  
>      ret = 0;

So at this point it's still possible that the daemon will quit without
the callbacks being called, but tat was possible even before.

Based on the above and the fact that I wasn't able to reproduce the
crash:

Reviewed-by: Peter Krempa <pkrempa at redhat.com>