Re: [PATCH] Fix reboot command for LXC containers

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(a)gmx.de&gt; > --- > 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.