Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented Apr 2, 2018

We've had two ways to specify a process to launch (for the container process and for hooks) for a while now. Last week, we landed #949 with a third process-launching structure. This pull request replaced that third structure with a recycled hook-process schema for launcing the hypervisor. I prefer using the more-fundamental container process schema, but @crosbymichael felt it had too many properties that wouldn't apply to the hypervisor-launching case. Versus master, this pull request adds:

  • The ability to set argv[0] independently of path (like execv and similar).
  • The ability to set env and timeout for the hypervisor process.
  • The ability to lean on existing hook-launching code for launching hypervisor processes, with the difference being just whether you pass the state in on stdin or not.

And removes:

If we get this or a similar unification landed before we cut a release that includes #949, then I think this change is all positive with no downside. If we punt on this until after 1.1.0, then this is probably just going to end up as one of those unfortunate historical quirks that are not worth the migration pain of fixing.

Ping @sameo and @crosbymichael, since they're the only two who have expressed opinions on this sort of thing so far.

wking added 2 commits March 21, 2018 09:23
So we'll be able to link to it without linking the rest of the 'hooks'
structure.

Signed-off-by: W. Trevor King <[email protected]>
We already have two ways to specify a process to launch (for the
container process and for hooks).  This commit recycles the
hook-process schema for launching the hypervisor.  I prefer using the
more-fundamental container process schema, but Michael felt it had too
many properties that wouldn't apply to the hypervisor-launching case
[1].

[1]: opencontainers#949 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@vbatts
Copy link
Member

vbatts commented Dec 17, 2019

house keeping: i'm inclined to close this for now. While I am all for re-using schema, this slight change imposes a change and hitching VM intricacies to the other hooks (which sounds fine, but may not align to ongoing reality)

@vbatts vbatts closed this Dec 17, 2019
@vbatts
Copy link
Member

vbatts commented Dec 17, 2019

(decided to close, since the neither of the two reviewers have chimed in on this in 1.5yrs...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants