[Devel] [PATCH 2/3] allow for distro-specific fix ups at creation time.
Glauber Costa
glommer at parallels.com
Mon May 20 13:40:00 PDT 2013
On 05/21/2013 12:32 AM, Kir Kolyshkin wrote:
> On 05/20/2013 05:49 AM, Glauber Costa wrote:
>> From: Glauber Costa <glommer at parallels.com>
>>
>> We will need that infrastucture when running with Linux upstream,
>> since some
>> support is very unlikely to ever land in the Kernel. We need to do
>> things like
>> account for the fact that udev may kick in and destroy all the setup
>> we have
>> done for /dev. Since preemptively preventing those actions to happen
>> is very
>> hard ( as an example, centos6 init binary will issue mount syscalls
>> itself),
>> the most robust approach is to insert a script that will be run
>> shortly after
>> rc.sysinit. Although this can't guarantee that it will run
>> *immediately* after,
>> it should be early enough to undo every harmful operation done by
>> /sbin/init
>> and rc.sysinit, therefore allowing operation to continue freely.
>
> I understand that
>
> 1 this is something redhat/centos-specific?
> 2 there is no need to do that for debian/ubuntu etc?
>
No, thi is old-udev specific.
In more up to date distributions, udev is already aware that it might be
being containerized and won't do all those initialization things.
I haven't tested *all* distros we support, but among the most up to date
in each variant, only centos6 needs it (ubuntu 12 runs fine, suse 12
runs fine)
> Am I right?
Yes and no, see above.
>
>>
>> Signed-off-by: Glauber Costa <glommer at parallels.com>
>> ---
>> etc/dists/redhat.conf | 1 +
>> etc/dists/scripts/fixups.sh | 43
>> +++++++++++++++++++++++++++++++++++++++++++
>> include/dist.h | 2 ++
>> include/env.h | 3 ++-
>> src/lib/dist.c | 10 +++++++++-
>> src/lib/env.c | 10 ++++++----
>> src/lib/exec.c | 2 +-
>> src/lib/hooks_ct.c | 35 +++++++++++++++++++++++++++++++++++
>> 8 files changed, 99 insertions(+), 7 deletions(-)
>> create mode 100755 etc/dists/scripts/fixups.sh
>>
>> diff --git a/etc/dists/redhat.conf b/etc/dists/redhat.conf
>> index 727461d..49226c5 100644
>> --- a/etc/dists/redhat.conf
>> +++ b/etc/dists/redhat.conf
>> @@ -25,3 +25,4 @@ SET_DNS=set_dns.sh
>> SET_USERPASS=set_userpass.sh
>> SET_UGID_QUOTA=set_ugid_quota.sh
>> POST_CREATE=postcreate.sh
>> +CT_FIXUP=fixups.sh
>> diff --git a/etc/dists/scripts/fixups.sh b/etc/dists/scripts/fixups.sh
>> new file mode 100755
>> index 0000000..da3fd24
>> --- /dev/null
>> +++ b/etc/dists/scripts/fixups.sh
>> @@ -0,0 +1,43 @@
>> +#!/bin/bash
>
> Why bash? It is not available in some distros by default (notably Debian).
> So, unless we do have a reason to use bash-specific features we'd rather
> not to.
>
Will change to /bin/sh
>> +# Copyright (C) 2000-2009, Parallels, Inc. All rights reserved.
>
> :))
That was written by a past me.
>
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write to the Free Software
>> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>> 02111-1307 USA
>> +#
>> +#
>> +# Run-time fixup for legacy distributions that will rely on udevd to
>> populate
>> +# their /dev. Those distributions will effectively mount an empty
>> tmpfs on top
>> +# of /dev, defeating any setup we may have done. This includes
>> RHEL-based
>> +# distributions up to RHEL6. We are better off not allowing that to
>> run at all
>> +
>> +function fixup_udev()
>
> s/function //
>
> as this is bashism
>
ok
>> index bde94ab..8047cf7 100644
>> --- a/src/lib/dist.c
>> +++ b/src/lib/dist.c
>> @@ -36,7 +36,8 @@ static struct distr_conf {
>> {"SET_DNS", SET_DNS},
>> {"SET_USERPASS", SET_USERPASS},
>> {"SET_UGID_QUOTA", SET_UGID_QUOTA},
>> - {"POST_CREATE", POST_CREATE}
>> + {"POST_CREATE", POST_CREATE},
>> + {"CT_FIXUP", CT_FIXUP }
>
> 1. Please add comma after } so the next author won't have to patch your
> line.
> I.e.
>
> + {"CT_FIXUP", CT_FIXUP },
>
>
> 2. Maybe we can give it more concrete name? FIXUP is, well, anything.
>
> This is very unfortunate. All the other distro scripts we maintain have
> very
> well defined names, associated actions and ways to run. They are described
> in details in etc/dists/scripts/distribution.conf-template file.
>
> In this case we have a script with a name that doesn't tell us anything,
> is only run for redhat* distros, only when user ns is available/enabled,
> and in an extremely strange way -- not by executing it in a context of CT
> like all other scripts do, but by putting it to /etc/rc{3,5}d/S00fixups.sh
> before CT start. More to say, the logic of putting it this way is built
> into
> vzctl C code.
>
> Distro scripts meant to be flexible and configurable. This one is quite
> the opposite.
>
> Can we make it more flexible and configurable? Like, have a PRE_START
> script
> which is to be executed in context of CT right before starting its init
> process?
No, because that needs to run after init.
init + rc.sysinit, is where the harm lies. /sbin/init will remount proc
undoing our /proc mount (this *still* does not harm us, because we have
no /proc fuse replacement, but that is the plan), and then in rc.sysinit
all the udev magic is done.
So we need something that runs after rc.sysinit.
> That script would then do all the logic of setting up those /etc/rc.d/
> scripts.
> If needed, we can pass some parameters to this script through the
> environment,
> like if we are enabling user ns.
>
If you can tell me a way to guarantee it will be run after rc.sysinit, I
am fine with it.
More information about the Devel
mailing list