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.

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

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

Merged
merged 3 commits into from Aug 30, 2018

Conversation

brauner
Copy link
Member

@brauner brauner commented Jun 27, 2018

This introduces a new config key lxc.rootfs.managed which can be used to
indicate whether this LXC instance is managing the container storage. If LXC is
not managing the storage then LXC will not modify the container storage.
For example, an API call to c->destroy(c) will then run any destroy hooks but
will not destroy the actual rootfs (Unless, of course, the hook does so behind
LXC's back.).

Signed-off-by: Christian Brauner christian.brauner@ubuntu.com
CC: Wolfgang Bumiller w.bumiller@proxmox.com
CC: Stéphane Graber stgraber@ubuntu.com
CC: Serge Hallyn serge@hallyn.com
CC: 2xsec dh48.jeong@samsung.com

@brauner brauner requested review from hallyn and stgraber June 27, 2018 09:42
@brauner
Copy link
Member Author

brauner commented Jun 27, 2018

//cc @Blub, @2xsec

@stgraber
Copy link
Member

lxc.rootfs.managed maybe? to keep everything namespaced together

@brauner
Copy link
Member Author

brauner commented Jun 27, 2018

Yeah, might be a good idea.

@brauner brauner force-pushed the 2018-06-27/storage_managed branch 2 times, most recently from 6ba9434 to d5bb5d1 Compare June 28, 2018 09:31
@brauner
Copy link
Member Author

brauner commented Jun 28, 2018

@stgraber, updated.

@@ -2645,6 +2645,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.

@brauner brauner changed the title [RFC] conf: introduce lxc.storage.managed [RFC] conf: introduce lxc.rootfs.managed Jun 28, 2018
@brauner brauner force-pushed the 2018-06-27/storage_managed branch from d5bb5d1 to 97dc156 Compare June 28, 2018 21:44
@brauner brauner force-pushed the 2018-06-27/storage_managed branch from 97dc156 to 4dd4131 Compare July 31, 2018 16:04
@brauner
Copy link
Member Author

brauner commented Jul 31, 2018

Updated.

Christian Brauner added 3 commits July 31, 2018 22:09
This introduces a new config key lxc.rootfs.managed which can be used to
indicate whether this LXC instance is managing the container storage. If LXC is
not managing the storage then LXC will not modify the container storage.
For example, an API call to c->destroy(c) will then run any destroy hooks but
will not destroy the actual rootfs (Unless, of course, the hook does so behind
LXC's back.).

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
CC: Wolfgang Bumiller <w.bumiller@proxmox.com>
CC: Stéphane Graber <stgraber@ubuntu.com>
CC: Serge Hallyn <serge@hallyn.com>
CC: 2xsec <dh48.jeong@samsung.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
@brauner brauner force-pushed the 2018-06-27/storage_managed branch from 4dd4131 to 6e0045b Compare July 31, 2018 20:09
@Blub
Copy link
Member

Blub commented Aug 29, 2018

LGTM, the only question I have is what to do about (other) files in /var/lib/lxc/$container, eg the config file. Currently an lxc-destroy on a container which otherwise sits in /var/lib/lxc regularly won't remove anything when lxc.rootfs.managed=0, iow. it's nop. I suppose it makes sense if they're setup only via liblxc rather than an actual dir in /var/lib/lxc, just thought I'd ask.

@brauner
Copy link
Member Author

brauner commented Aug 29, 2018

Yeah, my current idea is to basically treat the directory as a no-go area for destroy when managed is set to 0.

@Blub Blub merged commit 6b28940 into lxc:master Aug 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants