Skip to content

Fetch the environments for the default global options#216

Closed
laijs wants to merge 2 commits intoopencontainers:masterfrom
laijs:env-as-default-global-option
Closed

Fetch the environments for the default global options#216
laijs wants to merge 2 commits intoopencontainers:masterfrom
laijs:env-as-default-global-option

Conversation

@laijs
Copy link
Copy Markdown
Contributor

@laijs laijs commented Aug 21, 2015

So thus the user doesn't need to specific the global option on every execution.

get the default root path based on the environment OCI_ROOT
get the default criu path based on the environment RUNC_CRIU

laijs added 2 commits August 21, 2015 10:42
Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
@wking
Copy link
Copy Markdown
Contributor

wking commented Aug 21, 2015

On Thu, Aug 20, 2015 at 08:00:43PM -0700, Lai Jiangshan wrote:

  • get the default root path based on the environment OCI_ROOT
  • get the default criu path based on the environment RUNC_CRIU

It seems strange to use two different prefixes (OCI_ and RUNC_). I'm
still not clear on the need to share state via a collaborative
filesystem location (see 1 and my comments in #159), but if we have
all OCI implementations agreeing on where they store their state, I
don't see why we wouldn't have them agree on which CRIU command to
use.

@laijs
Copy link
Copy Markdown
Contributor Author

laijs commented Aug 21, 2015

Not all implementation needs CRIU. The hypervisor based implementations[1] don't need it. So criu is runc (or some implementations, not all) specialized option, so the environment starts with prefix RUNC_. And the collaborative root directory is (will be) required for all implementations. so the environment starts with prefix OCI_.

[1] https://github.com/hyperhq/runv

@wking
Copy link
Copy Markdown
Contributor

wking commented Aug 21, 2015

On Fri, Aug 21, 2015 at 12:24:24AM -0700, Lai Jiangshan wrote:

Not all implementation needs CRIU. The hypervisor based
implementations[1] don't need it. So criu is runc…

That makes sense, but “we don't all need $X” isn't enough to require
separate namespacing (runV will work fine with OCI_CRIU set). What
you need is “two of us need $X with different semantics”, and I expect
most OCI-related agents will have the same semantics for CRIU. The
first one with a non-standard CRIU requirement can leave the OCI_
namespace and add their own per-implementation variable.

@crosbymichael
Copy link
Copy Markdown
Member

I don't think env vars give us anything there. These are already flags on runc so why add yet another way to change the value?

@wking
Copy link
Copy Markdown
Contributor

wking commented Sep 14, 2015

On Mon, Sep 14, 2015 at 04:31:34PM -0700, Michael Crosby wrote:

I don't think env vars give us anything there. These are already
flags on runc so why add yet another way to change the value?

+1 to “we should only have one way to change the value”.

@crosbymichael
Copy link
Copy Markdown
Member

Lets close this and just go for the flags right now for setting these options.

stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
…ommits

.travis: Convert TRAVIS_COMMIT_RANGE base...head to base..head
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
Through 3297cd5 (Merge pull request opencontainers#216 from
wking/travis-test-branch-commits, 2017-01-24).

Signed-off-by: W. Trevor King <wking@tremily.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants