]> git.ipfire.org Git - thirdparty/lxc.git/commitdiff
lxccontainer: various container creation fixes
authorChristian Brauner <christian.brauner@ubuntu.com>
Tue, 28 Nov 2017 11:32:05 +0000 (12:32 +0100)
committerChristian Brauner <christian.brauner@ubuntu.com>
Sun, 17 Dec 2017 14:34:59 +0000 (15:34 +0100)
This is beneficial for LXD as well.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
src/lxc/lxccontainer.c
src/lxc/storage/storage.c

index de8828413edab06ff37447b14a81626d4f9f8b71..98514eed8f71a938e00c94f812417c22acbfdd43 100644 (file)
@@ -1139,10 +1139,10 @@ static struct lxc_storage *do_storage_create(struct lxc_container *c,
                                             const char *type,
                                             struct bdev_specs *specs)
 {
-       char *dest;
+       int ret;
        size_t len;
+       char *dest;
        struct lxc_storage *bdev;
-       int ret;
 
        /* rootfs.path or lxcpath/lxcname/rootfs */
        if (c->lxc_conf->rootfs.path && access(c->lxc_conf->rootfs.path, F_OK) == 0) {
@@ -1156,24 +1156,26 @@ static struct lxc_storage *do_storage_create(struct lxc_container *c,
                dest = alloca(len);
                ret = snprintf(dest, len, "%s/%s/rootfs", lxcpath, c->name);
        }
-       if (ret < 0 || ret >= len)
+       if (ret < 0 || (size_t)ret >= len)
                return NULL;
 
        bdev = storage_create(dest, type, c->name, specs);
        if (!bdev) {
-               ERROR("Failed to create backing store type %s", type);
+               ERROR("Failed to create \"%s\" storage", type);
                return NULL;
        }
 
        do_lxcapi_set_config_item(c, "lxc.rootfs", bdev->src);
        do_lxcapi_set_config_item(c, "lxc.rootfs.backend", bdev->type);
 
-       /* if we are not root, chown the rootfs dir to root in the
-        * target uidmap */
-
-       if (geteuid() != 0 || (c->lxc_conf && !lxc_list_empty(&c->lxc_conf->id_map))) {
-               if (chown_mapped_root(bdev->dest, c->lxc_conf) < 0) {
-                       ERROR("Error chowning %s to container root", bdev->dest);
+       /* If we are not root, chown the rootfs dir to root in the target user
+        * namespace.
+        */
+       ret = geteuid();
+       if (ret != 0 || (c->lxc_conf && !lxc_list_empty(&c->lxc_conf->id_map))) {
+               ret = chown_mapped_root(bdev->dest, c->lxc_conf);
+               if (ret < 0) {
+                       ERROR("Error chowning \"%s\" to container root", bdev->dest);
                        suggest_default_idmap();
                        storage_put(bdev);
                        return NULL;
@@ -1550,10 +1552,10 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t,
                const char *bdevtype, struct bdev_specs *specs, int flags,
                char *const argv[])
 {
-       bool ret = false;
+       int partial_fd;
        pid_t pid;
+       bool ret = false;
        char *tpath = NULL;
-       int partial_fd;
 
        if (!c)
                return false;
@@ -1561,26 +1563,26 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t,
        if (t) {
                tpath = get_template_path(t);
                if (!tpath) {
-                       ERROR("bad template: %s", t);
+                       ERROR("Unknown template \"%s\"", t);
                        goto out;
                }
        }
 
-       /*
-        * If a template is passed in, and the rootfs already is defined in
-        * the container config and exists, then * caller is trying to create
-        * an existing container.  Return an error, but do NOT delete the
-        * container.
+       /* If a template is passed in, and the rootfs already is defined in the
+        * container config and exists, then the caller is trying to create an
+        * existing container. Return an error, but do NOT delete the container.
         */
        if (do_lxcapi_is_defined(c) && c->lxc_conf && c->lxc_conf->rootfs.path &&
                        access(c->lxc_conf->rootfs.path, F_OK) == 0 && tpath) {
-               ERROR("Container %s:%s already exists", c->config_path, c->name);
+               ERROR("Container \"%s\" already exists in \"%s\"", c->name,
+                     c->config_path);
                goto free_tpath;
        }
 
        if (!c->lxc_conf) {
                if (!do_lxcapi_load_config(c, lxc_global_config_value("lxc.default_config"))) {
-                       ERROR("Error loading default configuration file %s", lxc_global_config_value("lxc.default_config"));
+                       ERROR("Error loading default configuration file %s",
+                             lxc_global_config_value("lxc.default_config"));
                        goto free_tpath;
                }
        }
@@ -1588,39 +1590,42 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t,
        if (!create_container_dir(c))
                goto free_tpath;
 
-       /*
-        * if both template and rootfs.path are set, template is setup as rootfs.path.
-        * container is already created if we have a config and rootfs.path is accessible
+       /* If both template and rootfs.path are set, template is setup as
+        * rootfs.path. The container is already created if we have a config and
+        * rootfs.path is accessible
         */
        if (!c->lxc_conf->rootfs.path && !tpath) {
-               /* no template passed in and rootfs does not exist */
+               /* No template passed in and rootfs does not exist. */
                if (!c->save_config(c, NULL)) {
-                       ERROR("failed to save starting configuration for %s\n", c->name);
+                       ERROR("Failed to save initial config for \"%s\"", c->name);
                        goto out;
                }
                ret = true;
                goto out;
        }
+
+       /* Rootfs passed into configuration, but does not exist. */
        if (c->lxc_conf->rootfs.path && access(c->lxc_conf->rootfs.path, F_OK) != 0)
-               /* rootfs passed into configuration, but does not exist: error */
                goto out;
+
        if (do_lxcapi_is_defined(c) && c->lxc_conf->rootfs.path && !tpath) {
-               /* Rootfs already existed, user just wanted to save the
-                * loaded configuration */
+               /* Rootfs already existed, user just wanted to save the loaded
+                * configuration.
+                */
                if (!c->save_config(c, NULL))
-                       ERROR("failed to save starting configuration for %s\n", c->name);
+                       ERROR("Failed to save initial config for \"%s\"", c->name);
                ret = true;
                goto out;
        }
 
        /* Mark that this container is being created */
-       if ((partial_fd = create_partial(c)) < 0)
+       partial_fd = create_partial(c);
+       if (partial_fd < 0)
                goto out;
 
-       /* no need to get disk lock bc we have the partial locked */
+       /* No need to get disk lock bc we have the partial lock. */
 
-       /*
-        * Create the backing store
+       /* Create the storage.
         * Note we can't do this in the same task as we use to execute the
         * template because of the way zfs works.
         * After you 'zfs create', zfs mounts the fs only in the initial
@@ -1628,7 +1633,7 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t,
         */
        pid = fork();
        if (pid < 0) {
-               SYSERROR("failed to fork task for container creation template");
+               SYSERROR("Failed to fork task for container creation template");
                goto out_unlock;
        }
 
@@ -1637,15 +1642,17 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t,
 
                bdev = do_storage_create(c, bdevtype, specs);
                if (!bdev) {
-                       ERROR("Error creating backing store type %s for %s",
-                               bdevtype ? bdevtype : "(none)", c->name);
+                       ERROR("Failed to create %s storage for %s",
+                             bdevtype ? bdevtype : "(none)", c->name);
                        exit(EXIT_FAILURE);
                }
 
-               /* save config file again to store the new rootfs location */
+               /* Save config file again to store the new rootfs location. */
                if (!do_lxcapi_save_config(c, NULL)) {
-                       ERROR("failed to save starting configuration for %s", c->name);
-                       // parent task won't see bdev in config so we delete it
+                       ERROR("Failed to save initial config for %s", c->name);
+                       /* Parent task won't see the storage driver in the
+                        * config so we delete it.
+                        */
                        bdev->ops->umount(bdev);
                        bdev->ops->destroy(bdev);
                        exit(EXIT_FAILURE);
@@ -1655,7 +1662,7 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t,
        if (wait_for_pid(pid) != 0)
                goto out_unlock;
 
-       /* reload config to get the rootfs */
+       /* Reload config to get the rootfs. */
        lxc_conf_free(c->lxc_conf);
        c->lxc_conf = NULL;
        if (!load_config_locked(c, c->configfile))
@@ -1670,7 +1677,7 @@ static bool do_lxcapi_create(struct lxc_container *c, const char *t,
 
        if (t) {
                if (!prepend_lxc_header(c->configfile, tpath, argv)) {
-                       ERROR("Error prepending header to configuration file");
+                       ERROR("Failed to prepend header to config file");
                        goto out_unlock;
                }
        }
@@ -1688,8 +1695,8 @@ free_tpath:
 }
 
 static bool lxcapi_create(struct lxc_container *c, const char *t,
-               const char *bdevtype, struct bdev_specs *specs, int flags,
-               char *const argv[])
+                         const char *bdevtype, struct bdev_specs *specs,
+                         int flags, char *const argv[])
 {
        bool ret;
        current_config = c ? c->lxc_conf : NULL;
@@ -1801,7 +1808,7 @@ static bool lxcapi_createl(struct lxc_container *c, const char *t,
        args = lxc_va_arg_list_to_argv(ap, 0, 0);
        va_end(ap);
        if (!args) {
-               ERROR("Memory allocation error.");
+               ERROR("Failed to allocate memory");
                goto out;
        }
 
@@ -4284,7 +4291,7 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
 
        c = malloc(sizeof(*c));
        if (!c) {
-               fprintf(stderr, "failed to malloc lxc_container\n");
+               fprintf(stderr, "Failed to allocate memory for %s\n", name);
                return NULL;
        }
        memset(c, 0, sizeof(*c));
@@ -4295,39 +4302,43 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
                c->config_path = strdup(lxc_global_config_value("lxc.lxcpath"));
 
        if (!c->config_path) {
-               fprintf(stderr, "Out of memory\n");
+               fprintf(stderr, "Failed to allocate memory for %s\n", name);
                goto err;
        }
 
        remove_trailing_slashes(c->config_path);
        c->name = malloc(strlen(name)+1);
        if (!c->name) {
-               fprintf(stderr, "Error allocating lxc_container name\n");
+               fprintf(stderr, "Failed to allocate memory for %s\n", name);
                goto err;
        }
        strcpy(c->name, name);
 
        c->numthreads = 1;
-       if (!(c->slock = lxc_newlock(c->config_path, name))) {
-               fprintf(stderr, "failed to create lock\n");
+       c->slock = lxc_newlock(c->config_path, name);
+       if (!c->slock) {
+               fprintf(stderr, "Failed to create lock for %s\n", name);
                goto err;
        }
 
-       if (!(c->privlock = lxc_newlock(NULL, NULL))) {
-               fprintf(stderr, "failed to alloc privlock\n");
+       c->privlock = lxc_newlock(NULL, NULL);
+       if (!c->privlock) {
+               fprintf(stderr, "Failed to create private lock for %s\n", name);
                goto err;
        }
 
        if (!set_config_filename(c)) {
-               fprintf(stderr, "Error allocating config file pathname\n");
+               fprintf(stderr, "Failed to create config file name for %s\n", name);
                goto err;
        }
 
-       if (file_exists(c->configfile) && !lxcapi_load_config(c, NULL))
+       if (file_exists(c->configfile) && !lxcapi_load_config(c, NULL)) {
+               fprintf(stderr, "Failed to load config for %s\n", name);
                goto err;
+       }
 
        if (ongoing_create(c) == 2) {
-               ERROR("Error: %s creation was not completed", c->name);
+               ERROR("Failed to complete container creation for %s", c->name);
                container_destroy(c);
                lxcapi_clear_config(c);
        }
index 90053152ed55fcb3a2f0600f374df1b075529382..765ba8278fa70074d265e5e271a16940f1e56d09 100644 (file)
@@ -484,20 +484,21 @@ err:
 struct lxc_storage *storage_create(const char *dest, const char *type,
                                   const char *cname, struct bdev_specs *specs)
 {
+       int ret;
        struct lxc_storage *bdev;
        char *best_options[] = {"btrfs", "zfs", "lvm", "dir", "rbd", NULL};
 
        if (!type)
                return do_storage_create(dest, "dir", cname, specs);
 
-       if (strcmp(type, "best") == 0) {
+       ret = strcmp(type, "best");
+       if (ret == 0) {
                int i;
                /* Try for the best backing store type, according to our
                 * opinionated preferences.
                 */
                for (i = 0; best_options[i]; i++) {
-                       bdev = do_storage_create(dest, best_options[i], cname,
-                                                specs);
+                       bdev = do_storage_create(dest, best_options[i], cname, specs);
                        if (bdev)
                                return bdev;
                }
@@ -506,12 +507,16 @@ struct lxc_storage *storage_create(const char *dest, const char *type,
        }
 
        /* -B lvm,dir */
-       if (strchr(type, ',') != NULL) {
-               char *dup = alloca(strlen(type) + 1), *saveptr = NULL, *token;
+       if (strchr(type, ',')) {
+               char *dup, *token;
+               char *saveptr = NULL;
+
+               dup = alloca(strlen(type) + 1);
                strcpy(dup, type);
                for (token = strtok_r(dup, ",", &saveptr); token;
                     token = strtok_r(NULL, ",", &saveptr)) {
-                       if ((bdev = do_storage_create(dest, token, cname, specs)))
+                       bdev = do_storage_create(dest, token, cname, specs);
+                       if (bdev)
                                return bdev;
                }
        }