[libvirt] [PATCH] lxc: Fix reboot via initctl

virInitctlSetRunLevel() returns 0 only if ended up doing nothing, 1 if it actually succeeded. Let's check for the error condition. Without this, a successful reboot would be treated as a failure and the LXC driver will proceed sending a TERM signal instead, effectively cancelling the shutdown. Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- src/lxc/lxc_domain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 51a9fd36eb..1dc2d0d4cf 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -457,7 +457,9 @@ lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED, data->st[i].st_ino == cont_sb.st_ino) continue; - return virInitctlSetRunLevel(fifo, data->runlevel); + if (virInitctlSetRunLevel(fifo, data->runlevel) == -1) + return -1; + return 0; } /* If no usable fifo was found then declare success. Caller -- 2.21.0

On Thu, Jul 04, 2019 at 11:04:15AM +0200, Lubomir Rintel wrote:
virInitctlSetRunLevel() returns 0 only if ended up doing nothing, 1 if it actually succeeded. Let's check for the error condition.
Without this, a successful reboot would be treated as a failure and the LXC driver will proceed sending a TERM signal instead, effectively cancelling the shutdown.
Can you elaborate on this a bit as I'm not seeing a way for this to happen with the current code. - virInitctlSetRunLevel returns 0 if no init was found, 1 if it was successful, -1 on error - lxcDomainInitctlCallback checks whether init exists and is not the same as the host init a then calls virInitctlSetRunLevel. Due to the checks virInitctlSetRunLevel can only ever return 1 or -1 in this scenario. 0 will never happen - virLXCDomainSetRunlevel calls lxcDomainInitctlCallback via virProcessRunInMountNamespace and honours the return status. So virLXCDomainSetRunlevel shoudl return -1 on error, 1 on success - lxcDomainShutdownFlags calls virLXCDomainSetRunlevel and saves its return value to a variable 'rc' - If 'rc' is -1 then it sends SIGTERM, otherwise it just returns 0 indicating success So I'm not following how we can end up successing talking to init, and then also sending SIGTERM. In fact there is a different problem here I believe. We should be sending SIGTERM when 'rc' is 0 not -1, because rc==0 is what indicates no init was present.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- src/lxc/lxc_domain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 51a9fd36eb..1dc2d0d4cf 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -457,7 +457,9 @@ lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED, data->st[i].st_ino == cont_sb.st_ino) continue;
- return virInitctlSetRunLevel(fifo, data->runlevel); + if (virInitctlSetRunLevel(fifo, data->runlevel) == -1) + return -1; + return 0; }
/* If no usable fifo was found then declare success. Caller -- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé
-
Lubomir Rintel