Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

> -----Original Message----- > From: Richard Weinberger [mailto:richard@nod.at] > Sent: Friday, March 20, 2015 1:41 AM > To: Daniel P. Berrange > Cc: Chen, Hanxiao/陈 晗霄; libvir-list(a)redhat.com > Subject: Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns > but disable netns > > Am 19.03.2015 um 18:28 schrieb Daniel P. Berrange: > > On Thu, Mar 19, 2015 at 06:04:57PM +0100, Richard Weinberger wrote: > >> Am 19.03.2015 um 17:58 schrieb Daniel P. Berrange: > >>> On Thu, Mar 19, 2015 at 05:54:32PM +0100, Richard Weinberger wrote: > >>>> Am 11.03.2015 um 10:36 schrieb Richard Weinberger: > >>>>> Am 11.03.2015 um 03:30 schrieb Chen, Hanxiao: > >>>>>>>> @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool > userns_enabled) > >>>>>>>> bool bindOverReadonly; > >>>>>>>> virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i]; > >>>>>>>> > >>>>>>>> + /* When enable userns but disable netns, kernel will > >>>>>>>> + * forbid us doing a new fresh mount for sysfs. > >>>>>>>> + * So we had to do a bind mount for sysfs instead. > >>>>>>>> + */ > >>>>>>>> + if (userns_enabled && netns_disabled && > >>>>>>>> + STREQ(mnt->src, "sysfs")) { > >>>>>>>> + if (VIR_STRDUP(mnt_src, "/sys") < 0) { > >>>>>>>> + goto cleanup; > >>>>>>>> + } > >>>>>>> > >>>>>>> This is clearly broken and looks very untested to me. > >>>>>>> > >>>>>> It's broken now. > >>>>>> But when I submitted this patch last year, it's not. > >>>>> > >>>>> Are you sure? > >>>>> Just built libvirt v1.2.6-222-ga86b621, head is > >>>>> commit a86b6215a74b1feb2667204e214fbfd2f7decc5c > >>>>> Author: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com&gt; > >>>>> Date: Mon Jul 14 18:01:51 2014 +0800 > >>>>> > >>>>> LXC: create a bind mount for sysfs when enable userns but disable netns > >>>>> > >>>>> /sys is still an empty directory but as at this time (most likely due to another > bug) > >>>>> libvirt was able to create /sys/fs/cgroup and mounted groups there. > >>>>> But no sysfs at all is at /sys. > >>>>> > >>>>> I mean, how is this supposed to work? You bind mount /sys over /sys... > >>>> > >>>> Any further comments on that? > >>> > >>> It just looks impossible for it to work in this way > >> > >> That's also my impression. > >> > >> Therefore containers without their own network namespace currently don't work > >> and have never worked as expected. > > > > No, it is only a problem if userns is used. If userns is not used then > > they do work > > Agreed. > That's what I tried to do. Sorry for my mistake. > >> Shall we revert commit a86b6215a74b and try to bind mount > >> before the pivot_root()? > > > > Not sure if that works with userns is active either. > > Fact is that commit a86b6215a74 is broken. > We could also refuse to create container with userns enabled but netns disabled... > I think we should refuse it too, rather than do something to work around. Dan, what's your opinion?