[lxc-devel] call to setup_dev_symlinks with lxc.autodev

[Serge Hallyn]

Quoting Michael H. Warfield (mhw at WittsEnd.com):
> On Thu, 2014-04-17 at 11:14 +0200, William Dauchy wrote:
> > On Wed, Apr 16, 2014 at 9:15 PM, Michael H. Warfield <mhw at wittsend.com> wrote:
> > > What version of LXC?
> > 
> > 1.0.3
> > 
> > > What distro (template) and revision was installed in that container?
> > > What custom configuration changes have you made?
> > > Are you running with autodev=0 or autodev=1?
> > > It sounds like this "system disk" was created as an image file or
> > > physical disk.  Which and how was it created?
> > 
> > I'm not using any template. Everything made by myself since I'm using
> > lxc. The system disk is also generated my by own scripts.
> > Here is my config:
> > 
> > lxc.pivotdir = mnt
> > 
> > ## CONSOLES
> > lxc.tty = 4
> > lxc.pts = 8
> > 
> > # Capabilities
> > lxc.cap.drop = sys_boot sys_module sys_time
> > lxc.cap.drop = net_raw
> > lxc.cap.drop = mac_override audit_control mac_admin
> > 
> > # Memory
> > lxc.cgroup.memory.limit_in_bytes = 268435456
> > lxc.cgroup.memory.memsw.limit_in_bytes = 268435456
> > 
> > # devices
> > lxc.cgroup.devices.deny = a
> > lxc.cgroup.devices.allow = c 1:3 rwm # null
> > lxc.cgroup.devices.allow = c 1:5 rwm # zero
> > lxc.cgroup.devices.allow = c 1:7 rwm # full
> > lxc.cgroup.devices.allow = c 1:8 rwm # random
> > lxc.cgroup.devices.allow = c 1:9 rwm # urandom
> > lxc.cgroup.devices.allow = c 4:0 rwm # tty1
> > lxc.cgroup.devices.allow = c 4:1 rwm # tty2
> > lxc.cgroup.devices.allow = c 4:2 rwm # tty3
> > lxc.cgroup.devices.allow = c 4:3 rwm # tty4
> > lxc.cgroup.devices.allow = c 4:4 rwm # tty5
> > lxc.cgroup.devices.allow = c 4:5 rwm # tty6
> > lxc.cgroup.devices.allow = c 4:6 rwm # tty7
> > lxc.cgroup.devices.allow = c 4:7 rwm # tty8
> > lxc.cgroup.devices.allow = c 5:0 rwm # tty
> > lxc.cgroup.devices.allow = c 5:1 rwm # console
> > lxc.cgroup.devices.allow = c 5:2 rwm # ptmx
> > lxc.cgroup.devices.allow = c 136:* rwm # pts/*
> > 
> > lxc.utsname = cgroups0
> > 
> > lxc.rootfs = /var/lib/lxc/VM_X/rootfs
> > lxc.autodev = 0
> > lxc.kmsg = 0
> > 
> > lxc.console = /srv/X/log
> > 
> > lxc.cgroup.cpu.shares = 64
> > 
> > lxc.network.type = veth
> > lxc.network.name = eth0
> > lxc.network.link = dev
> > lxc.network.flags = up
> > lxc.network.hwaddr = 00:16:3e:f3:35:93
> > lxc.network.veth.pair = vethX.0
> > 
> > lxc.network.ipv4 = 10.20.7.236/21
> > lxc.network.ipv4.gateway = 10.20.7.254
> > lxc.mount.entry = /srv/X /var/lib/lxc/VM_X/rootfs/ none
> > ro,bind,nosuid,noatime,nodiratime 0 0
> > # this hook is just doing a `ls` in the /dev directory
> > lxc.hook.mount = /usr/bin/setup_dev.sh /srv/X/dev
> > 
> > 
> > > On Mon, 2014-04-14 at 15:18 +0200, William Dauchy wrote:
> > >> but in my case I need to manually access the /dev directory in order
> > >> to make it return EEXIST.
> > >
> > > I don't understand this statement.  What are you doing "in order to make
> > > it return EEXIST?"  Create the symlinks?  In your previous message, you
> > > said the 4 symlinks already exist.  Ok...  Then what does this "I need
> > > to manually access the /dev directory" mean?  What are you doing when
> > > you do?
> 
> > I was also quoting the code which is calling the `symlink` function
> > and should return EEXIST in order to not fail. The thing is I need to
> > access the directory /dev before starting (see lxc.hook.mount in my
> > config) to make the `symlink` function return EEXIST. That's a
> > workaround to make my container start.
> 
> Ok...  Now I see what you're doing.  Thank you.
> 
> When your initial scripts create the system disk image and populate
> the /dev directory, why are they not creating those symlinks.  They are
> required per the kernel's Documentation/devices.txt doc.  They're listed
> in the "Compulsory" section meaning your custom image is not in
> compliance with the Linux kernel documentation (i.e. broken).
> 
> > I also don't understand what is so difficult to get: it does not make
> > sense to have options to enable/disable creations of stuff in /dev
> > (kmsg/autodev), but not for these four links. That's what I need.
> 
> The reason we can't make it dependent on autodev is that there are known
> cases with autodev == 0 (CentOS and Oracle at the very least) where we
> need it and known cases with autodev == 1 where we need it as well
> (non-systemd init).  We can't make it a dependency.
> 
> If we ignore the error, we're allowing broken images that may result in
> other errors (which have already been reported to us).
> 
> In theory, I could argue that the only case where symlink is going to
> fail without an EEXIST error would be your case, and I could accept just
> ignoring the error does no actual harm and the caller will just have to
> accept errors that may occur elsewhere.  I'm not sure I'm happy with
> that choice, but I can go with it if that's what Serge and Dwight
> decide.  It's also the easiest patch to make.  :-)
> 
> We could add another option but the only case the option is covering is
> the case of a broken image with an incomplete /dev that is mounted ro so
> we can not correct the missing pieces in /dev.  That's not the choice I
> would make.  Seems like overkill to make something optional that's
> mandatory when there are already better fixes.
> 
> If you can't correct the deficiencies in the system disk images you have
> and you have to have an image where those mandatory symlinks do not
> exist, then I would rather see you stick with this once-off case of
> using a hook script (which is exactly what they are there for) rather
> than add an option to support broken ro images like this or reopen
> corner cases where we end up with broken containers.
> 
> So you have two solutions already at hand.
> 
> 1) Fix the broken image and finish populating the /dev directory in a
> hook script as you are now doing.
> 
> 2) Create the mandatory symlinks when you create the image and fix the
> existing images on disk.
> 
> You've already done #1.  In my mind, that seems like the best solution
> whether or not you do #2.

So it sounds like you think that if he had not done either of these and
lxc had simply ignored the errors, he would have had an error in
userspace later on?

Perhaps what we need is a clearer error message on the failure.  Does
someone mind writing the patch to do so?

-serge