[libvirt] [PATCH] lxc: Prevent shutting down the host
When the container has the same '/dev' mount as host (no chroot), calling domainShutdown(WithFlags) shouldn't shutdown the host it is running on. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- This is also valid for 1.0.[23]-maint branches, so in case this gets ACK'd I'll either send a follow-up for those or push it there as well (if the ACK says so). src/lxc/lxc_driver.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8603078..ba14db7 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_driver.c: linux container driver functions @@ -2778,13 +2778,19 @@ lxcDomainShutdownFlags(virDomainPtr dom, virLXCDriverPtr driver = dom->conn->privateData; virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; + virDomainFSDefPtr root; char *vroot = NULL; int ret = -1; - int rc; + int rc = 0; + bool methodSignal; + bool methodInitctl; virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL | VIR_DOMAIN_SHUTDOWN_SIGNAL, -1); + methodSignal = !!(flags & VIR_DOMAIN_SHUTDOWN_SIGNAL); + methodInitctl = !!(flags & VIR_DOMAIN_SHUTDOWN_INITCTL); + lxcDriverLock(driver); vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); lxcDriverUnlock(driver); @@ -2798,6 +2804,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, } priv = vm->privateData; + root = virDomainGetRootFilesystem(vm->def); if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -2817,27 +2824,31 @@ lxcDomainShutdownFlags(virDomainPtr dom, goto cleanup; } - if (flags == 0 || - (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { - if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, - vroot)) < 0) { + if (root && root->src) { + if (flags == 0) + methodSignal = methodInitctl = true; + } else if (methodInitctl) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Cannot shutdown container using initctl " + "without separated namespace")); + goto cleanup; + } else { + methodSignal = true; + } + + if (methodInitctl) { + rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, vroot); + if (rc < 0) goto cleanup; - } - if (rc == 0 && flags != 0 && - ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { + if (rc == 0 && !methodSignal) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Container does not provide an initctl pipe")); goto cleanup; } - } else { - rc = 0; } - - if (rc == 0 && - (flags == 0 || - (flags & VIR_DOMAIN_SHUTDOWN_SIGNAL))) { - if (kill(priv->initpid, SIGTERM) < 0 && - errno != ESRCH) { + if (rc == 0 && methodSignal) { + ret = kill(priv->initpid, SIGTERM); + if (ret < 0 && errno != ESRCH) { virReportSystemError(errno, _("Unable to send SIGTERM to init pid %llu"), (unsigned long long)priv->initpid); -- 1.8.1.5
On Thu, Mar 21, 2013 at 04:10:45PM +0100, Martin Kletzander wrote:
When the container has the same '/dev' mount as host (no chroot), calling domainShutdown(WithFlags) shouldn't shutdown the host it is running on.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- This is also valid for 1.0.[23]-maint branches, so in case this gets ACK'd I'll either send a follow-up for those or push it there as well (if the ACK says so).
src/lxc/lxc_driver.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-)
ACK, as a temporary measure. I think we need to make sure that /dev is always private for all LXC containers. That can wait for a more general refactoring of LXC filesystem setup though. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 03/22/2013 06:17 PM, Daniel P. Berrange wrote:
On Thu, Mar 21, 2013 at 04:10:45PM +0100, Martin Kletzander wrote:
When the container has the same '/dev' mount as host (no chroot), calling domainShutdown(WithFlags) shouldn't shutdown the host it is running on.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- This is also valid for 1.0.[23]-maint branches, so in case this gets ACK'd I'll either send a follow-up for those or push it there as well (if the ACK says so).
src/lxc/lxc_driver.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-)
ACK,
Thanks, I pushed it to master, how do you (or anyone else) feel about the maintenance branches with this patch?
as a temporary measure. I think we need to make sure that /dev is always private for all LXC containers. That can wait for a more general refactoring of LXC filesystem setup though.
All the containers should definitely have a private separated environment, even when no root mount is specified, I meant this as an interim fix. Martin
On Sat, Mar 23, 2013 at 5:21 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
On 03/22/2013 06:17 PM, Daniel P. Berrange wrote:
On Thu, Mar 21, 2013 at 04:10:45PM +0100, Martin Kletzander wrote:
When the container has the same '/dev' mount as host (no chroot), calling domainShutdown(WithFlags) shouldn't shutdown the host it is running on.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- This is also valid for 1.0.[23]-maint branches, so in case this gets ACK'd I'll either send a follow-up for those or push it there as well (if the ACK says so).
src/lxc/lxc_driver.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-)
ACK,
Thanks, I pushed it to master, how do you (or anyone else) feel about the maintenance branches with this patch?
You are welcome to push them into both but the rule of thumb is really that once a new release goes out the prior one gets no more love, unless someone steps up that it will be a stable series. The idea behind the maint branches is really to upstream what the downstreams do. e.g. We shipped broken Python bindings in 1.0.2 and every downstream that packaged 1.0.2 would have to carry a patch. But how do they find out they need to carry that patch if they don't follow the developer mailing list closely. -- Doug Goldstein
On 03/23/2013 09:25 PM, Doug Goldstein wrote:
On Sat, Mar 23, 2013 at 5:21 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
On 03/22/2013 06:17 PM, Daniel P. Berrange wrote:
On Thu, Mar 21, 2013 at 04:10:45PM +0100, Martin Kletzander wrote:
When the container has the same '/dev' mount as host (no chroot), calling domainShutdown(WithFlags) shouldn't shutdown the host it is running on.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- This is also valid for 1.0.[23]-maint branches, so in case this gets ACK'd I'll either send a follow-up for those or push it there as well (if the ACK says so).
src/lxc/lxc_driver.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-)
ACK,
Thanks, I pushed it to master, how do you (or anyone else) feel about the maintenance branches with this patch?
You are welcome to push them into both but the rule of thumb is really that once a new release goes out the prior one gets no more love, unless someone steps up that it will be a stable series. The idea behind the maint branches is really to upstream what the downstreams do. e.g. We shipped broken Python bindings in 1.0.2 and every downstream that packaged 1.0.2 would have to carry a patch. But how do they find out they need to carry that patch if they don't follow the developer mailing list closely.
I wasn't sure, but since this is not such a huge deal, I will keep it as is. Thanks for the info, now I know for next time ;) Have a nice day, Martin
participants (3)
-
Daniel P. Berrange -
Doug Goldstein -
Martin Kletzander