Quoting TAMUKI Shoichi (tam...@linet.gr.jp):
> Split figureout_rootfs() off from cleate_run_template() to allow
> common use of the function.
Hi,
this worries me actually. You'll see why when I ask "who frees the
bdev you allocated?"
The code you're splitting off was previously only run from within a
short-lived task that will exit after the template runs. But by
splitting it off into a common function, you have to make sure to
not leak memory etc.
It's possible that that's the only thing that needs to be cleaned
up is to get a strdup() of the bdev->dest, bdev_put(bdev), then
return the copy, and require (clearly comment) that callers
free the returned string.
Oh, no, you'll also have to stop doing exit(1) on failure paths.
> Signed-off-by: TAMUKI Shoichi <tam...@linet.gr.jp>
> ---
> src/lxc/lxccontainer.c | 104
> +++++++++++++++++++++++++++----------------------
> 1 file changed, 57 insertions(+), 47 deletions(-)
>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 4f90f35..b2ecfb3 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -876,6 +876,8 @@ static char *lxcbasename(char *path)
> return p;
> }
>
> +static char *figureout_rootfs(struct lxc_conf *conf);
> +
> static bool create_run_template(struct lxc_container *c, char *tpath, bool
> quiet,
> char *const argv[])
> {
> @@ -891,8 +893,7 @@ static bool create_run_template(struct lxc_container *c,
> char *tpath, bool quiet
> }
>
> if (pid == 0) { // child
> - char *patharg, *namearg, *rootfsarg, *src;
> - struct bdev *bdev = NULL;
> + char *patharg, *namearg, *rootfsarg, *rootfs;
> int i;
> int ret, len, nargs = 0;
> char **newargv;
> @@ -907,49 +908,7 @@ static bool create_run_template(struct lxc_container *c,
> char *tpath, bool quiet
> open("/dev/null", O_RDWR);
> }
>
> - src = c->lxc_conf->rootfs.path;
> - /*
> - * for an overlay create, what the user wants is the template
> to fill
> - * in what will become the readonly lower layer. So don't
> mount for
> - * the template
> - */
> - if (strncmp(src, "overlayfs:", 10) == 0)
> - src = overlay_getlower(src+10);
> - if (strncmp(src, "aufs:", 5) == 0)
> - src = overlay_getlower(src+5);
> -
> - bdev = bdev_init(c->lxc_conf, src, c->lxc_conf->rootfs.mount,
> NULL);
> - if (!bdev) {
> - ERROR("Error opening rootfs");
> - exit(1);
> - }
> -
> - if (geteuid() == 0) {
> - if (unshare(CLONE_NEWNS) < 0) {
> - ERROR("error unsharing mounts");
> - exit(1);
> - }
> - if (detect_shared_rootfs()) {
> - if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC,
> NULL)) {
> - SYSERROR("Failed to make / rslave to
> run template");
> - ERROR("Continuing...");
> - }
> - }
> - }
> - if (strcmp(bdev->type, "dir") && strcmp(bdev->type, "btrfs")) {
> - if (geteuid() != 0) {
> - ERROR("non-root users can only create btrfs and
> directory-backed containers");
> - exit(1);
> - }
> - if (bdev->ops->mount(bdev) < 0) {
> - ERROR("Error mounting rootfs");
> - exit(1);
> - }
> - } else { // TODO come up with a better way here!
> - if (bdev->dest)
> - free(bdev->dest);
> - bdev->dest = strdup(bdev->src);
> - }
> + rootfs=figureout_rootfs(conf);
>
> /*
> * create our new array, pre-pend the template name and
> @@ -981,11 +940,11 @@ static bool create_run_template(struct lxc_container
> *c, char *tpath, bool quiet
> exit(1);
> newargv[2] = namearg;
>
> - len = strlen("--rootfs=") + 1 + strlen(bdev->dest);
> + len = strlen("--rootfs=") + 1 + strlen(rootfs);
> rootfsarg = malloc(len);
> if (!rootfsarg)
> exit(1);
> - ret = snprintf(rootfsarg, len, "--rootfs=%s", bdev->dest);
> + ret = snprintf(rootfsarg, len, "--rootfs=%s", rootfs);
> if (ret < 0 || ret >= len)
> exit(1);
> newargv[3] = rootfsarg;
> @@ -1125,6 +1084,57 @@ static bool create_run_template(struct lxc_container
> *c, char *tpath, bool quiet
> return true;
> }
>
> +static char *figureout_rootfs(struct lxc_conf *conf)
> +{
> + char *src;
> + struct bdev *bdev = NULL;
> +
> + src = conf->rootfs.path;
> + /*
> + * for an overlay create, what the user wants is the template to fill
> + * in what will become the readonly lower layer. So don't mount for
> + * the template
> + */
> + if (strncmp(src, "overlayfs:", 10) == 0)
> + src = overlay_getlower(src+10);
> + if (strncmp(src, "aufs:", 5) == 0)
> + src = overlay_getlower(src+5);
> +
> + bdev = bdev_init(conf, src, conf->rootfs.mount, NULL);
> + if (!bdev) {
> + ERROR("Error opening rootfs");
> + exit(1);
> + }
> +
> + if (geteuid() == 0) {
> + if (unshare(CLONE_NEWNS) < 0) {
> + ERROR("error unsharing mounts");
> + exit(1);
> + }
> + if (detect_shared_rootfs()) {
> + if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL)) {
> + SYSERROR("Failed to make / rslave to run
> template");
> + ERROR("Continuing...");
> + }
> + }
> + }
> + if (strcmp(bdev->type, "dir") && strcmp(bdev->type, "btrfs")) {
> + if (geteuid() != 0) {
> + ERROR("non-root users can only create btrfs and
> directory-backed containers");
> + exit(1);
> + }
> + if (bdev->ops->mount(bdev) < 0) {
> + ERROR("Error mounting rootfs");
> + exit(1);
> + }
> + } else { // TODO come up with a better way here!
> + if (bdev->dest)
> + free(bdev->dest);
> + bdev->dest = strdup(bdev->src);
> + }
> + return bdev->dest;
> +}
> +
> static bool prepend_lxc_header(char *path, const char *t, char *const argv[])
> {
> long flen;
> --
> 1.9.0
> _______________________________________________
> lxc-devel mailing list
> lxc-devel@lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
