Re: [PATCH 1/2] virnetdaemon: Introduce virNetDaemonQuitExecRestart

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 3/10/21 5:37 PM, Peter Krempa wrote:
> Recent changes which meant to fix daemon shutdown broke the exec-restart
> capability of virtlogd and virtlockd, since the code actually closed all
> the sockets and shut down all the internals.
>
> Add virNetDaemonQuitExecRestart, which requests a shutdown of the
> process, but keeps all the services open and registered since they are
> preserved across the restart.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
>   src/libvirt_remote.syms |  1 +
>   src/rpc/virnetdaemon.c  | 19 +++++++++++++++++++
>   src/rpc/virnetdaemon.h  |  1 +
>   3 files changed, 21 insertions(+)
>
> diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
> index 3cd84a0854..605271bcb2 100644
> --- a/src/libvirt_remote.syms
> +++ b/src/libvirt_remote.syms
> @@ -85,6 +85,7 @@ virNetDaemonNew;
>   virNetDaemonNewPostExecRestart;
>   virNetDaemonPreExecRestart;
>   virNetDaemonQuit;
> +virNetDaemonQuitExecRestart;
>   virNetDaemonRemoveShutdownInhibition;
>   virNetDaemonRun;
>   virNetDaemonSetShutdownCallbacks;
> diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
> index 327540c4b4..cac60ef488 100644
> --- a/src/rpc/virnetdaemon.c
> +++ b/src/rpc/virnetdaemon.c
> @@ -76,6 +76,7 @@ struct _virNetDaemon {
>       bool quit;
>       bool finished;
>       bool graceful;
> +    bool execRestart;
>
>       unsigned int autoShutdownTimeout;
>       size_t autoShutdownInhibitions;
> @@ -857,6 +858,10 @@ virNetDaemonRun(virNetDaemonPtr dmn)
>
>           virHashForEach(dmn->servers, daemonServerProcessClients, NULL);
>
> +        /* don't shutdown services when performing an exec-restart */
> +        if (dmn->quit && dmn->execRestart)

1:  you'd check only for @execRestart here.
> +            goto cleanup;
> +
>           if (dmn->quit && dmn->finishTimer == -1) {
>               virHashForEach(dmn->servers, daemonServerClose, NULL);
>               if (dmn->shutdownPrepareCb && dmn->shutdownPrepareCb() < 0)
> @@ -912,6 +917,20 @@ virNetDaemonQuit(virNetDaemonPtr dmn)
>       virObjectUnlock(dmn);
>   }
>
> +
> +void
> +virNetDaemonQuitExecRestart(virNetDaemonPtr dmn)
> +{
> +    virObjectLock(dmn);
> +
> +    VIR_DEBUG("Exec-restart requested %p", dmn);
> +    dmn->quit = true;
> +    dmn->execRestart = true;

I think you don't need to set @quit flag, if [1].

> +
> +    virObjectUnlock(dmn);
> +}
> +
> +

Michal