Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RFC] conf: introduce lxc.rootfs.managed #2435

Merged
merged 3 commits into from Aug 30, 2018
Merged

[RFC] conf: introduce lxc.rootfs.managed #2435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e54330
conf: introduce lxc.rootfs.managed
Jun 27, 2018
2a6d08c
tests: cleanup get_item.c
Jun 28, 2018
6e0045b
tests: add lxc.rootfs.managed test
Jun 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/lxc/conf.c
View file
Open in desktop
Expand Up @@ -2760,6 +2760,7 @@ struct lxc_conf *lxc_conf_init(void)
free(new);
return NULL;
}
new->rootfs.managed = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't lxc_conf_init() set this to the same as the clear method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other way around actually. The default should be that it's managed. So clear should set to true most likely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless that is to confusing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whichever is less of a change to the previous behavior I guess?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lxc.rootfs.managed is supposed to mean "the rootfs is managed by this lxc instance" so the default should be true.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would calling it lxc.rootfs.external and reversing the logic make it less confusing maybe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but I like the managed part and external is harder to grasp. Usually the lxc.rootfs.managed config key will not be set anyway. It will only be set when set to false.

new->logfd = -1;
lxc_list_init(&new->cgroup);
lxc_list_init(&new->cgroup2);
Expand Down
2 changes: 2 additions & 0 deletions src/lxc/conf.h
View file
Open in desktop
Expand Up @@ -154,6 +154,7 @@ struct lxc_tty_info {
* @options : mount options
* @mountflags : the portion of @options that are flags
* @data : the porition of @options that are not flags
* @managed : whether it is managed by LXC
*/
struct lxc_rootfs {
char *path;
Expand All @@ -162,6 +163,7 @@ struct lxc_rootfs {
char *options;
unsigned long mountflags;
char *data;
bool managed;
};

/*
Expand Down
40 changes: 40 additions & 0 deletions src/lxc/confile.c
View file
Open in desktop
Expand Up @@ -141,6 +141,7 @@ lxc_config_define(no_new_privs);
lxc_config_define(personality);
lxc_config_define(prlimit);
lxc_config_define(pty_max);
lxc_config_define(rootfs_managed);
lxc_config_define(rootfs_mount);
lxc_config_define(rootfs_options);
lxc_config_define(rootfs_path);
Expand Down Expand Up @@ -226,6 +227,7 @@ static struct lxc_config_t config[] = {
{ "lxc.no_new_privs", set_config_no_new_privs, get_config_no_new_privs, clr_config_no_new_privs, },
{ "lxc.prlimit", set_config_prlimit, get_config_prlimit, clr_config_prlimit, },
{ "lxc.pty.max", set_config_pty_max, get_config_pty_max, clr_config_pty_max, },
{ "lxc.rootfs.managed", set_config_rootfs_managed, get_config_rootfs_managed, clr_config_rootfs_managed, },
{ "lxc.rootfs.mount", set_config_rootfs_mount, get_config_rootfs_mount, clr_config_rootfs_mount, },
{ "lxc.rootfs.options", set_config_rootfs_options, get_config_rootfs_options, clr_config_rootfs_options, },
{ "lxc.rootfs.path", set_config_rootfs_path, get_config_rootfs_path, clr_config_rootfs_path, },
Expand Down Expand Up @@ -2134,6 +2136,31 @@ static int set_config_rootfs_path(const char *key, const char *value,
return ret;
}

static int set_config_rootfs_managed(const char *key, const char *value,
struct lxc_conf *lxc_conf, void *data)
{
unsigned int val = 0;

if (lxc_config_value_empty(value)) {
lxc_conf->rootfs.managed = true;
return 0;
}

if (lxc_safe_uint(value, &val) < 0)
return -EINVAL;

switch (val) {
case 0:
lxc_conf->rootfs.managed = false;
return 0;
case 1:
lxc_conf->rootfs.managed = true;
return 0;
}

return -EINVAL;
}

static int set_config_rootfs_mount(const char *key, const char *value,
struct lxc_conf *lxc_conf, void *data)
{
Expand Down Expand Up @@ -3356,6 +3383,12 @@ static int get_config_rootfs_path(const char *key, char *retv, int inlen,
return lxc_get_conf_str(retv, inlen, c->rootfs.path);
}

static int get_config_rootfs_managed(const char *key, char *retv, int inlen,
struct lxc_conf *c, void *data)
{
return lxc_get_conf_bool(c, retv, inlen, c->rootfs.managed);
}

static int get_config_rootfs_mount(const char *key, char *retv, int inlen,
struct lxc_conf *c, void *data)
{
Expand Down Expand Up @@ -3976,6 +4009,13 @@ static inline int clr_config_rootfs_path(const char *key, struct lxc_conf *c,
return 0;
}

static inline int clr_config_rootfs_managed(const char *key, struct lxc_conf *c,
void *data)
{
c->rootfs.managed = true;
return 0;
}

static inline int clr_config_rootfs_mount(const char *key, struct lxc_conf *c,
void *data)
{
Expand Down
15 changes: 15 additions & 0 deletions src/lxc/confile_utils.c
View file
Open in desktop
Expand Up @@ -672,6 +672,21 @@ int lxc_get_conf_str(char *retv, int inlen, const char *value)
return value_len;
}

int lxc_get_conf_bool(struct lxc_conf *c, char *retv, int inlen, bool v)
{
int len;
int fulllen = 0;

if (!retv)
inlen = 0;
else
memset(retv, 0, inlen);

strprint(retv, inlen, "%d", v);

return fulllen;
}

int lxc_get_conf_int(struct lxc_conf *c, char *retv, int inlen, int v)
{
int len;
Expand Down
1 change: 1 addition & 0 deletions src/lxc/confile_utils.h
View file
Open in desktop
Expand Up @@ -86,6 +86,7 @@ extern bool lxc_config_net_hwaddr(const char *line);
extern void update_hwaddr(const char *line);
extern bool new_hwaddr(char *hwaddr);
extern int lxc_get_conf_str(char *retv, int inlen, const char *value);
extern int lxc_get_conf_bool(struct lxc_conf *c, char *retv, int inlen, bool v);
extern int lxc_get_conf_int(struct lxc_conf *c, char *retv, int inlen, int v);
extern int lxc_get_conf_size_t(struct lxc_conf *c, char *retv, int inlen, size_t v);
extern int lxc_get_conf_uint64(struct lxc_conf *c, char *retv, int inlen, uint64_t v);
Expand Down
21 changes: 14 additions & 7 deletions src/lxc/lxccontainer.c
View file
Open in desktop
Expand Up @@ -2973,6 +2973,10 @@ static bool container_destroy(struct lxc_container *c,
}
}

/* LXC is not managing the storage of the container. */
if (conf && !conf->rootfs.managed)
goto on_success;

if (conf && conf->rootfs.path && conf->rootfs.mount) {
if (!do_destroy_container(conf)) {
ERROR("Error destroying rootfs for %s", c->name);
Expand Down Expand Up @@ -3045,6 +3049,7 @@ static bool container_destroy(struct lxc_container *c,
}
INFO("Destroyed directory \"%s\" for \"%s\"", path, c->name);

on_success:
bret = true;

out:
Expand All @@ -3060,14 +3065,16 @@ static bool do_lxcapi_destroy(struct lxc_container *c)
if (!c || !lxcapi_is_defined(c))
return false;

if (has_snapshots(c)) {
ERROR("Container %s has snapshots; not removing", c->name);
return false;
}
if (c->lxc_conf && c->lxc_conf->rootfs.managed) {
if (has_snapshots(c)) {
ERROR("Container %s has snapshots; not removing", c->name);
return false;
}

if (has_fs_snapshots(c)) {
ERROR("container %s has snapshots on its rootfs", c->name);
return false;
if (has_fs_snapshots(c)) {
ERROR("container %s has snapshots on its rootfs", c->name);
return false;
}
}

return container_destroy(c, NULL);
Expand Down