From: Christian Brauner Date: Tue, 28 Nov 2017 11:32:05 +0000 (+0100) Subject: lxccontainer: various container creation fixes X-Git-Tag: lxc-2.0.10~551 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0e5eb43a1dcd72e338263dc59bfba228a8167406;p=thirdparty%2Flxc.git lxccontainer: various container creation fixes This is beneficial for LXD as well. Signed-off-by: Christian Brauner Signed-off-by: Adrian Reber --- diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index de8828413..98514eed8 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -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); } diff --git a/src/lxc/storage/storage.c b/src/lxc/storage/storage.c index 90053152e..765ba8278 100644 --- a/src/lxc/storage/storage.c +++ b/src/lxc/storage/storage.c @@ -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; } }