[PATCH] Fix reboot command for LXC containers

[Michal Prívozník]

On 12/1/21 22:38, Joachim Falk wrote:
> The virNetDaemonQuit(dmn) command in virLXCControllerSignalChildIO triggers an
> early close of all clients of lxc_controller. Here, libvirtd itself is a client
> of this controller, and the client connection is used to notify libvirtd if a
> reboot of the container is required. However, the client connection was closed
> before such a status could be sent to libvirtd. To fix this bug, we will
> immediately send the reboot or shutdown status of the container to libvirtd,
> and only after client disconnect will we trigger virNetDaemonQuit.
> 
> Fixes: #237
> Bug: https://gitlab.com/libvirt/libvirt/-/issues/237
> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991773
> Signed-off-by: Joachim Falk <joachim.falk at gmx.de>
> ---
>  src/lxc/lxc_controller.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)

This patch is broken - I'm unable to apply it. Did you hand edit it
perhaps? Also please make sure that the patch is rebased onto current
master.

> 
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index 444f728af4..75107822ba 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -894,8 +894,10 @@ static void virLXCControllerClientCloseHook(virNetServerClient *client)
>      virLXCController *ctrl = virNetServerClientGetPrivateData(client);
>       VIR_DEBUG("Client %p has closed", client);
> -    if (ctrl->client == client)
> +    if (ctrl->client == client) {
>          ctrl->client = NULL;
> +        VIR_DEBUG("Client has gone away");
> +    }
>      if (ctrl->inShutdown) {
>          VIR_DEBUG("Arm timer to quit event loop");
>          virEventUpdateTimeout(ctrl->timerShutdown, 0);
> @@ -1006,6 +1008,9 @@ static int lxcControllerClearCapabilities(void)
>  static bool wantReboot;
>  static virMutex lock = VIR_MUTEX_INITIALIZER;
>  +static int
> +virLXCControllerEventSendExit(virLXCController *ctrl,
> +                              int exitstatus);
>   static void virLXCControllerSignalChildIO(virNetDaemon *dmn,
>                                            siginfo_t *info G_GNUC_UNUSED,
> @@ -1015,10 +1020,10 @@ static void virLXCControllerSignalChildIO(virNetDaemon *dmn,
>      int ret;
>      int status;
>  +    ignore_value(dmn);

No. We use G_GNUC_UNUSED, just like you can see in this context ^^^.

>      ret = waitpid(-1, &status, WNOHANG);
>      VIR_DEBUG("Got sig child %d vs %lld", ret, (long long)ctrl->initpid);
>      if (ret == ctrl->initpid) {
> -        virNetDaemonQuit(dmn);
>          virMutexLock(&lock);
>          if (WIFSIGNALED(status) &&
>              WTERMSIG(status) == SIGHUP) {
> @@ -1026,6 +1031,7 @@ static void virLXCControllerSignalChildIO(virNetDaemon *dmn,
>              wantReboot = true;
>          }
>          virMutexUnlock(&lock);
> +        virLXCControllerEventSendExit(ctrl, wantReboot ? 1 : 0);
>      }
>  }
>  @@ -2276,9 +2282,10 @@ virLXCControllerEventSendExit(virLXCController *ctrl,
>          VIR_DEBUG("Waiting for client to complete dispatch");
>          ctrl->inShutdown = true;
>          virNetServerClientDelayedClose(ctrl->client);
> -        virNetDaemonRun(ctrl->daemon);
> +    } else {
> +        VIR_DEBUG("Arm timer to quit event loop");
> +        virEventUpdateTimeout(ctrl->timerShutdown, 0);
>      }
> -    VIR_DEBUG("Client has gone away");
>      return 0;
>  }
>  @@ -2430,8 +2437,6 @@ virLXCControllerRun(virLXCController *ctrl)
>       rc = virLXCControllerMain(ctrl);
>  -    virLXCControllerEventSendExit(ctrl, rc);
> -
>   cleanup:
>      VIR_FORCE_CLOSE(control[0]);
>      VIR_FORCE_CLOSE(control[1]);

Otherwise this approach looks reasonable.

Michal