-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix bug convert volume host path to absolute #3537
Conversation
pkg/spec/storage.go
Outdated
@@ -663,6 +664,12 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string | |||
|
|||
if strings.HasPrefix(src, "/") || strings.HasPrefix(src, ".") { | |||
// This is not a named volume | |||
// convert src to an absolute path | |||
src, err := filepath.Abs(src) | |||
log.Println("src is , ", src) |
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.
This looks like debugging information.
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.
Can you use source
as the new variable, so we can refer back to src.
pkg/spec/storage.go
Outdated
src, err := filepath.Abs(src) | ||
log.Println("src is , ", src) | ||
if err != nil { | ||
return nil, nil, errors.Wrapf(err, "error getting absolute source path") |
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.
Could you wrap include the src
directory.
pkg/spec/storage.go
Outdated
@@ -663,10 +663,15 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string | |||
|
|||
if strings.HasPrefix(src, "/") || strings.HasPrefix(src, ".") { | |||
// This is not a named volume | |||
// convert src to an absolute path | |||
source, err := filepath.Abs(src) |
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.
This is a bad place to handle things since it only hits the --volume
flag and not --mount
. Better place would be the loop at https://github.com/containers/libpod/blob/943c8daf6db1ec9995f0e7a8f1392c2d83dc4376/pkg/spec/storage.go#L203-L214 - after we check for tmpfs, check if it's a bind mount, and if it is do a filepath.Abs() on the source path.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, QiWang19 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Needs rebase to fix tests |
fix containers#3504 If --volume host:dest host is not a named volume, convert the host to a absolute directory path. Signed-off-by: Qi Wang <[email protected]>
@mheon rebased, PTAL |
LGTM on my end. @vrothberg @giuseppe @rhatdan PTAL |
/lgtm |
fix #3504 If --volume host:dest host is not a named volume, convert the host to a absolute directory path.
Signed-off-by: Qi Wang [email protected]