From bcaf6c2359d83acd5da54f499e21f4a148f491c5 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 5 Dec 2013 22:18:02 +0100 Subject: [PATCH] Add Put() to graphdriver API and use it This makes all users of Put() have a corresponding call to Get() which means we will be able to track whether any particular ID is in use and if not unmount it. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- container.go | 18 +++++++++++++++-- graph.go | 2 ++ graphdriver/aufs/aufs.go | 3 +++ graphdriver/devmapper/driver.go | 3 +++ graphdriver/driver.go | 1 + graphdriver/vfs/driver.go | 5 +++++ image.go | 35 ++++++++++++++++++++++++--------- integration/utils_test.go | 1 + runtime.go | 15 +++++++++++--- utils.go | 27 +++++++++++++++++++++++++ 10 files changed, 96 insertions(+), 14 deletions(-) diff --git a/container.go b/container.go index 3eebb641a1..63afad0ced 100644 --- a/container.go +++ b/container.go @@ -203,6 +203,7 @@ func (container *Container) Inject(file io.Reader, pth string) error { if err := container.EnsureMounted(); err != nil { return fmt.Errorf("inject: error mounting container %s: %s", container.ID, err) } + defer container.Unmount() // Return error if path exists destPath := path.Join(container.RootfsPath(), pth) @@ -1276,6 +1277,7 @@ func (container *Container) ExportRw() (archive.Archive, error) { if container.runtime == nil { return nil, fmt.Errorf("Can't load storage driver for unregistered container %s", container.ID) } + defer container.Unmount() return container.runtime.Diff(container) } @@ -1284,7 +1286,12 @@ func (container *Container) Export() (archive.Archive, error) { if err := container.EnsureMounted(); err != nil { return nil, err } - return archive.Tar(container.RootfsPath(), archive.Uncompressed) + + archive, err := archive.Tar(container.RootfsPath(), archive.Uncompressed) + if err != nil { + return nil, err + } + return EofReader(archive, func() { container.Unmount() }), nil } func (container *Container) WaitTimeout(timeout time.Duration) error { @@ -1409,6 +1416,7 @@ func (container *Container) GetSize() (int64, int64) { utils.Errorf("Warning: failed to compute size of container rootfs %s: %s", container.ID, err) return sizeRw, sizeRootfs } + defer container.Unmount() if differ, ok := container.runtime.driver.(graphdriver.Differ); ok { sizeRw, err = differ.DiffSize(container.ID) @@ -1443,6 +1451,7 @@ func (container *Container) Copy(resource string) (archive.Archive, error) { basePath := path.Join(container.RootfsPath(), resource) stat, err := os.Stat(basePath) if err != nil { + container.Unmount() return nil, err } if !stat.IsDir() { @@ -1453,11 +1462,16 @@ func (container *Container) Copy(resource string) (archive.Archive, error) { filter = []string{path.Base(basePath)} basePath = path.Dir(basePath) } - return archive.TarFilter(basePath, &archive.TarOptions{ + + archive, err := archive.TarFilter(basePath, &archive.TarOptions{ Compression: archive.Uncompressed, Includes: filter, Recursive: true, }) + if err != nil { + return nil, err + } + return EofReader(archive, func() { container.Unmount() }), nil } // Returns true if the container exposes a certain port diff --git a/graph.go b/graph.go index 176626d60a..42da42c8af 100644 --- a/graph.go +++ b/graph.go @@ -97,6 +97,7 @@ func (graph *Graph) Get(name string) (*Image, error) { if err != nil { return nil, fmt.Errorf("Driver %s failed to get image rootfs %s: %s", graph.driver, img.ID, err) } + defer graph.driver.Put(img.ID) var size int64 if img.Parent == "" { @@ -193,6 +194,7 @@ func (graph *Graph) Register(jsonData []byte, layerData archive.Archive, img *Im if err != nil { return fmt.Errorf("Driver %s failed to get image rootfs %s: %s", graph.driver, img.ID, err) } + defer graph.driver.Put(img.ID) img.graph = graph if err := StoreImage(img, jsonData, layerData, tmp, rootfs); err != nil { return err diff --git a/graphdriver/aufs/aufs.go b/graphdriver/aufs/aufs.go index 2e9fa8298a..3070200042 100644 --- a/graphdriver/aufs/aufs.go +++ b/graphdriver/aufs/aufs.go @@ -222,6 +222,9 @@ func (a *Driver) Get(id string) (string, error) { return out, nil } +func (a *Driver) Put(id string) { +} + // Returns an archive of the contents for the id func (a *Driver) Diff(id string) (archive.Archive, error) { return archive.TarFilter(path.Join(a.rootPath(), "diff", id), &archive.TarOptions{ diff --git a/graphdriver/devmapper/driver.go b/graphdriver/devmapper/driver.go index 10ac172562..0bf5fb24e1 100644 --- a/graphdriver/devmapper/driver.go +++ b/graphdriver/devmapper/driver.go @@ -97,6 +97,9 @@ func (d *Driver) Get(id string) (string, error) { return path.Join(mp, "rootfs"), nil } +func (d *Driver) Put(id string) { +} + func (d *Driver) mount(id, mountPoint string) error { // Create the target directories if they don't exist if err := osMkdirAll(mountPoint, 0755); err != nil && !osIsExist(err) { diff --git a/graphdriver/driver.go b/graphdriver/driver.go index 1d5995dffc..2be3f05f3a 100644 --- a/graphdriver/driver.go +++ b/graphdriver/driver.go @@ -17,6 +17,7 @@ type Driver interface { Remove(id string) error Get(id string) (dir string, err error) + Put(id string) Exists(id string) bool Status() [][2]string diff --git a/graphdriver/vfs/driver.go b/graphdriver/vfs/driver.go index 12230f463a..21da63878a 100644 --- a/graphdriver/vfs/driver.go +++ b/graphdriver/vfs/driver.go @@ -84,6 +84,11 @@ func (d *Driver) Get(id string) (string, error) { return dir, nil } +func (d *Driver) Put(id string) { + // The vfs driver has no runtime resources (e.g. mounts) + // to clean up, so we don't need anything here +} + func (d *Driver) Exists(id string) bool { _, err := os.Stat(d.dir(id)) return err == nil diff --git a/image.go b/image.go index f062910ef8..7652824d49 100644 --- a/image.go +++ b/image.go @@ -104,6 +104,7 @@ func StoreImage(img *Image, jsonData []byte, layerData archive.Archive, root, la if err != nil { return err } + defer driver.Put(img.Parent) changes, err := archive.ChangesDirs(layer, parent) if err != nil { return err @@ -147,7 +148,7 @@ func jsonPath(root string) string { } // TarLayer returns a tar archive of the image's filesystem layer. -func (img *Image) TarLayer() (archive.Archive, error) { +func (img *Image) TarLayer() (arch archive.Archive, err error) { if img.graph == nil { return nil, fmt.Errorf("Can't load storage driver for unregistered image %s", img.ID) } @@ -160,19 +161,35 @@ func (img *Image) TarLayer() (archive.Archive, error) { if err != nil { return nil, err } + + defer func() { + if err == nil { + driver.Put(img.ID) + } + }() + if img.Parent == "" { - return archive.Tar(imgFs, archive.Uncompressed) - } else { - parentFs, err := driver.Get(img.Parent) + archive, err := archive.Tar(imgFs, archive.Uncompressed) if err != nil { return nil, err } - changes, err := archive.ChangesDirs(imgFs, parentFs) - if err != nil { - return nil, err - } - return archive.ExportChanges(imgFs, changes) + return EofReader(archive, func() { driver.Put(img.ID) }), nil } + + parentFs, err := driver.Get(img.Parent) + if err != nil { + return nil, err + } + defer driver.Put(img.Parent) + changes, err := archive.ChangesDirs(imgFs, parentFs) + if err != nil { + return nil, err + } + archive, err := archive.ExportChanges(imgFs, changes) + if err != nil { + return nil, err + } + return EofReader(archive, func() { driver.Put(img.ID) }), nil } func ValidateID(id string) error { diff --git a/integration/utils_test.go b/integration/utils_test.go index 63ac3a44b9..69686ba69e 100644 --- a/integration/utils_test.go +++ b/integration/utils_test.go @@ -74,6 +74,7 @@ func containerFileExists(eng *engine.Engine, id, dir string, t utils.Fataler) bo if err := c.EnsureMounted(); err != nil { t.Fatal(err) } + defer c.Unmount() if _, err := os.Stat(path.Join(c.RootfsPath(), dir)); err != nil { if os.IsNotExist(err) { return false diff --git a/runtime.go b/runtime.go index 52f03f84be..5576cb2aee 100644 --- a/runtime.go +++ b/runtime.go @@ -134,6 +134,7 @@ func (runtime *Runtime) Register(container *Container) error { if err != nil { return fmt.Errorf("Error getting container filesystem %s from driver %s: %s", container.ID, runtime.driver, err) } + defer runtime.driver.Put(container.ID) container.rootfs = rootfs container.runtime = runtime @@ -460,6 +461,8 @@ func (runtime *Runtime) Create(config *Config, name string) (*Container, []strin if err != nil { return nil, nil, err } + defer runtime.driver.Put(initID) + if err := setupInitLayer(initPath); err != nil { return nil, nil, err } @@ -520,6 +523,7 @@ func (runtime *Runtime) Commit(container *Container, repository, tag, comment, a if err := container.EnsureMounted(); err != nil { return nil, err } + defer container.Unmount() rwTar, err := container.ExportRw() if err != nil { @@ -763,8 +767,7 @@ func (runtime *Runtime) Mount(container *Container) error { } func (runtime *Runtime) Unmount(container *Container) error { - // FIXME: Unmount is deprecated because drivers are responsible for mounting - // and unmounting when necessary. Use driver.Remove() instead. + runtime.driver.Put(container.ID) return nil } @@ -776,10 +779,12 @@ func (runtime *Runtime) Changes(container *Container) ([]archive.Change, error) if err != nil { return nil, fmt.Errorf("Error getting container rootfs %s from driver %s: %s", container.ID, container.runtime.driver, err) } + defer runtime.driver.Put(container.ID) initDir, err := runtime.driver.Get(container.ID + "-init") if err != nil { return nil, fmt.Errorf("Error getting container init rootfs %s from driver %s: %s", container.ID, container.runtime.driver, err) } + defer runtime.driver.Put(container.ID + "-init") return archive.ChangesDirs(cDir, initDir) } @@ -798,7 +803,11 @@ func (runtime *Runtime) Diff(container *Container) (archive.Archive, error) { return nil, fmt.Errorf("Error getting container rootfs %s from driver %s: %s", container.ID, container.runtime.driver, err) } - return archive.ExportChanges(cDir, changes) + archive, err := archive.ExportChanges(cDir, changes) + if err != nil { + return nil, err + } + return EofReader(archive, func() { runtime.driver.Put(container.ID) }), nil } func (runtime *Runtime) Run(c *Container, startCallback execdriver.StartCallback) (int, error) { diff --git a/utils.go b/utils.go index f0591158a4..e3ba08d51c 100644 --- a/utils.go +++ b/utils.go @@ -5,8 +5,10 @@ import ( "github.com/dotcloud/docker/archive" "github.com/dotcloud/docker/pkg/namesgenerator" "github.com/dotcloud/docker/utils" + "io" "strconv" "strings" + "sync/atomic" ) type Change struct { @@ -339,3 +341,28 @@ func (c *checker) Exists(name string) bool { func generateRandomName(runtime *Runtime) (string, error) { return namesgenerator.GenerateRandomName(&checker{runtime}) } + +// Read an io.Reader and call a function when it returns EOF +func EofReader(r io.Reader, callback func()) *eofReader { + return &eofReader{ + Reader: r, + callback: callback, + } +} + +type eofReader struct { + io.Reader + gotEOF int32 + callback func() +} + +func (r *eofReader) Read(p []byte) (n int, err error) { + n, err = r.Reader.Read(p) + if err == io.EOF { + // Use atomics to make the gotEOF check threadsafe + if atomic.CompareAndSwapInt32(&r.gotEOF, 0, 1) { + r.callback() + } + } + return +}