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@xxxxxx> >> --- >> 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@xxxxxxxx. Best, Joachim
