[PATCH] Fix reboot command for LXC containers

[Joachim Falk]

Hi Michal,

Am 02.12.21 um 16:02 schrieb 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.

it is on the current master. However, I had to resend it due to a spam bounce.
Maybe it got mangled by this process.

>
>>
>> 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 ^^^.

Thanks for the heads up. It is quite surprising that one can miss such obvious stuff.

>
>>      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.

I resend the patch with the required changes. However, you might need to check
your SPAM filter. Patch should be send from address joachim.falk at jfalk.de.

Best,

Joachim