[PATCH 5/9] cr: capabilities: define checkpoint and restore fns

Serge E. Hallyn serue at us.ibm.com
Tue Jun 2 07:23:53 PDT 2009


Quoting Andrew G. Morgan (morgan at kernel.org):
> On Mon, Jun 1, 2009 at 3:18 PM, Serge E. Hallyn <serue at us.ibm.com> wrote:
> > Quoting Andrew G. Morgan (morgan at kernel.org):
> >> On Mon, Jun 1, 2009 at 6:35 AM, Serge E. Hallyn <serue at us.ibm.com> wrote:
> >> >> > I'll put in a commented BUILD_BUG_ON like Alexey suggests - does that
> >> >> > suffice?
> >>
> >> I can't speak for other subsystems, but it seems to me as if for the
> >> capabilities, I'd want to create something like this in
> >> include/linux/capabilities.h
> >>
> >> typedef struct checkpoint_caps_s {
> >>    /* what goes in here is the capability code's business */
> >> } checkpoint_caps_t;
> >
> > Sigh - Did a patch this way, but the problem is userspace needs to be
> > able to parse the checkpoint image, so it needs to know what this struct
> > looks like.  So if I put it the struct definition
> > include/linux/capability.h, I run into a whole new set of problems
> > trying to compile a userspace program to do a sys_restart().
> 
> Does the user space app need to be able to modify the data in some
> way? It seems like embedding a length with the structure or something
> might simplify such a user space dependency.

Hmm, I suppose I could do something like define struct ckpt_capabilities
in capabilities.h, then in checkpoint_hdr.h do

struct ckpt_capabilities;
struct ckpt_cap_dummy {
	__u64 dummies[9];
};

struct ckpt_hdr_cred {
	...
	union {
		struct ckpt_capabilities r;
		struct ckpt_cap_dummy d;
	} caps;
};

with a BUILD_BUG_ON to ensure that sizeof(r)==sizeof(d).  Ugly, but
should suit everyone?

> > So I went part-way to what you suggested in the patchset I'm about to
> > send out (please see patch 6/8).  I think the caps code does look
> > nicer in this new version.
> 
> Better, but I remain concerned that the code looks hard to maintain
> when structured this way.

Why exactly?  Just having the struct defined in checkpoint_hdr.h?  Or
is there something else I'm unwittingly doing?

thanks,
-serge


More information about the Containers mailing list