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

[Linux Config] Clarify which filesystem is being referenced #577

Closed
wants to merge 3 commits into from

Conversation

RobDolinMS
Copy link
Collaborator

Signed-off-by: Rob Dolin robdolin@microsoft.com

Signed-off-by: Rob Dolin <robdolin@microsoft.com>
@@ -8,7 +8,7 @@ The Linux container specification uses various kernel features like namespaces,
The Linux ABI includes both syscalls and several special file paths.
Applications expecting a Linux environment will very likely expect these files paths to be setup correctly.

The following filesystems MUST be made available in each application's filesystem
The following filesystems MUST be made available in each containerized application's filesystem
Copy link
Contributor

@wking wking Sep 19, 2016

Choose a reason for hiding this comment

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

@crosbymichael pointed out a while back that we do not define “application”. Can we use “… in the container mount namespace:”? The glossary does define “container namespace”.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

You can just say in each container's filesystem.

@wking you are using the term incorrectly when referring to namespaces. A mount namespace is nothing more than a new mounttable for the container and is decoupled from the root filesystem.

@wking
Copy link
Contributor

wking commented Sep 20, 2016

On Tue, Sep 20, 2016 at 09:21:24AM -0700, Michael Crosby wrote:

@@ -8,7 +8,7 @@ The Linux container specification uses various kernel features like namespaces,
The Linux ABI includes both syscalls and several special file paths.
Applications expecting a Linux environment will very likely expect these files paths to be setup correctly.

-The following filesystems MUST be made available in each application's filesystem
+The following filesystems MUST be made available in each containerized application's filesystem

You can just say in each container's filesystem.

@wking you are using the term incorrectly when referring to
namespaces. A mount namespace is nothing more than a new mounttable
for the container and is decoupled from the root filesystem.

And that mount table is where these filesystems must be available,
isn't it? mount(8) calls the whole tree available from / the “file
hierarchy” and “the big file tree” ;). I'm ok with something like
that, but “the container mount namespace” seems to clearly cover the
same idea. Talking about “the container's filesystem”, on the other
hand, sounds more like a single mount, and less like the whole tree.

Updating language as suggested by Michael Crosby in comments

Signed-off-by: Rob Dolin <robdolin@microsoft.com>
Copy link
Collaborator Author

@RobDolinMS RobDolinMS left a comment

Choose a reason for hiding this comment

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

Updating per @crosbymichael

@mrunalp
Copy link
Contributor

mrunalp commented Sep 20, 2016

@RobDolinMS Could you squash the commits?

@hqhq
Copy link
Contributor

hqhq commented Oct 5, 2016

ping @RobDolinMS

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

LGTM

Any reason we can't just use GitHub's "squash-and-merge" if there's a strong preference for this to be a single commit?

@tianon
Copy link
Member

tianon commented Nov 4, 2016

LGTM

(repeated to appease PullApprove.....)

Approved with PullApprove

@mrunalp
Copy link
Contributor

mrunalp commented Nov 10, 2016

Needs rebase :/

@RobDolinMS RobDolinMS added this to the 1.0.0 milestone Jan 11, 2017
@RobDolinMS RobDolinMS self-assigned this Jan 11, 2017
@RobDolinMS
Copy link
Collaborator Author

Signed-off-by: Rob Dolin robdolin@microsoft.com

RobDolinMS added a commit that referenced this pull request Jan 17, 2017
Replaces PR #577

Signed-off-by: Rob Dolin <robdolin@microsoft.com>
@RobDolinMS
Copy link
Collaborator Author

Replaced with #658.

@RobDolinMS RobDolinMS closed this Jan 17, 2017
RobDolinMS added a commit that referenced this pull request Jan 18, 2017
Replaces #577

Signed-off-by: Rob Dolin (MSFT) <robdolin@microsoft.com>
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.

7 participants