[Devel] [PATCH v5 6/6] allow for distro-specific fix ups at creation time.
Kir Kolyshkin
kir at openvz.org
Fri May 17 20:28:14 PDT 2013
PLease see inline.
On 05/17/2013 09:26 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.
>
> 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 | 38 ++++++++++++++++++++++++++++++++++++++
> 8 files changed, 102 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
> +# 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()
> +{
> + if [ -f /etc/fedora-release ]; then
> + return;
> + 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
> +
> 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 }
> };
>
> 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 96400c0..9a130e2 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);
> @@ -635,7 +637,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 f002fa0..8ea1a74 100644
> --- a/src/lib/hooks_ct.c
> +++ b/src/lib/hooks_ct.c
> @@ -370,6 +370,43 @@ static int _env_create(void *data)
> return exec_container_init(arg, &create_param);
> }
>
> +static int apply_fixups(vps_handler *h, envid_t veid, const char *root, dist_actions *actions)
> +{
> + char buf[STR_SIZE];
> +
> + /* Distributions that don't need the fixup will can stop right here */
> + if (!actions || !actions->ct_fixup)
> + return 0;
> +
> + if (snprintf(buf, sizeof(buf), "%s/%s", root, "/etc/rc3.d/S00vz-fixups.sh") < 0)
Again and again :(
How this snprintf can return negative value?
> + goto out;
> +
> + if (open(buf, O_RDWR|O_CREAT, 0) < 0)
> + goto out;
> +
> + if (mount(actions->ct_fixup, buf, "", MS_BIND, 0) < 0)
> + goto out;
Please remind me why are you bind-mounting rather than just copying the
file?
> +
> + /*
> + * Being safe never killed anyone...
> + * We bind mount instead of symlinking the original rc3 so it won't appear in the original
> + * private mount.
> + */
> + if (snprintf(buf, sizeof(buf), "%s/%s", root, "/etc/rc5.d/S00vz-fixups.sh") < 0)
> + goto out;
> +
> + if (open(buf, O_RDWR|O_CREAT, 0) < 0)
> + goto out;
> +
> + if (mount(actions->ct_fixup, buf, "", MS_BIND, 0) < 0)
> + goto out;
This is copy-pasting. Can you at least redo this with something like
char *dirs[] = {"/etc/rc3.d", "/etc/rc5.d"};
for(i = 0; i < ARRAY_SIZE(dirs); i++) {
bla bla bla
}
> + return 0;
> +
> +out:
> + logger(-1, errno, "Could not install fixup script\n");
> + return VZ_RESOURCE_ERROR;
> +}
> +
> static int ct_env_create(struct arg_start *arg)
> {
>
> @@ -438,6 +475,7 @@ static int ct_env_create(struct arg_start *arg)
>
> 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 = clone(_env_create, child_stack, clone_flags, arg);
More information about the Devel
mailing list