Re: [libvirt] [PATCH] lxc: hoist supplemental group detection before clone

On 07/13/2013 01:35 AM, Eric Blake wrote: > Commit 75c1256 states that virGetGroupList must not be called > between fork and exec, then commit ee777e99 promptly violated > that for lxc. Hoist the group detection to occur before clone. > > * src/lxc/lxc_container.c (__lxc_child_argv): Add members. > (lxcContainerSetID): Adjust signature. > (lxcContainerChild, lxcContainerStart): Adjust callers. > > Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; > --- > src/lxc/lxc_container.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c > index 940233b..50aaa36 100644 > --- a/src/lxc/lxc_container.c > +++ b/src/lxc/lxc_container.c > @@ -106,6 +106,8 @@ struct __lxc_child_argv { > char **ttyPaths; > size_t nttyPaths; > int handshakefd; > + gid_t *groups; > + int ngroups; > }; > > static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, > @@ -349,26 +351,20 @@ int lxcContainerWaitForContinue(int control) > * > * Returns 0 on success or -1 in case of error > */ > -static int lxcContainerSetID(virDomainDefPtr def) > +static int > +lxcContainerSetID(virDomainDefPtr def, gid_t* groups, int ngroups) > { > - gid_t *groups; > - int ngroups; > - > /* Only call virSetUIDGID when user namespace is enabled > * for this container. And user namespace is only enabled > * when nuidmap&ngidmap is not zero */ > > VIR_DEBUG("Set UID/GID to 0/0"); > - if (def->idmap.nuidmap && > - ((ngroups = virGetGroupList(0, 0, &groups) < 0) || this is a bug... ngroups is 0 here if virGetGroupList(,,) is greater than 0.. I personal think it has no bad affection if we don't set supplementary groups here.

Since we are not quite care about what the supplementary groups the root user of container belongs to. and if we want to set supplementary groups for the users of container, we should also make sure these supplementary groups is valid in the user namespace this container belongs to. setgroups will failed if we don't have proper <idmap><gid/></idmap> context.