[Devel] [PATCH 2/3] allow for distro-specific fix ups at creation time.
Kir Kolyshkin
kir at openvz.org
Mon May 20 13:32:04 PDT 2013
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?
Am I right?
>
> 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.
> +# Copyright (C) 2000-2009, Parallels, Inc. All rights reserved.
:))
> +#
> +# 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
> +{
> + if [ -f /etc/fedora-release ]; then
> + return;
s/;//
> + fi
> +
> + # All the first two may fail if the distribution didn't actually mount
> + # then.
> + umount /dev/pts
> + umount /dev/shm
> + # still may hold references to files open by rc.sysinit. Even if this is the
> + # case those will be simple things like /dev/null and should go away shortly.
> + # let us not fail because of that.
> + umount /dev -l
> +}
> +
> +fixup_udev
> +
> +exit 0
> +
Empty line at EOL is treated as whitespace error by git-am. I tend to agree.
> diff --git a/include/dist.h b/include/dist.h
> index 1f6a5a6..4faa452 100644
> --- a/include/dist.h
> +++ b/include/dist.h
> @@ -29,6 +29,7 @@
> #define SET_USERPASS 5
> #define SET_UGID_QUOTA 6
> #define POST_CREATE 7
> +#define CT_FIXUP 8
>
> typedef struct {
> char *def_ostmpl;
> @@ -46,6 +47,7 @@ typedef struct dist_actions {
> char *set_userpass; /**< setup user password. */
> char *set_ugid_quota; /**< setup 2level quota. */
> char *post_create; /**< sostcreate actions. */
> + char *ct_fixup; /**< run-time fixups. */
> } dist_actions;
>
> /* Read distribution specific actions configuration file.
> diff --git a/include/env.h b/include/env.h
> index 4fef438..b10dfd9 100644
> --- a/include/env.h
> +++ b/include/env.h
> @@ -104,7 +104,7 @@ int vps_stop(vps_handler *h, envid_t veid, vps_param *param, int stop_mode,
> int vz_chroot(const char *root);
> int vz_env_create(vps_handler *h, envid_t veid, vps_res *res,
> int wait_p[2], int old_wait_p[2], int err_p[2],
> - env_create_FN fn, void *data);
> + env_create_FN fn, dist_actions *actions, void *data);
>
> struct vps_res;
> struct arg_start {
> @@ -116,6 +116,7 @@ struct arg_start {
> vps_handler *h;
> void *data;
> env_create_FN fn;
> + dist_actions *actions;
> int userns_p; /* while running in userns, there's extra sync needed */
> };
>
> diff --git a/src/lib/dist.c b/src/lib/dist.c
> 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?
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.
> };
>
> static int get_action_id(char *name)
> @@ -100,6 +101,12 @@ static int add_dist_action(dist_actions *d_actions, char *name, char *action,
> break;
> d_actions->post_create = strdup(file);
> break;
> + case CT_FIXUP:
> + if (d_actions->ct_fixup != NULL)
> + break;
> + d_actions->ct_fixup = strdup(file);
> + break;
> +
> }
> return 0;
> }
> @@ -115,6 +122,7 @@ void free_dist_actions(dist_actions *d_actions)
> free(d_actions->set_userpass);
> free(d_actions->set_ugid_quota);
> free(d_actions->post_create);
> + free(d_actions->ct_fixup);
> }
>
> static int get_dist_conf_name(char *dist_name, char *dir, char *file, int len)
> diff --git a/src/lib/env.c b/src/lib/env.c
> index 6923b33..56aecb5 100644
> --- a/src/lib/env.c
> +++ b/src/lib/env.c
> @@ -300,7 +300,8 @@ int exec_container_init(struct arg_start *arg,
> }
>
> static int vz_real_env_create(vps_handler *h, envid_t veid, vps_res *res,
> - int wait_p, int old_wait_p, int err_p, env_create_FN fn, void *data)
> + int wait_p, int old_wait_p, int err_p, env_create_FN fn, dist_actions *actions,
> + void *data)
>
> {
> struct arg_start arg;
> @@ -313,6 +314,7 @@ static int vz_real_env_create(vps_handler *h, envid_t veid, vps_res *res,
> arg.h = h;
> arg.data = data;
> arg.fn = fn;
> + arg.actions = actions;
>
> return h->env_create(&arg);
> }
> @@ -419,7 +421,7 @@ static void get_osrelease(vps_res *res)
>
> int vz_env_create(vps_handler *h, envid_t veid, vps_res *res,
> int wait_p[2], int old_wait_p[2], int err_p[2],
> - env_create_FN fn, void *data)
> + env_create_FN fn, dist_actions *actions, void *data)
> {
> int ret, pid, errcode;
> int old_wait_fd;
> @@ -461,7 +463,7 @@ int vz_env_create(vps_handler *h, envid_t veid, vps_res *res,
> old_wait_fd = -1;
>
> ret = vz_real_env_create(h, veid, res, wait_p[0],
> - old_wait_fd, err_p[1], fn, data);
> + old_wait_fd, err_p[1], fn, actions, data);
> if (ret)
> write(STDIN_FILENO, &ret, sizeof(ret));
> exit(ret);
> @@ -649,7 +651,7 @@ int vps_start_custom(vps_handler *h, envid_t veid, vps_param *param,
> fix_cpu(&res->cpu);
>
> ret = vz_env_create(h, veid, res, wait_p,
> - old_wait_p, err_p, fn, data);
> + old_wait_p, err_p, fn, &actions, data);
> if (ret)
> goto err;
>
> diff --git a/src/lib/exec.c b/src/lib/exec.c
> index 9cf811c..74375dd 100644
> --- a/src/lib/exec.c
> +++ b/src/lib/exec.c
> @@ -454,7 +454,7 @@ int vps_run_script(vps_handler *h, envid_t veid, char *script, vps_param *vps_p)
> }
> }
> if ((ret = vz_env_create(h, veid, &vps_p->res, rd_p, NULL,
> - wr_p, NULL, NULL)))
> + wr_p, NULL, NULL, NULL)))
> {
> return ret;
> }
> diff --git a/src/lib/hooks_ct.c b/src/lib/hooks_ct.c
> index 7bc9814..f769865 100644
> --- a/src/lib/hooks_ct.c
> +++ b/src/lib/hooks_ct.c
> @@ -283,6 +283,40 @@ static void create_devices(vps_handler *h, envid_t veid, const char *root)
> }
> }
>
> +static int apply_fixups(vps_handler *h, envid_t veid, const char *root, dist_actions *actions)
> +{
> + char buf[STR_SIZE];
> + int runlevels[] = { 3, 5 }; /* where to apply the fixups */
> + unsigned int i;
> +
> + /* Distributions that don't need the fixup will can stop right here */
> + if (!actions || !actions->ct_fixup)
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(runlevels); i++) {
> + int fd;
> + snprintf(buf, sizeof(buf), "%s/etc/rc%d.d/S00vz-fixups.sh",
> + root, runlevels[i]);
> +
> + fd = open(buf, O_CREAT, S_IRWXU|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
> + if (fd < 0)
> + goto out;
> + close(fd);
> + /*
> + * We bind mount instead of symlinking the original file so it
> + * won't appear in the original private mount.
> + */
> + if (mount(actions->ct_fixup, buf, "", MS_BIND, 0) < 0)
> + goto out;
> + }
> +
> + return 0;
> +
> +out:
> + logger(-1, errno, "Could not install fixup script\n");
> + return VZ_RESOURCE_ERROR;
> +}
> +
> static int _env_create(void *data)
> {
> struct arg_start *arg = data;
> @@ -308,6 +342,7 @@ static int _env_create(void *data)
>
> if (arg->h->can_join_userns) {
> create_devices(arg->h, arg->veid, arg->res->fs.root);
> + apply_fixups(arg->h, arg->veid, arg->res->fs.root, arg->actions);
> }
>
> ret = ct_chroot(arg->res->fs.root);
More information about the Devel
mailing list