From ef5bfad3210a9e9c8b761f2c11c0c6289490ebff Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Thu, 18 Feb 2016 17:24:59 -0800 Subject: [PATCH] Adding readOnly parameter to graphdriver Create method Since the layer store was introduced, the level above the graphdriver now differentiates between read/write and read-only layers. This distinction is useful for graphdrivers that need to take special steps when creating a layer based on whether it is read-only or not. Adding this parameter allows the graphdrivers to differentiate, which in the case of the Windows graphdriver, removes our dependence on parsing the id of the parent for "-init" in order to infer this information. This will also set the stage for unblocking some of the layer store unit tests in the next preview build of Windows. Signed-off-by: Stefan J. Wernli --- daemon/graphdriver/aufs/aufs.go | 6 ++++ daemon/graphdriver/aufs/aufs_test.go | 14 ++++---- daemon/graphdriver/btrfs/btrfs.go | 6 ++++ daemon/graphdriver/btrfs/btrfs_test.go | 2 +- daemon/graphdriver/devmapper/driver.go | 6 ++++ daemon/graphdriver/driver.go | 3 ++ .../graphdriver/graphtest/graphtest_unix.go | 2 +- daemon/graphdriver/overlay/overlay.go | 6 ++++ daemon/graphdriver/proxy.go | 16 +++++++++ daemon/graphdriver/vfs/driver.go | 6 ++++ daemon/graphdriver/windows/windows.go | 35 +++++++++++++------ daemon/graphdriver/zfs/zfs.go | 6 ++++ ...cker_cli_external_graphdriver_unix_test.go | 15 ++++++++ layer/layer_store.go | 2 +- 14 files changed, 104 insertions(+), 21 deletions(-) diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index 8dc4c4aaf8..dfc5938bd1 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -194,6 +194,12 @@ func (a *Driver) Exists(id string) bool { return true } +// CreateReadWrite creates a layer that is writable for use as a container +// file system. +func (a *Driver) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + return a.Create(id, parent, mountLabel, storageOpt) +} + // Create three folders for each id // mnt, layers, and diff func (a *Driver) Create(id, parent, mountLabel string, storageOpt map[string]string) error { diff --git a/daemon/graphdriver/aufs/aufs_test.go b/daemon/graphdriver/aufs/aufs_test.go index 0ee9aa1201..1d0a268f94 100644 --- a/daemon/graphdriver/aufs/aufs_test.go +++ b/daemon/graphdriver/aufs/aufs_test.go @@ -320,7 +320,7 @@ func TestGetDiff(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", "", "", nil); err != nil { + if err := d.CreateReadWrite("1", "", "", nil); err != nil { t.Fatal(err) } @@ -357,7 +357,7 @@ func TestChanges(t *testing.T) { if err := d.Create("1", "", "", nil); err != nil { t.Fatal(err) } - if err := d.Create("2", "1", "", nil); err != nil { + if err := d.CreateReadWrite("2", "1", "", nil); err != nil { t.Fatal(err) } @@ -403,7 +403,7 @@ func TestChanges(t *testing.T) { t.Fatalf("Change kind should be ChangeAdd got %s", change.Kind) } - if err := d.Create("3", "2", "", nil); err != nil { + if err := d.CreateReadWrite("3", "2", "", nil); err != nil { t.Fatal(err) } mntPoint, err = d.Get("3", "") @@ -448,7 +448,7 @@ func TestDiffSize(t *testing.T) { d := newDriver(t) defer os.RemoveAll(tmp) - if err := d.Create("1", "", "", nil); err != nil { + if err := d.CreateReadWrite("1", "", "", nil); err != nil { t.Fatal(err) } @@ -490,7 +490,7 @@ func TestChildDiffSize(t *testing.T) { defer os.RemoveAll(tmp) defer d.Cleanup() - if err := d.Create("1", "", "", nil); err != nil { + if err := d.CreateReadWrite("1", "", "", nil); err != nil { t.Fatal(err) } @@ -592,7 +592,7 @@ func TestApplyDiff(t *testing.T) { defer os.RemoveAll(tmp) defer d.Cleanup() - if err := d.Create("1", "", "", nil); err != nil { + if err := d.CreateReadWrite("1", "", "", nil); err != nil { t.Fatal(err) } @@ -671,7 +671,7 @@ func testMountMoreThan42Layers(t *testing.T, mountPath string) { } current = hash(current) - if err := d.Create(current, parent, "", nil); err != nil { + if err := d.CreateReadWrite(current, parent, "", nil); err != nil { t.Logf("Current layer %d", i) t.Error(err) } diff --git a/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go index 9d69474b1c..6e6581baa8 100644 --- a/daemon/graphdriver/btrfs/btrfs.go +++ b/daemon/graphdriver/btrfs/btrfs.go @@ -241,6 +241,12 @@ func (d *Driver) subvolumesDirID(id string) string { return path.Join(d.subvolumesDir(), id) } +// CreateReadWrite creates a layer that is writable for use as a container +// file system. +func (d *Driver) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + return d.Create(id, parent, mountLabel, storageOpt) +} + // Create the filesystem with given id. func (d *Driver) Create(id, parent, mountLabel string, storageOpt map[string]string) error { diff --git a/daemon/graphdriver/btrfs/btrfs_test.go b/daemon/graphdriver/btrfs/btrfs_test.go index 7d708ca41c..54442c0a97 100644 --- a/daemon/graphdriver/btrfs/btrfs_test.go +++ b/daemon/graphdriver/btrfs/btrfs_test.go @@ -30,7 +30,7 @@ func TestBtrfsCreateSnap(t *testing.T) { func TestBtrfsSubvolDelete(t *testing.T) { d := graphtest.GetDriver(t, "btrfs") - if err := d.Create("test", "", "", nil); err != nil { + if err := d.CreateReadWrite("test", "", "", nil); err != nil { t.Fatal(err) } defer graphtest.PutDriver(t) diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go index 97366b4a71..100c4ba8a8 100644 --- a/daemon/graphdriver/devmapper/driver.go +++ b/daemon/graphdriver/devmapper/driver.go @@ -117,6 +117,12 @@ func (d *Driver) Cleanup() error { return err } +// CreateReadWrite creates a layer that is writable for use as a container +// file system. +func (d *Driver) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + return d.Create(id, parent, mountLabel, storageOpt) +} + // Create adds a device with a given id and the parent. func (d *Driver) Create(id, parent, mountLabel string, storageOpt map[string]string) error { if err := d.DeviceSet.AddDevice(id, parent, storageOpt); err != nil { diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index a3b912f29a..495bac2cf5 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -46,6 +46,9 @@ type InitFunc func(root string, options []string, uidMaps, gidMaps []idtools.IDM type ProtoDriver interface { // String returns a string representation of this driver. String() string + // CreateReadWrite creates a new, empty filesystem layer that is ready + // to be used as the storage for a container. + CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error // Create creates a new, empty, filesystem layer with the // specified id and parent and mountLabel. Parent and mountLabel may be "". Create(id, parent, mountLabel string, storageOpt map[string]string) error diff --git a/daemon/graphdriver/graphtest/graphtest_unix.go b/daemon/graphdriver/graphtest/graphtest_unix.go index 6ea3ed0422..78f137dff4 100644 --- a/daemon/graphdriver/graphtest/graphtest_unix.go +++ b/daemon/graphdriver/graphtest/graphtest_unix.go @@ -215,7 +215,7 @@ func createBase(t *testing.T, driver graphdriver.Driver, name string) { oldmask := syscall.Umask(0) defer syscall.Umask(oldmask) - if err := driver.Create(name, "", "", nil); err != nil { + if err := driver.CreateReadWrite(name, "", "", nil); err != nil { t.Fatal(err) } diff --git a/daemon/graphdriver/overlay/overlay.go b/daemon/graphdriver/overlay/overlay.go index a1cc1451b5..34abb03b2f 100644 --- a/daemon/graphdriver/overlay/overlay.go +++ b/daemon/graphdriver/overlay/overlay.go @@ -220,6 +220,12 @@ func (d *Driver) Cleanup() error { return nil } +// CreateReadWrite creates a layer that is writable for use as a container +// file system. +func (d *Driver) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + return d.Create(id, parent, mountLabel, storageOpt) +} + // Create is used to create the upper, lower, and merge directories required for overlay fs for a given id. // The parent filesystem is used to configure these directories for the overlay. func (d *Driver) Create(id, parent, mountLabel string, storageOpt map[string]string) (retErr error) { diff --git a/daemon/graphdriver/proxy.go b/daemon/graphdriver/proxy.go index df31c01dcb..3a8d599cb6 100644 --- a/daemon/graphdriver/proxy.go +++ b/daemon/graphdriver/proxy.go @@ -54,6 +54,22 @@ func (d *graphDriverProxy) String() string { return d.name } +func (d *graphDriverProxy) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + args := &graphDriverRequest{ + ID: id, + Parent: parent, + MountLabel: mountLabel, + } + var ret graphDriverResponse + if err := d.client.Call("GraphDriver.CreateReadWrite", args, &ret); err != nil { + return err + } + if ret.Err != "" { + return errors.New(ret.Err) + } + return nil +} + func (d *graphDriverProxy) Create(id, parent, mountLabel string, storageOpt map[string]string) error { args := &graphDriverRequest{ ID: id, diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index 8094b3e900..a058f08bc8 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -68,6 +68,12 @@ func (d *Driver) Cleanup() error { return nil } +// CreateReadWrite creates a layer that is writable for use as a container +// file system. +func (d *Driver) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + return d.Create(id, parent, mountLabel, storageOpt) +} + // Create prepares the filesystem for the VFS driver and copies the directory for the given id under the parent. func (d *Driver) Create(id, parent, mountLabel string, storageOpt map[string]string) error { if len(storageOpt) != 0 { diff --git a/daemon/graphdriver/windows/windows.go b/daemon/graphdriver/windows/windows.go index 239df05c46..fd06c9b349 100644 --- a/daemon/graphdriver/windows/windows.go +++ b/daemon/graphdriver/windows/windows.go @@ -106,8 +106,18 @@ func (d *Driver) Exists(id string) bool { return result } -// Create creates a new layer with the given id. +// CreateReadWrite creates a layer that is writable for use as a container +// file system. +func (d *Driver) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + return d.create(id, parent, mountLabel, false, storageOpt) +} + +// Create creates a new read-only layer with the given id. func (d *Driver) Create(id, parent, mountLabel string, storageOpt map[string]string) error { + return d.create(id, parent, mountLabel, true, storageOpt) +} + +func (d *Driver) create(id, parent, mountLabel string, readOnly bool, storageOpt map[string]string) error { if len(storageOpt) != 0 { return fmt.Errorf("--storage-opt is not supported for windows") } @@ -124,27 +134,30 @@ func (d *Driver) Create(id, parent, mountLabel string, storageOpt map[string]str var layerChain []string - parentIsInit := strings.HasSuffix(rPId, "-init") - - if !parentIsInit && rPId != "" { + if rPId != "" { parentPath, err := hcsshim.GetLayerMountPath(d.info, rPId) if err != nil { return err } - layerChain = []string{parentPath} + if _, err := os.Stat(filepath.Join(parentPath, "Files")); err == nil { + // This is a legitimate parent layer (not the empty "-init" layer), + // so include it in the layer chain. + layerChain = []string{parentPath} + } } layerChain = append(layerChain, parentChain...) - if parentIsInit { - if len(layerChain) == 0 { - return fmt.Errorf("Cannot create a read/write layer without a parent layer.") - } - if err := hcsshim.CreateSandboxLayer(d.info, id, layerChain[0], layerChain); err != nil { + if readOnly { + if err := hcsshim.CreateLayer(d.info, id, rPId); err != nil { return err } } else { - if err := hcsshim.CreateLayer(d.info, id, rPId); err != nil { + var parentPath string + if len(layerChain) != 0 { + parentPath = layerChain[0] + } + if err := hcsshim.CreateSandboxLayer(d.info, id, parentPath, layerChain); err != nil { return err } } diff --git a/daemon/graphdriver/zfs/zfs.go b/daemon/graphdriver/zfs/zfs.go index 90347dd02f..b979b2af90 100644 --- a/daemon/graphdriver/zfs/zfs.go +++ b/daemon/graphdriver/zfs/zfs.go @@ -240,6 +240,12 @@ func (d *Driver) mountPath(id string) string { return path.Join(d.options.mountPath, "graph", getMountpoint(id)) } +// CreateReadWrite creates a layer that is writable for use as a container +// file system. +func (d *Driver) CreateReadWrite(id, parent, mountLabel string, storageOpt map[string]string) error { + return d.Create(id, parent, mountLabel, storageOpt) +} + // Create prepares the dataset and filesystem for the ZFS driver for the given id under the parent. func (d *Driver) Create(id string, parent string, mountLabel string, storageOpt map[string]string) error { if len(storageOpt) != 0 { diff --git a/integration-cli/docker_cli_external_graphdriver_unix_test.go b/integration-cli/docker_cli_external_graphdriver_unix_test.go index 257c60f029..b7237f8944 100644 --- a/integration-cli/docker_cli_external_graphdriver_unix_test.go +++ b/integration-cli/docker_cli_external_graphdriver_unix_test.go @@ -67,6 +67,7 @@ func (s *DockerExternalGraphdriverSuite) SetUpSuite(c *check.C) { ID string `json:",omitempty"` Parent string `json:",omitempty"` MountLabel string `json:",omitempty"` + ReadOnly bool `json:",omitempty"` } type graphDriverResponse struct { @@ -115,6 +116,20 @@ func (s *DockerExternalGraphdriverSuite) SetUpSuite(c *check.C) { respond(w, "{}") }) + mux.HandleFunc("/GraphDriver.CreateReadWrite", func(w http.ResponseWriter, r *http.Request) { + s.ec.creations++ + + var req graphDriverRequest + if err := decReq(r.Body, &req, w); err != nil { + return + } + if err := driver.CreateReadWrite(req.ID, req.Parent, "", nil); err != nil { + respond(w, err) + return + } + respond(w, "{}") + }) + mux.HandleFunc("/GraphDriver.Create", func(w http.ResponseWriter, r *http.Request) { s.ec.creations++ diff --git a/layer/layer_store.go b/layer/layer_store.go index 1f56abb126..f18aff2145 100644 --- a/layer/layer_store.go +++ b/layer/layer_store.go @@ -461,7 +461,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, mountLabel stri m.initID = pid } - if err = ls.driver.Create(m.mountID, pid, "", storageOpt); err != nil { + if err = ls.driver.CreateReadWrite(m.mountID, pid, "", storageOpt); err != nil { return nil, err }