This repository was archived by the owner on Mar 9, 2022. It is now read-only.
Get the mountInfo by 'LookupMount' in containerd #295
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
There is something wrong in my local environment, |
Random-Liu
reviewed
Sep 28, 2017
| return false | ||
| } | ||
|
|
||
| //TODO: Replace with `mount.Lookup()`in containerd after #257 is marged |
Member
There was a problem hiding this comment.
Why do we still need this loop?
LookupMount checks device id directly: https://github.com/containerd/containerd/blob/master/mount/lookup_unix.go
miaoyq
commented
Sep 28, 2017
pkg/server/helpers.go
Outdated
| } | ||
|
|
||
| path := source | ||
| for { |
Member
Author
There was a problem hiding this comment.
Seems not necessary, I will remove this loop, Thanks :-)
Member
|
Yeah
…On Sep 27, 2017 7:05 PM, "MiaoYanqiang" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkg/server/helpers.go
<#295 (comment)>
:
> -//TODO: Replace with `mount.Lookup()`in containerd after #257 is marged
-func getMountInfo(mountInfos []*mount.Info, dir string) *mount.Info {
- for _, m := range mountInfos {
- if m.Mountpoint == dir {
- return m
- }
- }
- return nil
-}
-
-func getSourceMount(source string, mountInfos []*mount.Info) (string, string, error) {
- mountinfo := getMountInfo(mountInfos, source)
- if mountinfo != nil {
+func getSourceMount(source string, lookupMount func(string) (mount.Info, error)) (string, string, error) {
+ mountinfo, err := lookupMount(source)
+ if err == nil {
return source, mountinfo.Optional, nil
}
path := source
for {
@Random-Liu <https://github.com/random-liu> Do you mean this loop?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#295 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFjVu9MgpC_uxJFwfAlrk8X_ihhmRwqmks5smv7dgaJpZM4PlusT>
.
|
miaoyq
force-pushed
the
use-mount-lookup
branch
from
bb1ddde to
bcd0950
Compare
Member
Author
|
@Random-Liu Have updated the code based on #342, PTAL |
Random-Liu
reviewed
Oct 11, 2017
pkg/server/container_create.go
Outdated
| func ensureShared(path string, mountInfos []*mount.Info) error { | ||
| sourceMount, optionalOpts, err := getSourceMount(path, mountInfos) | ||
| func ensureShared(path string, lookupMount func(string) (mount.Info, error)) error { | ||
| sourceMount, optionalOpts, err := getSourceMount(path, lookupMount) |
Member
There was a problem hiding this comment.
Do we still need getSourceMount? I think we could just call lookupMount directly here.
pkg/server/container_create_test.go
Outdated
| "github.com/containerd/containerd/contrib/apparmor" | ||
| "github.com/containerd/containerd/contrib/seccomp" | ||
| "github.com/docker/docker/pkg/mount" | ||
| //"github.com/docker/docker/pkg/mount" |
Member
|
@miaoyq LGTM overall other than the comments. |
miaoyq
force-pushed
the
use-mount-lookup
branch
from
bcd0950 to
c65921b
Compare
Signed-off-by: Yanqiang Miao <[email protected]>
Member
|
/lgtm |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Yanqiang Miao [email protected]
Address this comment.
/cc @Random-Liu