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.

>> 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...