From dee6b481fe0da1d845261ffff2e610fb05898d3c Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Wed, 10 Sep 2014 20:30:52 -0700 Subject: [PATCH 1/2] Refactor use of graphdriver.Differ Some graphdrivers are Differs and type assertions are made in various places throughout the project. Differ offers some convenience in generating/applying diffs of filesystem layers but for most graphdrivers another code path is taken. This patch brings all of the logic related to filesystem diffs in one place, and simplifies the implementation of some common types like Image, Daemon, and Container. Signed-off-by: Josh Hawn --- archive/changes.go | 7 +- daemon/container.go | 23 ++-- daemon/daemon.go | 41 +------ daemon/graphdriver/aufs/aufs.go | 36 ++++-- daemon/graphdriver/aufs/aufs_test.go | 16 +-- daemon/graphdriver/btrfs/btrfs.go | 6 +- daemon/graphdriver/devmapper/driver.go | 2 +- daemon/graphdriver/driver.go | 41 ++++++- daemon/graphdriver/fsdiff.go | 162 +++++++++++++++++++++++++ daemon/graphdriver/vfs/driver.go | 2 +- graph/graph.go | 31 +---- image/image.go | 87 +------------ 12 files changed, 263 insertions(+), 191 deletions(-) create mode 100644 daemon/graphdriver/fsdiff.go diff --git a/archive/changes.go b/archive/changes.go index 231f3064c1..f80f7219be 100644 --- a/archive/changes.go +++ b/archive/changes.go @@ -313,7 +313,8 @@ func collectFileInfo(sourceDir string) (*FileInfo, error) { return root, nil } -// Compare two directories and generate an array of Change objects describing the changes +// ChangesDirs compares two directories and generates an array of Change objects describing the changes. +// If oldDir is "", then all files in newDir will be Add-Changes. func ChangesDirs(newDir, oldDir string) ([]Change, error) { var ( oldRoot, newRoot *FileInfo @@ -321,7 +322,9 @@ func ChangesDirs(newDir, oldDir string) ([]Change, error) { errs = make(chan error, 2) ) go func() { - oldRoot, err1 = collectFileInfo(oldDir) + if oldDir != "" { + oldRoot, err1 = collectFileInfo(oldDir) + } errs <- err1 }() go func() { diff --git a/daemon/container.go b/daemon/container.go index 014899fc3c..963af1b915 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -18,7 +18,6 @@ import ( "github.com/docker/docker/archive" "github.com/docker/docker/daemon/execdriver" - "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/engine" "github.com/docker/docker/image" "github.com/docker/docker/links" @@ -755,21 +754,13 @@ func (container *Container) GetSize() (int64, int64) { } defer container.Unmount() - if differ, ok := driver.(graphdriver.Differ); ok { - sizeRw, err = differ.DiffSize(container.ID) - if err != nil { - log.Errorf("Warning: driver %s couldn't return diff size of container %s: %s", driver, container.ID, err) - // FIXME: GetSize should return an error. Not changing it now in case - // there is a side-effect. - sizeRw = -1 - } - } else { - changes, _ := container.changes() - if changes != nil { - sizeRw = archive.ChangesSize(container.basefs, changes) - } else { - sizeRw = -1 - } + initID := fmt.Sprintf("%s-init", container.ID) + sizeRw, err = driver.DiffSize(container.ID, initID) + if err != nil { + log.Errorf("Warning: driver %s couldn't return diff size of container %s: %s", driver, container.ID, err) + // FIXME: GetSize should return an error. Not changing it now in case + // there is a side-effect. + sizeRw = -1 } if _, err = os.Stat(container.basefs); err != nil { diff --git a/daemon/daemon.go b/daemon/daemon.go index 7ee7948a28..db37d24bff 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -937,46 +937,13 @@ func (daemon *Daemon) Unmount(container *Container) error { } func (daemon *Daemon) Changes(container *Container) ([]archive.Change, error) { - if differ, ok := daemon.driver.(graphdriver.Differ); ok { - return differ.Changes(container.ID) - } - cDir, err := daemon.driver.Get(container.ID, "") - if err != nil { - return nil, fmt.Errorf("Error getting container rootfs %s from driver %s: %s", container.ID, container.daemon.driver, err) - } - defer daemon.driver.Put(container.ID) - initDir, err := daemon.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.daemon.driver, err) - } - defer daemon.driver.Put(container.ID + "-init") - return archive.ChangesDirs(cDir, initDir) + initID := fmt.Sprintf("%s-init", container.ID) + return daemon.driver.Changes(container.ID, initID) } func (daemon *Daemon) Diff(container *Container) (archive.Archive, error) { - if differ, ok := daemon.driver.(graphdriver.Differ); ok { - return differ.Diff(container.ID) - } - - changes, err := daemon.Changes(container) - if err != nil { - return nil, err - } - - cDir, err := daemon.driver.Get(container.ID, "") - if err != nil { - return nil, fmt.Errorf("Error getting container rootfs %s from driver %s: %s", container.ID, container.daemon.driver, err) - } - - archive, err := archive.ExportChanges(cDir, changes) - if err != nil { - return nil, err - } - return ioutils.NewReadCloserWrapper(archive, func() error { - err := archive.Close() - daemon.driver.Put(container.ID) - return err - }), nil + initID := fmt.Sprintf("%s-init", container.ID) + return daemon.driver.Diff(container.ID, initID) } func (daemon *Daemon) Run(c *Container, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (int, error) { diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index ebd4929389..8dc0f0de1b 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -294,23 +294,44 @@ func (a *Driver) Put(id string) { } } -// Returns an archive of the contents for the id -func (a *Driver) Diff(id string) (archive.Archive, error) { +// Diff produces an archive of the changes between the specified +// layer and its parent layer which may be "". +func (a *Driver) Diff(id, parent string) (archive.Archive, error) { + // AUFS doesn't need the parent layer to produce a diff. return archive.TarWithOptions(path.Join(a.rootPath(), "diff", id), &archive.TarOptions{ Compression: archive.Uncompressed, }) } -func (a *Driver) ApplyDiff(id string, diff archive.ArchiveReader) error { +func (a *Driver) applyDiff(id string, diff archive.ArchiveReader) error { return archive.Untar(diff, path.Join(a.rootPath(), "diff", id), nil) } -// Returns the size of the contents for the id -func (a *Driver) DiffSize(id string) (int64, error) { +// DiffSize calculates the changes between the specified id +// and its parent and returns the size in bytes of the changes +// relative to its base filesystem directory. +func (a *Driver) DiffSize(id, parent string) (bytes int64, err error) { + // AUFS doesn't need the parent layer to calculate the diff size. return utils.TreeSize(path.Join(a.rootPath(), "diff", id)) } -func (a *Driver) Changes(id string) ([]archive.Change, error) { +// ApplyDiff extracts the changeset from the given diff into the +// layer with the specified id and parent, returning the size of the +// new layer in bytes. +func (a *Driver) ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) { + // AUFS doesn't need the parent id to apply the diff. + if err = a.applyDiff(id, diff); err != nil { + return + } + + return a.DiffSize(id, parent) +} + +// Changes produces a list of changes between the specified layer +// and its parent layer. If parent is "", then all changes will be ADD changes. +func (a *Driver) Changes(id, parent string) ([]archive.Change, error) { + // AUFS doesn't have snapshots, so we need to get changes from all parent + // layers. layers, err := a.getParentLayerPaths(id) if err != nil { return nil, err @@ -323,9 +344,6 @@ func (a *Driver) getParentLayerPaths(id string) ([]string, error) { if err != nil { return nil, err } - if len(parentIds) == 0 { - return nil, fmt.Errorf("Dir %s does not have any parent layers", id) - } layers := make([]string, len(parentIds)) // Get the diff paths for all the parent ids diff --git a/daemon/graphdriver/aufs/aufs_test.go b/daemon/graphdriver/aufs/aufs_test.go index 081fb88984..a5ea8579e5 100644 --- a/daemon/graphdriver/aufs/aufs_test.go +++ b/daemon/graphdriver/aufs/aufs_test.go @@ -330,7 +330,7 @@ func TestGetDiff(t *testing.T) { } f.Close() - a, err := d.Diff("1") + a, err := d.Diff("1", "") if err != nil { t.Fatal(err) } @@ -374,7 +374,7 @@ func TestChanges(t *testing.T) { t.Fatal(err) } - changes, err := d.Changes("2") + changes, err := d.Changes("2", "") if err != nil { t.Fatal(err) } @@ -413,7 +413,7 @@ func TestChanges(t *testing.T) { t.Fatal(err) } - changes, err = d.Changes("3") + changes, err = d.Changes("3", "") if err != nil { t.Fatal(err) } @@ -465,7 +465,7 @@ func TestDiffSize(t *testing.T) { t.Fatal(err) } - diffSize, err := d.DiffSize("1") + diffSize, err := d.DiffSize("1", "") if err != nil { t.Fatal(err) } @@ -507,7 +507,7 @@ func TestChildDiffSize(t *testing.T) { t.Fatal(err) } - diffSize, err := d.DiffSize("1") + diffSize, err := d.DiffSize("1", "") if err != nil { t.Fatal(err) } @@ -519,7 +519,7 @@ func TestChildDiffSize(t *testing.T) { t.Fatal(err) } - diffSize, err = d.DiffSize("2") + diffSize, err = d.DiffSize("2", "") if err != nil { t.Fatal(err) } @@ -602,7 +602,7 @@ func TestApplyDiff(t *testing.T) { } f.Close() - diff, err := d.Diff("1") + diff, err := d.Diff("1", "") if err != nil { t.Fatal(err) } @@ -614,7 +614,7 @@ func TestApplyDiff(t *testing.T) { t.Fatal(err) } - if err := d.ApplyDiff("3", diff); err != nil { + if err := d.applyDiff("3", diff); err != nil { t.Fatal(err) } diff --git a/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go index c491fd7908..1064a26630 100644 --- a/daemon/graphdriver/btrfs/btrfs.go +++ b/daemon/graphdriver/btrfs/btrfs.go @@ -44,9 +44,11 @@ func Init(home string, options []string) (graphdriver.Driver, error) { return nil, err } - return &Driver{ + driver := &Driver{ home: home, - }, nil + } + + return graphdriver.NewGenericDriverWrapper(driver), nil } type Driver struct { diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go index 5940d58798..215af40153 100644 --- a/daemon/graphdriver/devmapper/driver.go +++ b/daemon/graphdriver/devmapper/driver.go @@ -43,7 +43,7 @@ func Init(home string, options []string) (graphdriver.Driver, error) { home: home, } - return d, nil + return graphdriver.NewGenericDriverWrapper(d), nil } func (d *Driver) String() string { diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index 90ed1d8162..c389705df9 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -19,26 +19,55 @@ const ( type InitFunc func(root string, options []string) (Driver, error) -type Driver interface { +// GenericDriver defines the basic capabilities of a driver. +type GenericDriver interface { + // String returns a string representation of this driver. String() string + // Create creates a new, empty, filesystem layer with the + // specified id and parent. Parent may be "". Create(id, parent string) error + // Remove attepmts to remove the filesystem layer with this id. Remove(id string) error + // Get returns the mountpoint for the layered filesystem referred + // to by this id. You can optionally specify a mountLabel or "". + // Returns the absolute path to the mounted layered filesystem. Get(id, mountLabel string) (dir string, err error) + // Put releases the system resources for the specified id, + // e.g, unmounting layered filesystem. Put(id string) + // Exists returns whether a filesystem layer with the specified + // ID exists on this driver. Exists(id string) bool + // Status returns a set of key-value pairs which give low + // level diagnostic status about this driver. Status() [][2]string + // Cleanup performs necessary tasks to release resources + // held by the driver, e.g., unmounting all layered filesystems + // known to this driver. Cleanup() error } -type Differ interface { - Diff(id string) (archive.Archive, error) - Changes(id string) ([]archive.Change, error) - ApplyDiff(id string, diff archive.ArchiveReader) error - DiffSize(id string) (bytes int64, err error) +type Driver interface { + GenericDriver + + // Diff produces an archive of the changes between the specified + // layer and its parent layer which may be "". + Diff(id, parent string) (archive.Archive, error) + // Changes produces a list of changes between the specified layer + // and its parent layer. If parent is "", then all changes will be ADD changes. + Changes(id, parent string) ([]archive.Change, error) + // ApplyDiff extracts the changeset from the given diff into the + // layer with the specified id and parent, returning the size of the + // new layer in bytes. + ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) + // DiffSize calculates the changes between the specified id + // and its parent and returns the size in bytes of the changes + // relative to its base filesystem directory. + DiffSize(id, parent string) (bytes int64, err error) } var ( diff --git a/daemon/graphdriver/fsdiff.go b/daemon/graphdriver/fsdiff.go new file mode 100644 index 0000000000..03d109e930 --- /dev/null +++ b/daemon/graphdriver/fsdiff.go @@ -0,0 +1,162 @@ +package graphdriver + +import ( + "fmt" + "time" + + "github.com/docker/docker/archive" + "github.com/docker/docker/pkg/ioutils" + "github.com/docker/docker/pkg/log" + "github.com/docker/docker/utils" +) + +// GenericDriverWrapper takes a generic Driver and adds the +// capability of the following methods which it doesn't +// support on its own: +// Diff(id, parent string) (archive.Archive, error) +// Changes(id, parent string) ([]archive.Change, error) +// ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) +// DiffSize(id, parent string) (bytes int64, err error) +// Notably, the AUFS driver doesn't need to be wrapped like this. +type GenericDriverWrapper struct { + GenericDriver +} + +// NewGenericDriverWrapper returns a fully functional driver that wraps the given GenericDriver. +func NewGenericDriverWrapper(driver GenericDriver) Driver { + return &GenericDriverWrapper{GenericDriver: driver} +} + +// Diff produces an archive of the changes between the specified +// layer and its parent layer which may be "". +func (gdw *GenericDriverWrapper) Diff(id, parent string) (arch archive.Archive, err error) { + driver := gdw.GenericDriver + + layerFs, err := driver.Get(id, "") + if err != nil { + return nil, err + } + + defer func() { + if err != nil { + driver.Put(id) + } + }() + + if parent == "" { + archive, err := archive.Tar(layerFs, archive.Uncompressed) + if err != nil { + return nil, err + } + return ioutils.NewReadCloserWrapper(archive, func() error { + err := archive.Close() + driver.Put(id) + return err + }), nil + } + + parentFs, err := driver.Get(parent, "") + if err != nil { + return nil, err + } + defer driver.Put(parent) + + changes, err := archive.ChangesDirs(layerFs, parentFs) + if err != nil { + return nil, err + } + + archive, err := archive.ExportChanges(layerFs, changes) + if err != nil { + return nil, err + } + + return ioutils.NewReadCloserWrapper(archive, func() error { + err := archive.Close() + driver.Put(id) + return err + }), nil +} + +// Changes produces a list of changes between the specified layer +// and its parent layer. If parent is "", then all changes will be ADD changes. +func (gdw *GenericDriverWrapper) Changes(id, parent string) ([]archive.Change, error) { + driver := gdw.GenericDriver + + layerFs, err := driver.Get(id, "") + if err != nil { + return nil, err + } + defer driver.Put(id) + + parentFs := "" + + if parent != "" { + parentFs, err = driver.Get(parent, "") + if err != nil { + return nil, err + } + defer driver.Put(parent) + } + + return archive.ChangesDirs(layerFs, parentFs) +} + +// ApplyDiff extracts the changeset from the given diff into the +// layer with the specified id and parent, returning the size of the +// new layer in bytes. +func (gdw *GenericDriverWrapper) ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) { + driver := gdw.GenericDriver + + // Mount the root filesystem so we can apply the diff/layer. + layerFs, err := driver.Get(id, "") + if err != nil { + return + } + defer driver.Put(id) + + start := time.Now().UTC() + log.Debugf("Start untar layer") + if err = archive.ApplyLayer(layerFs, diff); err != nil { + return + } + log.Debugf("Untar time: %vs", time.Now().UTC().Sub(start).Seconds()) + + if parent == "" { + return utils.TreeSize(layerFs) + } + + parentFs, err := driver.Get(parent, "") + if err != nil { + err = fmt.Errorf("Driver %s failed to get image parent %s: %s", driver, parent, err) + return + } + defer driver.Put(parent) + + changes, err := archive.ChangesDirs(layerFs, parentFs) + if err != nil { + return + } + + return archive.ChangesSize(layerFs, changes), nil +} + +// DiffSize calculates the changes between the specified layer +// and its parent and returns the size in bytes of the changes +// relative to its base filesystem directory. +func (gdw *GenericDriverWrapper) DiffSize(id, parent string) (bytes int64, err error) { + driver := gdw.GenericDriver + + changes, err := gdw.Changes(id, parent) + if err != nil { + return + } + + layerFs, err := driver.Get(id, "") + if err != nil { + return + } + defer driver.Put(id) + + return archive.ChangesSize(layerFs, changes), nil +} diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index 4075892c49..0bbf9d421a 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -19,7 +19,7 @@ func Init(home string, options []string) (graphdriver.Driver, error) { d := &Driver{ home: home, } - return d, nil + return graphdriver.NewGenericDriverWrapper(d), nil } type Driver struct { diff --git a/graph/graph.go b/graph/graph.go index e6c9cb43d2..84a3729789 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -100,27 +100,9 @@ func (graph *Graph) Get(name string) (*image.Image, error) { img.SetGraph(graph) if img.Size < 0 { - rootfs, err := graph.driver.Get(img.ID, "") + size, err := graph.driver.DiffSize(img.ID, img.Parent) 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 == "" { - if size, err = utils.TreeSize(rootfs); err != nil { - return nil, err - } - } else { - parentFs, err := graph.driver.Get(img.Parent, "") - if err != nil { - return nil, err - } - changes, err := archive.ChangesDirs(rootfs, parentFs) - if err != nil { - return nil, err - } - size = archive.ChangesSize(rootfs, changes) + return nil, fmt.Errorf("unable to calculate size of image id %q: %s", img.ID, err) } img.Size = size @@ -197,14 +179,9 @@ func (graph *Graph) Register(img *image.Image, jsonData []byte, layerData archiv if err := graph.driver.Create(img.ID, img.Parent); err != nil { return fmt.Errorf("Driver %s failed to create image rootfs %s: %s", graph.driver, img.ID, err) } - // Mount the root filesystem so we can apply the diff/layer - rootfs, err := graph.driver.Get(img.ID, "") - 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) + // Apply the diff/layer img.SetGraph(graph) - if err := image.StoreImage(img, jsonData, layerData, tmp, rootfs); err != nil { + if err := image.StoreImage(img, jsonData, layerData, tmp); err != nil { return err } // Commit diff --git a/image/image.go b/image/image.go index 468d01293d..9e58cfe724 100644 --- a/image/image.go +++ b/image/image.go @@ -10,8 +10,6 @@ import ( "time" "github.com/docker/docker/archive" - "github.com/docker/docker/daemon/graphdriver" - "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/log" "github.com/docker/docker/runconfig" "github.com/docker/docker/utils" @@ -72,51 +70,18 @@ func LoadImage(root string) (*Image, error) { return img, nil } -func StoreImage(img *Image, jsonData []byte, layerData archive.ArchiveReader, root, layer string) error { +func StoreImage(img *Image, jsonData []byte, layerData archive.ArchiveReader, root string) error { // Store the layer var ( size int64 err error driver = img.graph.Driver() ) - if err := os.MkdirAll(layer, 0755); err != nil { - return err - } // If layerData is not nil, unpack it into the new layer if layerData != nil { - if differ, ok := driver.(graphdriver.Differ); ok { - if err := differ.ApplyDiff(img.ID, layerData); err != nil { - return err - } - - if size, err = differ.DiffSize(img.ID); err != nil { - return err - } - } else { - start := time.Now().UTC() - log.Debugf("Start untar layer") - if err := archive.ApplyLayer(layer, layerData); err != nil { - return err - } - log.Debugf("Untar time: %vs", time.Now().UTC().Sub(start).Seconds()) - - if img.Parent == "" { - if size, err = utils.TreeSize(layer); err != nil { - return err - } - } else { - parent, err := driver.Get(img.Parent, "") - if err != nil { - return err - } - defer driver.Put(img.Parent) - changes, err := archive.ChangesDirs(layer, parent) - if err != nil { - return err - } - size = archive.ChangesSize(layer, changes) - } + if size, err = driver.ApplyDiff(img.ID, img.Parent, layerData); err != nil { + return err } } @@ -178,52 +143,10 @@ 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) } + driver := img.graph.Driver() - if differ, ok := driver.(graphdriver.Differ); ok { - return differ.Diff(img.ID) - } - imgFs, err := driver.Get(img.ID, "") - if err != nil { - return nil, err - } - - defer func() { - if err != nil { - driver.Put(img.ID) - } - }() - - if img.Parent == "" { - archive, err := archive.Tar(imgFs, archive.Uncompressed) - if err != nil { - return nil, err - } - return ioutils.NewReadCloserWrapper(archive, func() error { - err := archive.Close() - driver.Put(img.ID) - return err - }), 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 ioutils.NewReadCloserWrapper(archive, func() error { - err := archive.Close() - driver.Put(img.ID) - return err - }), nil + return driver.Diff(img.ID, img.Parent) } // Image includes convenience proxy functions to its graph From 09ad65ebd5b50fdd7621f42136278102586a7ea8 Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Tue, 16 Sep 2014 12:13:50 -0700 Subject: [PATCH 2/2] graphdriver interface name change, typo fix Signed-off-by: Josh Hawn --- daemon/graphdriver/btrfs/btrfs.go | 2 +- daemon/graphdriver/devmapper/driver.go | 2 +- daemon/graphdriver/driver.go | 19 ++++++------ daemon/graphdriver/fsdiff.go | 41 ++++++++++++++------------ daemon/graphdriver/vfs/driver.go | 2 +- 5 files changed, 35 insertions(+), 31 deletions(-) diff --git a/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go index 1064a26630..26102aa1ef 100644 --- a/daemon/graphdriver/btrfs/btrfs.go +++ b/daemon/graphdriver/btrfs/btrfs.go @@ -48,7 +48,7 @@ func Init(home string, options []string) (graphdriver.Driver, error) { home: home, } - return graphdriver.NewGenericDriverWrapper(driver), nil + return graphdriver.NaiveDiffDriver(driver), nil } type Driver struct { diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go index 215af40153..4ea1489f34 100644 --- a/daemon/graphdriver/devmapper/driver.go +++ b/daemon/graphdriver/devmapper/driver.go @@ -43,7 +43,7 @@ func Init(home string, options []string) (graphdriver.Driver, error) { home: home, } - return graphdriver.NewGenericDriverWrapper(d), nil + return graphdriver.NaiveDiffDriver(d), nil } func (d *Driver) String() string { diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index c389705df9..677a1d7e18 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -19,17 +19,20 @@ const ( type InitFunc func(root string, options []string) (Driver, error) -// GenericDriver defines the basic capabilities of a driver. -type GenericDriver interface { +// ProtoDriver defines the basic capabilities of a driver. +// This interface exists solely to be a minimum set of methods +// for client code which choose not to implement the entire Driver +// interface and use the NaiveDiffDriver wrapper constructor. +// +// Use of ProtoDriver directly by client code is not recommended. +type ProtoDriver interface { // String returns a string representation of this driver. String() string - // Create creates a new, empty, filesystem layer with the // specified id and parent. Parent may be "". Create(id, parent string) error - // Remove attepmts to remove the filesystem layer with this id. + // Remove attempts to remove the filesystem layer with this id. Remove(id string) error - // Get returns the mountpoint for the layered filesystem referred // to by this id. You can optionally specify a mountLabel or "". // Returns the absolute path to the mounted layered filesystem. @@ -40,20 +43,18 @@ type GenericDriver interface { // Exists returns whether a filesystem layer with the specified // ID exists on this driver. Exists(id string) bool - // Status returns a set of key-value pairs which give low // level diagnostic status about this driver. Status() [][2]string - // Cleanup performs necessary tasks to release resources // held by the driver, e.g., unmounting all layered filesystems // known to this driver. Cleanup() error } +// Driver is the interface for layered/snapshot file system drivers. type Driver interface { - GenericDriver - + ProtoDriver // Diff produces an archive of the changes between the specified // layer and its parent layer which may be "". Diff(id, parent string) (archive.Archive, error) diff --git a/daemon/graphdriver/fsdiff.go b/daemon/graphdriver/fsdiff.go index 03d109e930..d9d4434aec 100644 --- a/daemon/graphdriver/fsdiff.go +++ b/daemon/graphdriver/fsdiff.go @@ -10,27 +10,30 @@ import ( "github.com/docker/docker/utils" ) -// GenericDriverWrapper takes a generic Driver and adds the -// capability of the following methods which it doesn't -// support on its own: +// naiveDiffDriver takes a ProtoDriver and adds the +// capability of the Diffing methods which it may or may not +// support on its own. See the comment on the exported +// NaiveDiffDriver function below. +// Notably, the AUFS driver doesn't need to be wrapped like this. +type naiveDiffDriver struct { + ProtoDriver +} + +// NaiveDiffDriver returns a fully functional driver that wraps the +// given ProtoDriver and adds the capability of the following methods which +// it may or may not support on its own: // Diff(id, parent string) (archive.Archive, error) // Changes(id, parent string) ([]archive.Change, error) // ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) // DiffSize(id, parent string) (bytes int64, err error) -// Notably, the AUFS driver doesn't need to be wrapped like this. -type GenericDriverWrapper struct { - GenericDriver -} - -// NewGenericDriverWrapper returns a fully functional driver that wraps the given GenericDriver. -func NewGenericDriverWrapper(driver GenericDriver) Driver { - return &GenericDriverWrapper{GenericDriver: driver} +func NaiveDiffDriver(driver ProtoDriver) Driver { + return &naiveDiffDriver{ProtoDriver: driver} } // Diff produces an archive of the changes between the specified // layer and its parent layer which may be "". -func (gdw *GenericDriverWrapper) Diff(id, parent string) (arch archive.Archive, err error) { - driver := gdw.GenericDriver +func (gdw *naiveDiffDriver) Diff(id, parent string) (arch archive.Archive, err error) { + driver := gdw.ProtoDriver layerFs, err := driver.Get(id, "") if err != nil { @@ -80,8 +83,8 @@ func (gdw *GenericDriverWrapper) Diff(id, parent string) (arch archive.Archive, // Changes produces a list of changes between the specified layer // and its parent layer. If parent is "", then all changes will be ADD changes. -func (gdw *GenericDriverWrapper) Changes(id, parent string) ([]archive.Change, error) { - driver := gdw.GenericDriver +func (gdw *naiveDiffDriver) Changes(id, parent string) ([]archive.Change, error) { + driver := gdw.ProtoDriver layerFs, err := driver.Get(id, "") if err != nil { @@ -105,8 +108,8 @@ func (gdw *GenericDriverWrapper) Changes(id, parent string) ([]archive.Change, e // ApplyDiff extracts the changeset from the given diff into the // layer with the specified id and parent, returning the size of the // new layer in bytes. -func (gdw *GenericDriverWrapper) ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) { - driver := gdw.GenericDriver +func (gdw *naiveDiffDriver) ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) { + driver := gdw.ProtoDriver // Mount the root filesystem so we can apply the diff/layer. layerFs, err := driver.Get(id, "") @@ -144,8 +147,8 @@ func (gdw *GenericDriverWrapper) ApplyDiff(id, parent string, diff archive.Archi // DiffSize calculates the changes between the specified layer // and its parent and returns the size in bytes of the changes // relative to its base filesystem directory. -func (gdw *GenericDriverWrapper) DiffSize(id, parent string) (bytes int64, err error) { - driver := gdw.GenericDriver +func (gdw *naiveDiffDriver) DiffSize(id, parent string) (bytes int64, err error) { + driver := gdw.ProtoDriver changes, err := gdw.Changes(id, parent) if err != nil { diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index 0bbf9d421a..a186060d03 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -19,7 +19,7 @@ func Init(home string, options []string) (graphdriver.Driver, error) { d := &Driver{ home: home, } - return graphdriver.NewGenericDriverWrapper(d), nil + return graphdriver.NaiveDiffDriver(d), nil } type Driver struct {