Re: [libvirt] [PATCH] Fix a compilation problem with LXC drop capabilities

On Fri, May 29, 2009 at 04:42:54PM -0500, Serge E. Hallyn wrote: > Quoting Ryota Ozaki (ozaki.ryota(a)gmail.com): > > On Fri, May 29, 2009 at 9:20 PM, Daniel Veillard <veillard(a)redhat.com&gt; wrote: > > >  The lxcContainerDropCapabilities() function requires PR_CAPBSET_DROP > > > to be defined in order to compile, but it may not be defined in older > > > kernels. So I made the compilation of the core of the function > > > conditional, raise an error but still return 0 to not make the > > > container initialization fail. But I'm unsure, should we just fail and > > > return -1 if we can't drop capabilities instead ? > > > > I think it depends on applications. AFAIK, libvirt intends to support > > two types of applications; application workload isolation and > > virtual private servers. In the latter case, we MUST drop the capability > > and if it fails we have to fail booting a container as well. OTOH, in > > the former case, we might not need to fail booting. > > > > Nonetheless, I agree with the patch because old kernels that don't > > support PR_CAPBSET_DROP (they would be 2.6.24 or earlier) don't > > have enough facilities to support VPSs (e.g., they lacks sysfs, devpts, etc.). > > Therefore, with the old kernels we don't need to care much about the > > dropping-failed-but-booting-success case. > > Hmm, yeah but note that often userspace is out of date with respect to > "recent" new kernel-related defines. I do a lot of testing on a rhel > 5.3 partition with spanking-new kernels, so rare is the time that I > don't have to do > > #ifndef PR_CAPBSET_DROP > #define PR_CAPBSET_DROP 24 > #endif > > and same for clone flags (CLONE_NEWIPC), securebits, capabilities, > etc. > > So if the prctl(PR_CAPBSET_DROP) returns -ENOSYS then absolutely I > agree, but the patch just does > > #ifdef PR_CAPBSET_DROP > > which seems wrong. Well, if you have a better patch to suggest, fire :-)