Separate linux-specific config from os-independent config #2
Comments
crosbymichael
pushed a commit
that referenced
this issue
Jun 17, 2015
Fix cross compile for network and node pkgs
|
Closing this as we moved this work into opencontainers/runtime-spec#7 and have made the separation. |
jhjeong-kr
added a commit
to jhjeong-kr/runc
that referenced
this issue
Jul 19, 2015
runc always shows "container in use" if /var/run/ocf/container exists
However, there are two cases
1) case 1: "container in use"
2) case 2: /var/run/ocf/container still exists after runc was terminated by SIGKILL or abnormal crash
For case 2, runc should yield "delete the lock dir" instead of "container in use"
This patch is for this issue using "pid" file in /var/run/ocf/container/task
minor revision opencontainers#1: error indentation, slice length check for "exeName"
minor revision opencontainers#2: use "filepath.join" instead of "fmt.Sprintf"
Signed-off-by: Jin-Hwan Jeong <[email protected]>
sallyom
pushed a commit
to sallyom/runc
that referenced
this issue
Jul 30, 2015
TL;DR: check for IsExist(err) after a failed MkdirAll() is both
redundant and wrong -- so two reasons to remove it.
Quoting MkdirAll documentation:
> MkdirAll creates a directory named path, along with any necessary
> parents, and returns nil, or else returns an error. If path
> is already a directory, MkdirAll does nothing and returns nil.
This means two things:
1. If a directory to be created already exists, no error is
returned.
2. If the error returned is IsExist (EEXIST), it means there exists
a non-directory with the same name as MkdirAll need to use for
directory. Example: we want to MkdirAll("a/b"), but file "a"
(or "a/b") already exists, so MkdirAll fails.
The above is a theory, based on quoted documentation and my UNIX
knowledge.
3. In practice, though, current MkdirAll implementation [1] returns
ENOTDIR in most of cases described in opencontainers#2, with the exception when
there is a race between MkdirAll and someone else creating the
last component of MkdirAll argument as a file. In this very case
MkdirAll() will indeed return EEXIST.
Because of opencontainers#1, IsExist check after MkdirAll is not needed.
Because of opencontainers#2 and opencontainers#3, ignoring IsExist error is just plain wrong,
as directory we require is not created. It's cleaner to report
the error now.
Note this error is all over the tree, I guess due to copy-paste,
or trying to follow the same usage pattern as for Mkdir(),
or some not quite correct examples on the Internet.
[1] https://github.com/golang/go/blob/f9ed2f75/src/os/path.go
Signed-off-by: Kir Kolyshkin <[email protected]>
clnperez
pushed a commit
to clnperez/runc
that referenced
this issue
Aug 12, 2015
TL;DR: check for IsExist(err) after a failed MkdirAll() is both
redundant and wrong -- so two reasons to remove it.
Quoting MkdirAll documentation:
> MkdirAll creates a directory named path, along with any necessary
> parents, and returns nil, or else returns an error. If path
> is already a directory, MkdirAll does nothing and returns nil.
This means two things:
1. If a directory to be created already exists, no error is
returned.
2. If the error returned is IsExist (EEXIST), it means there exists
a non-directory with the same name as MkdirAll need to use for
directory. Example: we want to MkdirAll("a/b"), but file "a"
(or "a/b") already exists, so MkdirAll fails.
The above is a theory, based on quoted documentation and my UNIX
knowledge.
3. In practice, though, current MkdirAll implementation [1] returns
ENOTDIR in most of cases described in opencontainers#2, with the exception when
there is a race between MkdirAll and someone else creating the
last component of MkdirAll argument as a file. In this very case
MkdirAll() will indeed return EEXIST.
Because of opencontainers#1, IsExist check after MkdirAll is not needed.
Because of opencontainers#2 and opencontainers#3, ignoring IsExist error is just plain wrong,
as directory we require is not created. It's cleaner to report
the error now.
Note this error is all over the tree, I guess due to copy-paste,
or trying to follow the same usage pattern as for Mkdir(),
or some not quite correct examples on the Internet.
[1] https://github.com/golang/go/blob/f9ed2f75/src/os/path.go
Signed-off-by: Kir Kolyshkin <[email protected]>
ghost
mentioned this issue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As discussed, we like the idea of having 2 different kinds of configuration in the manifest: 1) os-specific config (for example:
create a new pid namespace, orset cgroup foo to bar, ordrop CAP_SYSADMIN), and 2) os-independent config (for example:execute /bin/bash, orset environment DEBUG to 1).Currently these 2 different kinds of configs are mixed in the manifest. We should segment them more clearly, to make more visible the tradeoff between control and portability, and to allow for new sections to be created for other OSes - Windows, Solaris, FreeBSD etc.
The text was updated successfully, but these errors were encountered: