-
Notifications
You must be signed in to change notification settings - Fork 141
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
Mount test #81
Mount test #81
Conversation
} | ||
|
||
func validateMounts(spec *rspec.Spec) error { | ||
fmt.Println("validating mounts") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably be using a logging framework for these so folks can adjust the verbosity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use logrus which is already imported.
9f5bb68
to
3f334dc
Compare
@@ -0,0 +1 @@ | |||
[code from](github.com/docker/docker/pkg/mount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor thing: If we're going to start pulling code from other repositories we should mention the license (I know it's the same as this project but it's a different project). We should also mention the commit ID of when we pulled it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LICENCE.md and commit ID are added.
d6b98b3
to
49ecdec
Compare
Yes, it is forking and vendoring docker/pkg/mount and does not take much time actually. |
|
||
func validateMountsExistence(spec *rspec.Spec) error { | ||
fmt.Println("validating mounts existence") | ||
infos, err := mount.GetMounts() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: infos -> mounts ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The returned struct of mount.GetMounts() is named of 'Info", so I use infos
previously.
Now it changes to mountInfos.
The check for mount options doesn't work for |
On Mon, Jun 06, 2016 at 02:38:39PM -0700, Mrunal Patel wrote:
Just to clarify, that distinction is ‘mountflags’ vs. ‘data’ in |
@wking Yes, it mountflags vs data. And yes I would think that the filesystem-independent options may or may not be visible in the output. |
Signed-off-by: liang chenye <[email protected]>
Remove 'option' checking for now for three reasons:
|
LGTM. We need to remove cgroups mount from runc config which is non-standard. I'll create a follow-on PR for that. |
validate the runtime mount information by using docker/pkg/mount
and validate the rootfs readonly setting