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

runtimetest: add root filesystem validation #78

Merged

Conversation

Mashimiao
Copy link

Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com

@liangchenye
Copy link
Member

liangchenye commented May 26, 2016

Hi @Mashimiao I just submitted a PR #81 which does the same thing, but by using mount information.

I think it might be wrong to verify the readonly property of the root file system by using os.stat.
if you mount yourPartition -o /mnt/readonly-point and use os.stat, it might still show us 'drwxr-xr-x. But when you toucha` file in /tmp/readonly-point, it will tell:

touch: cannot touch 'a': Read-only file system

@liangchenye liangchenye mentioned this pull request May 26, 2016
76 tasks
@wking
Copy link
Contributor

wking commented May 26, 2016

On Thu, May 26, 2016 at 03:43:54AM -0700, 梁辰晔 (Liang Chenye) wrote:

if you mount yourPartition -o /mnt/readonly-point and use os.stat,
it might still have write permission. But when you touch a file in
/tmp/readonly-point…

Ah, good point. We should definitely be testing read-only by
attempting writes, because the per-file POSIX-mode permissions are
distinct from the per-mount filesystem options.

@Mashimiao
Copy link
Author

@liangchenye @wking re-implemented with pre-write

@wking
Copy link
Contributor

wking commented May 27, 2016

On Thu, May 26, 2016 at 09:47:37PM -0700, Ma Shimiao wrote:

@liangchenye @wking re-implemented with pre-write

I'll leave it to you and @liangchenye to decide whether this rerolled
version takes precedence over #81 ;).

@liangchenye
Copy link
Member

liangchenye commented May 27, 2016

@wking Ah! I'll rollback my second commit in #81 :)

}

tmpfile.Close()
os.RemoveAll(filepath.Join(path,tmpfile.Name()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably needs gofmt run.

Copy link
Author

Choose a reason for hiding this comment

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

@mrunalp Ah, sorry. fixed.

Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
@Mashimiao Mashimiao force-pushed the runtime-test-root-fs-validation branch from e78cae6 to d9b2746 Compare May 28, 2016 01:30
@mrunalp
Copy link
Contributor

mrunalp commented May 31, 2016

LGTM

@mrunalp mrunalp merged commit 923e88c into opencontainers:master May 31, 2016
@Mashimiao Mashimiao deleted the runtime-test-root-fs-validation branch November 14, 2016 09:33
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.

4 participants