Skip to content

skip non-existant volume host path#492

Closed
keloyang wants to merge 1 commit intoopencontainers:masterfrom
keloyang:skip-non-existant-volume
Closed

skip non-existant volume host path#492
keloyang wants to merge 1 commit intoopencontainers:masterfrom
keloyang:skip-non-existant-volume

Conversation

@keloyang
Copy link
Copy Markdown
Contributor

currently, docker create non-existent volume host path automatically.see the code of docker

func (m *MountPoint) Setup() (string, error) {
    if m.Volume != nil {
        return m.Volume.Mount()
    }
    if len(m.Source) > 0 {
        if _, err := os.Stat(m.Source); err != nil {
            if !os.IsNotExist(err) {
                return "", err
            }
            if runtime.GOOS != "windows" { // Windows does not have deprecation issues here
                logrus.Warnf("Auto-creating non-existent volume host path %s, this is deprecated and will be removed soon", m.Source)
                if err := system.MkdirAll(m.Source, 0755); err != nil {
                    return "", err
                }
            }
        }
        return m.Source, nil
    }
    return "", derr.ErrorCodeMountSetup
}

e.g.

docker run --rm -ti -v /hello:/hello ubuntu bash

if the /hello dir don't exist in host, docker will create it to avoid the fail of mount.

But it is not reasonable.in fact,we can skip non-existant volume host path and remove "MkdirAll" from Setup.

Signed-off-by: yangshukui [email protected]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this fmt.Print is going to be noisy to anyone importing this pkg

@crosbymichael
Copy link
Copy Markdown
Member

I don't think this is the level to do this type of change to, if the source does not exist for the mount then there is nothing that libcontainer can do about it and it should fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants