Refactor RWLayer to use referenced object instead of string

RWLayer will now have more operations and be protected through a referenced type rather than always looked up by string in the layer store. Separates creation of RWLayer (write capture layer) from mounting of the layer. This allows mount labels to be applied after creation and allowing RWLayer objects to have the same lifespan as a container without performance regressions from requiring mount. Signed-off-by: Derek McGowan <[email protected]> (github: dmcgowan)
jawher · Dec 23, 2015 · d04fa49 · d04fa49
1 parent 577cf61
commit d04fa49
Show file tree

Hide file tree

Showing 18 changed files with 388 additions and 236 deletions.
diff --git a/daemon/commit.go b/daemon/commit.go
@@ -145,7 +145,7 @@ func (daemon *Daemon) exportContainerRw(container *container.Container) (archive
 	}
 	return ioutils.NewReadCloserWrapper(archive, func() error {
 			archive.Close()
-			return daemon.layerStore.Unmount(container.ID)
+			return container.RWLayer.Unmount()
 		}),
 		nil
 }
diff --git a/daemon/container_operations_windows.go b/daemon/container_operations_windows.go
@@ -98,7 +98,7 @@ func (daemon *Daemon) populateCommand(c *container.Container, env []string) erro
 		}
 	}
 
-	m, err := daemon.layerStore.Metadata(c.ID)
+	m, err := c.RWLayer.Metadata()
 	if err != nil {
 		return derr.ErrorCodeGetLayerMetadata.WithArgs(err)
 	}

diff --git a/daemon/create.go b/daemon/create.go
@@ -7,6 +7,7 @@ import (
 	"github.com/docker/docker/container"
 	derr "github.com/docker/docker/errors"
 	"github.com/docker/docker/image"
+	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/volume"
@@ -95,6 +96,11 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig) (*container.Con
 		}
 	}()
 
+	// Set RWLayer for container after mount labels have been set
+	if err := daemon.setRWLayer(container); err != nil {
+		return nil, err
+	}
+
 	if err := daemon.createContainerPlatformSpecificSettings(container, params.Config, params.HostConfig, img); err != nil {
 		return nil, err
 	}
@@ -126,6 +132,24 @@ func (daemon *Daemon) generateSecurityOpt(ipcMode containertypes.IpcMode, pidMod
 	return nil, nil
 }
 
