Skip to content

L
lxc
Commit 6b0d5538 by Serge Hallyn Committed by Stéphane Graber
Options

unexpanded config file: turn into a string

Originally, we only kept a struct lxc_conf representing the current container configuration. This was insufficient because lxc.include's were expanded, so a clone or a snapshot would contain the expanded include file contents, rather than the original "lxc.include". If the host's include files are updated, clones and snapshots would not inherit those updates. To address this, we originally added a lxc_unexp_conf, which mirrored the lxc_conf, except that lxc.include was not expanded. This has its own cshortcomings, however, In particular, if a lxc.include has a lxc.cgroup setting, and you use the api to say: c.clear_config_item("lxc.cgroup") this is not representable in the lxc_unexp_conf. (The original problem, which was pointed out to me by stgraber, was slightly different, but unlike this problem it was not unsolvable). This patch changes the unexpanded configuration to be a textual representation of the configuration. This allows us *order* the configuration commands, which is what was not possible using the struct lxc_conf *lxc_unexp_conf. The write_config() now becomes a simple fwrite. However, lxc_clone is slightly complicated in parts, the worst of which is the need to rewrite the network configuration if we are changing the macaddrs. With this patch, lxc-clone and clear_config_item do the right thing. lxc-test-saveconfig and lxc-test-clonetest both pass. There is room for improvement - multiple calls to c.append_config_item("lxc.network.link", "lxcbr0") will result in multiple such lines in the configuration file. In that particular case it is harmless. There may be cases where it is not. Overall, this should be a huge improvement in terms of correctness. Changelog: Aug 1: updated to current lxc git head. All lxc-test* and python api test passed. Signed-off-by: 's avatarSerge Hallyn <serge.hallyn@ubuntu.com> Acked-by: 's avatarStéphane Graber <stgraber@ubuntu.com>
parent ff462013
Showing with 61 additions and 45 deletions
src/lxc/conf.c
...@@ -4495,6 +4495,7 @@ void lxc_conf_free(struct lxc_conf *conf) ...@@ -4495,6 +4495,7 @@ void lxc_conf_free(struct lxc_conf *conf)
free(conf->fstab); free(conf->fstab);
if (conf->rcfile) if (conf->rcfile)
free(conf->rcfile); free(conf->rcfile);
free(conf->unexpanded_config);
lxc_clear_config_network(conf); lxc_clear_config_network(conf);
if (conf->lsm_aa_profile) if (conf->lsm_aa_profile)
free(conf->lsm_aa_profile); free(conf->lsm_aa_profile);
......
src/lxc/conf.h
...@@ -287,7 +287,6 @@ struct saved_nic { ...@@ -287,7 +287,6 @@ struct saved_nic {
struct lxc_conf { struct lxc_conf {
int is_execute; int is_execute;
bool unexpanded;
char *fstab; char *fstab;
int tty; int tty;
int pts; int pts;
...@@ -352,6 +351,10 @@ struct lxc_conf { ...@@ -352,6 +351,10 @@ struct lxc_conf {
/* list of environment variables we'll add to the container when /* list of environment variables we'll add to the container when
* started */ * started */
struct lxc_list environment; struct lxc_list environment;
/* text representation of the config file */
char *unexpanded_config;
size_t unexpanded_len, unexpanded_alloced;
}; };
int run_lxc_hooks(const char *name, char *hook, struct lxc_conf *conf, int run_lxc_hooks(const char *name, char *hook, struct lxc_conf *conf,
......
src/lxc/confile.h
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include <stdio.h> #include <stdio.h>
#include <lxc/attach_options.h> #include <lxc/attach_options.h>
#include <stdbool.h>
struct lxc_conf; struct lxc_conf;
struct lxc_list; struct lxc_list;
...@@ -39,7 +40,8 @@ struct lxc_config_t { ...@@ -39,7 +40,8 @@ struct lxc_config_t {
extern struct lxc_config_t *lxc_getconfig(const char *key); extern struct lxc_config_t *lxc_getconfig(const char *key);
extern int lxc_list_nicconfigs(struct lxc_conf *c, const char *key, char *retv, int inlen); extern int lxc_list_nicconfigs(struct lxc_conf *c, const char *key, char *retv, int inlen);
extern int lxc_listconfigs(char *retv, int inlen); extern int lxc_listconfigs(char *retv, int inlen);
extern int lxc_config_read(const char *file, struct lxc_conf *conf, struct lxc_conf *unexp_conf); extern int lxc_config_read(const char *file, struct lxc_conf *conf, bool from_include);
extern int append_unexp_config_line(const char *line, struct lxc_conf *conf);
extern int lxc_config_define_add(struct lxc_list *defines, char* arg); extern int lxc_config_define_add(struct lxc_list *defines, char* arg);
extern int lxc_config_define_load(struct lxc_list *defines, extern int lxc_config_define_load(struct lxc_list *defines,
...@@ -52,4 +54,11 @@ extern int lxc_fill_elevated_privileges(char *flaglist, int *flags); ...@@ -52,4 +54,11 @@ extern int lxc_fill_elevated_privileges(char *flaglist, int *flags);
extern int lxc_get_config_item(struct lxc_conf *c, const char *key, char *retv, int inlen); extern int lxc_get_config_item(struct lxc_conf *c, const char *key, char *retv, int inlen);
extern int lxc_clear_config_item(struct lxc_conf *c, const char *key); extern int lxc_clear_config_item(struct lxc_conf *c, const char *key);
extern void write_config(FILE *fout, struct lxc_conf *c); extern void write_config(FILE *fout, struct lxc_conf *c);
extern bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key, const char *v);
/* These are used when cloning a container */
extern void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_subkeys);
extern bool clone_update_unexp_hooks(struct lxc_conf *c);
extern bool clone_update_unexp_network(struct lxc_conf *c);
#endif #endif
src/lxc/lxccontainer.c
...@@ -238,10 +238,6 @@ static void lxc_container_free(struct lxc_container *c) ...@@ -238,10 +238,6 @@ static void lxc_container_free(struct lxc_container *c)
lxc_conf_free(c->lxc_conf); lxc_conf_free(c->lxc_conf);
c->lxc_conf = NULL; c->lxc_conf = NULL;
} }
if (c->lxc_unexp_conf) {
lxc_conf_free(c->lxc_unexp_conf);
c->lxc_unexp_conf = NULL;
}
if (c->config_path) { if (c->config_path) {
free(c->config_path); free(c->config_path);
c->config_path = NULL; c->config_path = NULL;
...@@ -414,14 +410,9 @@ static bool load_config_locked(struct lxc_container *c, const char *fname) ...@@ -414,14 +410,9 @@ static bool load_config_locked(struct lxc_container *c, const char *fname)
{ {
if (!c->lxc_conf) if (!c->lxc_conf)
c->lxc_conf = lxc_conf_init(); c->lxc_conf = lxc_conf_init();
if (!c->lxc_unexp_conf) { if (!c->lxc_conf)
c->lxc_unexp_conf = lxc_conf_init(); return false;
if (c->lxc_unexp_conf) if (!lxc_config_read(fname, c->lxc_conf, false))
c->lxc_unexp_conf->unexpanded = true;
}
if (c->lxc_conf && c->lxc_unexp_conf &&
!lxc_config_read(fname, c->lxc_conf,
c->lxc_unexp_conf))
return true; return true;
return false; return false;
} }
...@@ -1214,10 +1205,6 @@ static void lxcapi_clear_config(struct lxc_container *c) ...@@ -1214,10 +1205,6 @@ static void lxcapi_clear_config(struct lxc_container *c)
lxc_conf_free(c->lxc_conf); lxc_conf_free(c->lxc_conf);
c->lxc_conf = NULL; c->lxc_conf = NULL;
} }
if (c->lxc_unexp_conf) {
lxc_conf_free(c->lxc_unexp_conf);
c->lxc_unexp_conf = NULL;
}
} }
} }
...@@ -1342,7 +1329,6 @@ static bool lxcapi_create(struct lxc_container *c, const char *t, ...@@ -1342,7 +1329,6 @@ static bool lxcapi_create(struct lxc_container *c, const char *t,
/* reload config to get the rootfs */ /* reload config to get the rootfs */
lxc_conf_free(c->lxc_conf); lxc_conf_free(c->lxc_conf);
c->lxc_conf = NULL; c->lxc_conf = NULL;
c->lxc_unexp_conf = NULL;
if (!load_config_locked(c, c->configfile)) if (!load_config_locked(c, c->configfile))
goto out_unlock; goto out_unlock;
...@@ -1440,6 +1426,20 @@ out: ...@@ -1440,6 +1426,20 @@ out:
return bret; return bret;
} }
static void do_clear_unexp_config_line(struct lxc_conf *conf, const char *key)
{
if (strcmp(key, "lxc.cgroup") == 0)
clear_unexp_config_line(conf, key, true);
else if (strcmp(key, "lxc.network") == 0)
clear_unexp_config_line(conf, key, true);
else if (strcmp(key, "lxc.hook") == 0)
clear_unexp_config_line(conf, key, true);
else
clear_unexp_config_line(conf, key, false);
if (!do_append_unexp_config_line(conf, key, ""))
WARN("Error clearing configuration for %s", key);
}
static bool lxcapi_clear_config_item(struct lxc_container *c, const char *key) static bool lxcapi_clear_config_item(struct lxc_container *c, const char *key)
{ {
int ret; int ret;
...@@ -1449,6 +1449,8 @@ static bool lxcapi_clear_config_item(struct lxc_container *c, const char *key) ...@@ -1449,6 +1449,8 @@ static bool lxcapi_clear_config_item(struct lxc_container *c, const char *key)
if (container_mem_lock(c)) if (container_mem_lock(c))
return false; return false;
ret = lxc_clear_config_item(c->lxc_conf, key); ret = lxc_clear_config_item(c->lxc_conf, key);
if (!ret)
do_clear_unexp_config_line(c->lxc_conf, key);
container_mem_unlock(c); container_mem_unlock(c);
return ret == 0; return ret == 0;
} }
...@@ -1867,7 +1869,7 @@ static bool lxcapi_save_config(struct lxc_container *c, const char *alt_file) ...@@ -1867,7 +1869,7 @@ static bool lxcapi_save_config(struct lxc_container *c, const char *alt_file)
fout = fopen(alt_file, "w"); fout = fopen(alt_file, "w");
if (!fout) if (!fout)
goto out; goto out;
write_config(fout, c->lxc_unexp_conf); write_config(fout, c->lxc_conf);
fclose(fout); fclose(fout);
ret = true; ret = true;
...@@ -2140,26 +2142,20 @@ static bool lxcapi_destroy_with_snapshots(struct lxc_container *c) ...@@ -2140,26 +2142,20 @@ static bool lxcapi_destroy_with_snapshots(struct lxc_container *c)
return lxcapi_destroy(c); return lxcapi_destroy(c);
} }
static bool set_config_item_locked(struct lxc_container *c, const char *key, const char *v) static bool set_config_item_locked(struct lxc_container *c, const char *key, const char *v)
{ {
struct lxc_config_t *config; struct lxc_config_t *config;
if (!c->lxc_conf) if (!c->lxc_conf)
c->lxc_conf = lxc_conf_init(); c->lxc_conf = lxc_conf_init();
if (!c->lxc_unexp_conf) { if (!c->lxc_conf)
c->lxc_unexp_conf = lxc_conf_init();
if (c->lxc_unexp_conf)
c->lxc_unexp_conf->unexpanded = true;
}
if (!c->lxc_conf || !c->lxc_unexp_conf)
return false; return false;
config = lxc_getconfig(key); config = lxc_getconfig(key);
if (!config) if (!config)
return false; return false;
if (config->cb(key, v, c->lxc_unexp_conf) != 0) if (config->cb(key, v, c->lxc_conf) != 0)
return false; return false;
return (0 == config->cb(key, v, c->lxc_conf)); return do_append_unexp_config_line(c->lxc_conf, key, v);
} }
static bool lxcapi_set_config_item(struct lxc_container *c, const char *key, const char *v) static bool lxcapi_set_config_item(struct lxc_container *c, const char *key, const char *v)
...@@ -2415,6 +2411,10 @@ static int copyhooks(struct lxc_container *oldc, struct lxc_container *c) ...@@ -2415,6 +2411,10 @@ static int copyhooks(struct lxc_container *oldc, struct lxc_container *c)
} }
} }
if (!clone_update_unexp_hooks(c->lxc_conf)) {
ERROR("Error saving new hooks in clone");
return -1;
}
c->save_config(c, NULL); c->save_config(c, NULL);
return 0; return 0;
} }
...@@ -2456,6 +2456,8 @@ static int copy_fstab(struct lxc_container *oldc, struct lxc_container *c) ...@@ -2456,6 +2456,8 @@ static int copy_fstab(struct lxc_container *oldc, struct lxc_container *c)
if (!oldpath) if (!oldpath)
return 0; return 0;
clear_unexp_config_line(c->lxc_conf, "lxc.mount", false);
char *p = strrchr(oldpath, '/'); char *p = strrchr(oldpath, '/');
if (!p) if (!p)
return -1; return -1;
...@@ -2480,6 +2482,10 @@ static int copy_fstab(struct lxc_container *oldc, struct lxc_container *c) ...@@ -2480,6 +2482,10 @@ static int copy_fstab(struct lxc_container *oldc, struct lxc_container *c)
ERROR("error: allocating pathname"); ERROR("error: allocating pathname");
return -1; return -1;
} }
if (!do_append_unexp_config_line(c->lxc_conf, "lxc.mount", newpath)) {
ERROR("error saving new lxctab");
return -1;
}
return 0; return 0;
} }
...@@ -2549,10 +2555,10 @@ static int copy_storage(struct lxc_container *c0, struct lxc_container *c, ...@@ -2549,10 +2555,10 @@ static int copy_storage(struct lxc_container *c0, struct lxc_container *c,
ERROR("Out of memory while setting storage path"); ERROR("Out of memory while setting storage path");
return -1; return -1;
} }
free(c->lxc_unexp_conf->rootfs.path); // We will simply append a new lxc.rootfs entry to the unexpanded config
c->lxc_unexp_conf->rootfs.path = strdup(c->lxc_conf->rootfs.path); clear_unexp_config_line(c->lxc_conf, "lxc.rootfs", false);
if (!c->lxc_unexp_conf->rootfs.path) { if (!do_append_unexp_config_line(c->lxc_conf, "lxc.rootfs", c->lxc_conf->rootfs.path)) {
ERROR("Out of memory while setting storage path"); ERROR("Error saving new rootfs to cloend config");
return -1; return -1;
} }
if (flags & LXC_CLONE_SNAPSHOT) if (flags & LXC_CLONE_SNAPSHOT)
...@@ -2765,7 +2771,7 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n ...@@ -2765,7 +2771,7 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
SYSERROR("open %s", newpath); SYSERROR("open %s", newpath);
goto out; goto out;
} }
write_config(fout, c->lxc_unexp_conf); write_config(fout, c->lxc_conf);
fclose(fout); fclose(fout);
c->lxc_conf->rootfs.path = origroot; c->lxc_conf->rootfs.path = origroot;
...@@ -2793,6 +2799,8 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n ...@@ -2793,6 +2799,8 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
if (ret < 0) if (ret < 0)
goto out; goto out;
clear_unexp_config_line(c2->lxc_conf, "lxc.utsname", false);
// update utsname // update utsname
if (!set_config_item_locked(c2, "lxc.utsname", newname)) { if (!set_config_item_locked(c2, "lxc.utsname", newname)) {
ERROR("Error setting new hostname"); ERROR("Error setting new hostname");
...@@ -2812,8 +2820,13 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n ...@@ -2812,8 +2820,13 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
} }
// update macaddrs // update macaddrs
if (!(flags & LXC_CLONE_KEEPMACADDR)) if (!(flags & LXC_CLONE_KEEPMACADDR)) {
network_new_hwaddrs(c2); network_new_hwaddrs(c2);
if (!clone_update_unexp_network(c2->lxc_conf)) {
ERROR("Error updating network for clone");
goto out;
}
}
// We've now successfully created c2's storage, so clear it out if we // We've now successfully created c2's storage, so clear it out if we
// fail after this // fail after this
......
src/lxc/lxccontainer.h
...@@ -99,16 +99,6 @@ struct lxc_container { ...@@ -99,16 +99,6 @@ struct lxc_container {
*/ */
struct lxc_conf *lxc_conf; struct lxc_conf *lxc_conf;
/*!
* \private
* The non-common, unexpanded configuration. This includes the
* list of lxc.include files, and does not contain any
* individual configuration items from the include files.
* Anything coming from the container's own configuration file
* or from lxcapi_set_config_item() does get added here.
*/
struct lxc_conf *lxc_unexp_conf;
// public fields // public fields
/*! Human-readable string representing last error */ /*! Human-readable string representing last error */
char *error_string; char *error_string;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment