[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Quoting Daniel Veillard (veillard redhat com):
> On Fri, May 29, 2009 at 04:42:54PM -0500, Serge E. Hallyn wrote:
> > Quoting Ryota Ozaki (ozaki ryota gmail com):
> > > On Fri, May 29, 2009 at 9:20 PM, Daniel Veillard <veillard redhat com> 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 :-)

No, I'm not sure how it should be handled.  Some ugly autoconf thing?

Maybe PR_CAPBSET_DROP should get hand-defined, and the code at runtime
handles its failure?  I'll take a look next week.  Of course in the
meantime this patch is better than nothing  :)

thanks,
-serge


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]