+func (daemon *Daemon) setRWLayer(container *container.Container) error {
+	var layerID layer.ChainID
+	if container.ImageID != "" {
+		img, err := daemon.imageStore.Get(container.ImageID)
+		if err != nil {
+			return err
+		}
+		layerID = img.RootFS.ChainID()
+	}
+	rwLayer, err := daemon.layerStore.CreateRWLayer(container.ID, layerID, container.MountLabel, daemon.setupInitLayer)
+	if err != nil {
+		return err
+	}
+	container.RWLayer = rwLayer
+
+	return nil
+}
+
 // VolumeCreate creates a volume with the specified name, driver, and opts
 // This is called directly from the remote API
 func (daemon *Daemon) VolumeCreate(name, driverName string, opts map[string]string) (*types.Volume, error) {

diff --git a/daemon/daemon.go b/daemon/daemon.go
@@ -323,6 +323,13 @@ func (daemon *Daemon) restore() error {
 			continue
 		}
 
+		rwlayer, err := daemon.layerStore.GetRWLayer(container.ID)
+		if err != nil {
+			logrus.Errorf("Failed to load container mount %v: %v", id, err)
+			continue
+		}
+		container.RWLayer = rwlayer
+
 		// Ignore the container if it does not support the current driver being used by the graph
 		if (container.Driver == "" && currentDriver == "aufs") || container.Driver == currentDriver {
 			logrus.Debugf("Loaded container %v", container.ID)
@@ -961,19 +968,7 @@ func (daemon *Daemon) Shutdown() error {
 // Mount sets container.BaseFS
 // (is it not set coming in? why is it unset?)
 func (daemon *Daemon) Mount(container *container.Container) error {
-	var layerID layer.ChainID
-	if container.ImageID != "" {
-		img, err := daemon.imageStore.Get(container.ImageID)
-		if err != nil {
-			return err
-		}
-		layerID = img.RootFS.ChainID()
-	}
-	rwlayer, err := daemon.layerStore.Mount(container.ID, layerID, container.GetMountLabel(), daemon.setupInitLayer)
-	if err != nil {
-		return err
-	}
-	dir, err := rwlayer.Path()
+	dir, err := container.RWLayer.Mount(container.GetMountLabel())
 	if err != nil {
 		return err
 	}
@@ -990,13 +985,12 @@ func (daemon *Daemon) Mount(container *container.Container) error {
 		}
 	}
 	container.BaseFS = dir // TODO: combine these fields
-	container.RWLayer = rwlayer
 	return nil
 }
 
 // Unmount unsets the container base filesystem
 func (daemon *Daemon) Unmount(container *container.Container) {
-	if err := daemon.layerStore.Unmount(container.ID); err != nil {
+	if err := container.RWLayer.Unmount(); err != nil {
 		logrus.Errorf("Error unmounting container %s: %s", container.ID, err)
 	}
 }
@@ -1029,7 +1023,7 @@ func (daemon *Daemon) unsubscribeToContainerStats(c *container.Container, ch cha
 }
 
 func (daemon *Daemon) changes(container *container.Container) ([]archive.Change, error) {
-	return daemon.layerStore.Changes(container.ID)
+	return container.RWLayer.Changes()
 }
 
 // TagImage creates a tag in the repository reponame, pointing to the image named

diff --git a/daemon/delete.go b/daemon/delete.go
@@ -130,7 +130,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
 		return derr.ErrorCodeRmFS.WithArgs(container.ID, err)
 	}
 
-	metadata, err := daemon.layerStore.DeleteMount(container.ID)
+	metadata, err := daemon.layerStore.ReleaseRWLayer(container.RWLayer)
 	layer.LogReleaseMetadata(metadata)
 	if err != nil && err != layer.ErrMountDoesNotExist {
 		return derr.ErrorCodeRmDriverFS.WithArgs(daemon.driver, container.ID, err)

diff --git a/daemon/inspect.go b/daemon/inspect.go
@@ -163,7 +163,7 @@ func (daemon *Daemon) getInspectData(container *container.Container, size bool)
 
 	contJSONBase.GraphDriver.Name = container.Driver
 
-	graphDriverData, err := daemon.layerStore.Metadata(container.ID)
+	graphDriverData, err := container.RWLayer.Metadata()
 	if err != nil {
 		return nil, err
 	}

diff --git a/distribution/xfer/download_test.go b/distribution/xfer/download_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/docker/distribution/digest"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
-	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/progress"
 	"golang.org/x/net/context"
 )
@@ -115,25 +114,18 @@ func (ls *mockLayerStore) Get(chainID layer.ChainID) (layer.Layer, error) {
 func (ls *mockLayerStore) Release(l layer.Layer) ([]layer.Metadata, error) {
 	return []layer.Metadata{}, nil
 }
-
-func (ls *mockLayerStore) Mount(id string, parent layer.ChainID, label string, init layer.MountInit) (layer.RWLayer, error) {
+func (ls *mockLayerStore) CreateRWLayer(string, layer.ChainID, string, layer.MountInit) (layer.RWLayer, error) {
 	return nil, errors.New("not implemented")
 }
 
-func (ls *mockLayerStore) Unmount(id string) error {
-	return errors.New("not implemented")
-}
-
-func (ls *mockLayerStore) DeleteMount(id string) ([]layer.Metadata, error) {
+func (ls *mockLayerStore) GetRWLayer(string) (layer.RWLayer, error) {
 	return nil, errors.New("not implemented")
-}
 
-func (ls *mockLayerStore) Changes(id string) ([]archive.Change, error) {
-	return nil, errors.New("not implemented")
 }
 
-func (ls *mockLayerStore) Metadata(id string) (map[string]string, error) {
+func (ls *mockLayerStore) ReleaseRWLayer(layer.RWLayer) ([]layer.Metadata, error) {
 	return nil, errors.New("not implemented")
+
 }
 
 type mockDownloadDescriptor struct {

diff --git a/layer/layer.go b/layer/layer.go
@@ -31,6 +31,11 @@ var (
 	// attempted on a mount layer which does not exist.
 	ErrMountDoesNotExist = errors.New("mount does not exist")
 
+	// ErrMountNameConflict is used when a mount is attempted
+	// to be created but there is already a mount with the name
+	// used for creation.
+	ErrMountNameConflict = errors.New("mount already exists with name")
+
 	// ErrActiveMount is used when an operation on a
 	// mount is attempted but the layer is still
 	// mounted and the operation cannot be performed.
@@ -103,18 +108,33 @@ type Layer interface {
 type RWLayer interface {
 	TarStreamer
 
-	// Path returns the filesystem path to the writable
-	// layer.
-	Path() (string, error)
+	// Name of mounted layer
+	Name() string
 
 	// Parent returns the layer which the writable
 	// layer was created from.
 	Parent() Layer
 
+	// Mount mounts the RWLayer and returns the filesystem path
+	// the to the writable layer.
+	Mount(mountLabel string) (string, error)
+
+	// Unmount unmounts the RWLayer. This should be called
+	// for every mount. If there are multiple mount calls
+	// this operation will only decrement the internal mount counter.
+	Unmount() error
+
 	// Size represents the size of the writable layer
 	// as calculated by the total size of the files
 	// changed in the mutable layer.
 	Size() (int64, error)
+
+	// Changes returns the set of changes for the mutable layer
+	// from the base layer.
+	Changes() ([]archive.Change, error)
+
+	// Metadata returns the low level metadata for the mutable layer
+	Metadata() (map[string]string, error)
 }
 
 // Metadata holds information about a
@@ -147,11 +167,9 @@ type Store interface {
 	Get(ChainID) (Layer, error)
 	Release(Layer) ([]Metadata, error)
 
-	Mount(id string, parent ChainID, label string, init MountInit) (RWLayer, error)
-	Unmount(id string) error
-	DeleteMount(id string) ([]Metadata, error)
-	Changes(id string) ([]archive.Change, error)
-	Metadata(id string) (map[string]string, error)
+	CreateRWLayer(id string, parent ChainID, mountLabel string, initFunc MountInit) (RWLayer, error)
+	GetRWLayer(id string) (RWLayer, error)
+	ReleaseRWLayer(RWLayer) ([]Metadata, error)
 }
 
 // MetadataTransaction represents functions for setting layer metadata