From 38955d22f425a0747f3b324a6b13b4fbe951c4fc Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 28 Nov 2019 14:07:56 +0100 Subject: [PATCH] Split RW and RO Layer locks per method Accessing layers in RO or RW mode is now decided in the dedicated method which increases the flexibility in terms of locking. This allows use cases like listing container images while pushing an image to a remote location in parallel. Signed-off-by: Sascha Grunert Signed-off-by: Sascha Grunert --- cmd/containers-storage/layers.go | 2 +- layers.go | 38 ++--- store.go | 247 +++++++++++++++++++++---------- userns.go | 4 +- 4 files changed, 187 insertions(+), 104 deletions(-) diff --git a/cmd/containers-storage/layers.go b/cmd/containers-storage/layers.go index 91503d3bf1..764e834c15 100644 --- a/cmd/containers-storage/layers.go +++ b/cmd/containers-storage/layers.go @@ -12,7 +12,7 @@ import ( var listLayersTree = false func layers(flags *mflag.FlagSet, action string, m storage.Store, args []string) int { - layers, err := m.Layers() + layers, err := m.RWLayers() if err != nil { fmt.Fprintf(os.Stderr, "%+v\n", err) return 1 diff --git a/layers.go b/layers.go index 72f00b8d65..ee15608bbc 100644 --- a/layers.go +++ b/layers.go @@ -491,7 +491,10 @@ func (r *layerStore) saveMounts() error { return r.loadMounts() } -func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Driver) (LayerStore, error) { +// newLayerStore creates a new layer storage based on the provided directories +// and selected driver. The `withMountsLock` bool indicates if we want to +// enable mount locking, which is necessary to have write access to the storage. +func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Driver, withMountsLock bool) (LayerStore, error) { if err := os.MkdirAll(rundir, 0700); err != nil { return nil, err } @@ -502,9 +505,12 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri if err != nil { return nil, err } - mountsLockfile, err := GetLockfile(filepath.Join(rundir, "mountpoints.lock")) - if err != nil { - return nil, err + var mountsLockfile Locker + if withMountsLock { + mountsLockfile, err = GetLockfile(filepath.Join(rundir, "mountpoints.lock")) + if err != nil { + return nil, err + } } rlstore := layerStore{ lockfile: lockfile, @@ -524,27 +530,6 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri return &rlstore, nil } -func newROLayerStore(rundir string, layerdir string, driver drivers.Driver) (ROLayerStore, error) { - lockfile, err := GetROLockfile(filepath.Join(layerdir, "layers.lock")) - if err != nil { - return nil, err - } - rlstore := layerStore{ - lockfile: lockfile, - mountsLockfile: nil, - driver: driver, - rundir: rundir, - layerdir: layerdir, - byid: make(map[string]*Layer), - bymount: make(map[string]*Layer), - byname: make(map[string]*Layer), - } - if err := rlstore.Load(); err != nil { - return nil, err - } - return &rlstore, nil -} - func (r *layerStore) lookup(id string) (*Layer, bool) { if layer, ok := r.byid[id]; ok { return layer, ok @@ -1470,7 +1455,8 @@ func (r *layerStore) Modified() (bool, error) { } func (r *layerStore) IsReadWrite() bool { - return r.lockfile.IsReadWrite() + // the mountsLockfile is only set for read-write enabled stores. + return r.lockfile.IsReadWrite() && r.mountsLockfile != nil } func (r *layerStore) TouchedSince(when time.Time) bool { diff --git a/store.go b/store.go index fa595355d9..9e3aa9e045 100644 --- a/store.go +++ b/store.go @@ -347,6 +347,9 @@ type Store interface { // Layers returns a list of the currently known layers. Layers() ([]Layer, error) + // RWLayers returns the layers while cleaning up duplicates before. + RWLayers() ([]Layer, error) + // Images returns a list of the currently known images. Images() ([]Image, error) @@ -415,6 +418,9 @@ type Store interface { // Layer returns a specific layer. Layer(id string) (*Layer, error) + // LayerMountpoint returns a specific mountpoint for a layer. + LayerMountpoint(id string) (string, error) + // Image returns a specific image. Image(id string) (*Image, error) @@ -482,6 +488,11 @@ type Store interface { // of still-mounted layers is returned along with possible errors. Shutdown(force bool) (layers []string, err error) + // ShutdownRO shuts down the storage without doing any cleanup on the + // mounts or layers. This assumes that the store has not being modified and + // just used in read-only mode. + ShutdownRO() (err error) + // Version returns version information, in the form of key-value pairs, from // the storage package. Version() ([][2]string, error) @@ -582,6 +593,7 @@ type store struct { autoNsMaxSize uint32 graphDriver drivers.Driver layerStore LayerStore + roLayerStore ROLayerStore roLayerStores []ROLayerStore imageStore ImageStore roImageStores []ROImageStore @@ -827,36 +839,90 @@ func (s *store) GraphDriver() (drivers.Driver, error) { // LayerStore obtains and returns a handle to the writeable layer store object // used by the Store. Accessing this store directly will bypass locking and // synchronization, so it is not a part of the exported Store interface. +// Deprecated: Use RWLayerStore() instead func (s *store) LayerStore() (LayerStore, error) { - s.graphLock.Lock() - defer s.graphLock.Unlock() - if s.graphLock.TouchedSince(s.lastLoaded) { - s.graphDriver = nil - s.layerStore = nil - s.lastLoaded = time.Now() - } + return s.RWLayerStore() +} + +// RWLayerStore obtains and returns a handle to the writeable layer store object +// used by the Store. Accessing this store directly will bypass locking and +// synchronization, so it is not a part of the exported Store interface. +func (s *store) RWLayerStore() (LayerStore, error) { + s.resetIfNeeded() + + // Memoized read-write layer store if s.layerStore != nil { return s.layerStore, nil } - driver, err := s.getGraphDriver() + + rlpath, glpath, driver, err := s.makeLayerPaths() if err != nil { return nil, err } - driverPrefix := s.graphDriverName + "-" - rlpath := filepath.Join(s.runRoot, driverPrefix+"layers") - if err := os.MkdirAll(rlpath, 0700); err != nil { + + rls, err := s.newLayerStore(rlpath, glpath, driver, true) + if err != nil { return nil, err } - glpath := filepath.Join(s.graphRoot, driverPrefix+"layers") - if err := os.MkdirAll(glpath, 0700); err != nil { + + s.layerStore = rls + return s.layerStore, nil +} + +// ROLayerStore obtains and returns a handle to the read-only layer store object +// used by the Store. +func (s *store) ROLayerStore() (ROLayerStore, error) { + s.resetIfNeeded() + + // Memoized read-only layer store + if s.roLayerStore != nil { + return s.roLayerStore, nil + } + + rlpath, glpath, driver, err := s.makeLayerPaths() + if err != nil { return nil, err } - rls, err := s.newLayerStore(rlpath, glpath, driver) + + rls, err := s.newLayerStore(rlpath, glpath, driver, false) if err != nil { return nil, err } - s.layerStore = rls - return s.layerStore, nil + + s.roLayerStore = rls + return s.roLayerStore, nil +} + +// makeLayerPaths retrieves the run directory (rlpath) the layer directory +// (glpath) and the currently configured storage driver for the store. +func (s *store) makeLayerPaths() (rlpath, glpath string, driver drivers.Driver, err error) { + driver, err = s.getGraphDriver() + if err != nil { + return "", "", nil, err + } + layers := s.graphDriverName + "-layers" + rlpath = filepath.Join(s.runRoot, layers) + if err := os.MkdirAll(rlpath, 0700); err != nil { + return "", "", nil, err + } + glpath = filepath.Join(s.graphRoot, layers) + if err := os.MkdirAll(glpath, 0700); err != nil { + return "", "", nil, err + } + return rlpath, glpath, driver, nil +} + +// resetIfNeeded sets the stores memoized graph driver and layer stores back to +// nil if the graph lock has not been touched since it has been loaded. +func (s *store) resetIfNeeded() { + s.graphLock.Lock() + defer s.graphLock.Unlock() + if s.graphLock.TouchedSince(s.lastLoaded) { + s.graphDriver = nil + s.layerStore = nil + s.roLayerStore = nil + s.lastLoaded = time.Now() + } } // ROLayerStores obtains additional read/only layer store objects used by the @@ -872,14 +938,14 @@ func (s *store) ROLayerStores() ([]ROLayerStore, error) { if err != nil { return nil, err } - driverPrefix := s.graphDriverName + "-" - rlpath := filepath.Join(s.runRoot, driverPrefix+"layers") + layers := s.graphDriverName + "-layers" + rlpath := filepath.Join(s.runRoot, layers) if err := os.MkdirAll(rlpath, 0700); err != nil { return nil, err } for _, store := range driver.AdditionalImageStores() { - glpath := filepath.Join(store, driverPrefix+"layers") - rls, err := newROLayerStore(rlpath, glpath, driver) + glpath := filepath.Join(store, layers) + rls, err := s.newLayerStore(rlpath, glpath, driver, false) if err != nil { return nil, err } @@ -909,9 +975,9 @@ func (s *store) ROImageStores() ([]ROImageStore, error) { if err != nil { return nil, err } - driverPrefix := s.graphDriverName + "-" + images := s.graphDriverName + "-images" for _, store := range driver.AdditionalImageStores() { - gipath := filepath.Join(store, driverPrefix+"images") + gipath := filepath.Join(store, images) ris, err := newROImageStore(gipath) if err != nil { return nil, err @@ -933,7 +999,7 @@ func (s *store) ContainerStore() (ContainerStore, error) { func (s *store) PutLayer(id, parent string, names []string, mountLabel string, writeable bool, options *LayerOptions, diff io.Reader) (*Layer, int64, error) { var parentLayer *Layer - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return nil, -1, err } @@ -1046,7 +1112,7 @@ func (s *store) CreateImage(id string, names []string, layer, metadata string, o } if layer != "" { - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return nil, err } @@ -1223,7 +1289,7 @@ func (s *store) CreateContainer(id string, names []string, image, layer, metadat if options.HostGIDMapping { options.GIDMap = nil } - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return nil, err } @@ -1406,7 +1472,7 @@ func (s *store) CreateContainer(id string, names []string, image, layer, metadat } func (s *store) SetMetadata(id, metadata string) error { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return err } @@ -1454,7 +1520,7 @@ func (s *store) SetMetadata(id, metadata string) error { } func (s *store) Metadata(id string) (string, error) { - lstore, err := s.LayerStore() + lstore, err := s.ROLayerStore() if err != nil { return "", err } @@ -1644,10 +1710,10 @@ func (s *store) SetImageBigData(id, key string, data []byte, digestManifest func return ristore.SetBigData(id, key, data, digestManifest) } -func (s *store) ImageSize(id string) (int64, error) { +func (s *store) ImageSize(id string) (size int64, err error) { var image *Image - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return -1, errors.Wrapf(err, "error loading primary layer store data") } @@ -1704,7 +1770,6 @@ func (s *store) ImageSize(id string) (int64, error) { } visited := make(map[string]struct{}) // Walk all of the layers. - var size int64 for len(visited) < len(queue) { for layerID := range queue { // Visit each layer only once. @@ -1759,22 +1824,15 @@ func (s *store) ImageSize(id string) (int64, error) { } func (s *store) ContainerSize(id string) (int64, error) { - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return -1, err } - lstores, err := s.ROLayerStores() - if err != nil { - return -1, err - } - for _, s := range append([]ROLayerStore{lstore}, lstores...) { - store := s - store.RLock() - defer store.Unlock() - if modified, err := store.Modified(); modified || err != nil { - if err = store.Load(); err != nil { - return -1, err - } + lstore.RLock() + defer lstore.Unlock() + if modified, err := lstore.Modified(); err != nil || modified { + if err = lstore.Load(); err != nil { + return -1, err } } @@ -1810,13 +1868,10 @@ func (s *store) ContainerSize(id string) (int64, error) { // Read the container's layer's size. var layer *Layer var size int64 - for _, store := range append([]ROLayerStore{lstore}, lstores...) { - if layer, err = store.Get(container.LayerID); err == nil { - size, err = store.DiffSize("", layer.ID) - if err != nil { - return -1, errors.Wrapf(err, "error determining size of layer with ID %q", layer.ID) - } - break + if layer, err = lstore.Get(container.LayerID); err == nil { + size, err = lstore.DiffSize("", layer.ID) + if err != nil { + return -1, errors.Wrapf(err, "error determining size of layer with ID %q", layer.ID) } } if layer == nil { @@ -1929,7 +1984,7 @@ func (s *store) SetContainerBigData(id, key string, data []byte) error { } func (s *store) Exists(id string) bool { - lstore, err := s.LayerStore() + lstore, err := s.ROLayerStore() if err != nil { return false } @@ -2006,7 +2061,7 @@ func dedupeNames(names []string) []string { func (s *store) SetNames(id string, names []string) error { deduped := dedupeNames(names) - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return err } @@ -2054,7 +2109,7 @@ func (s *store) SetNames(id string, names []string) error { } func (s *store) Names(id string) ([]string, error) { - lstore, err := s.LayerStore() + lstore, err := s.ROLayerStore() if err != nil { return nil, err } @@ -2116,7 +2171,7 @@ func (s *store) Names(id string) ([]string, error) { } func (s *store) Lookup(name string) (string, error) { - lstore, err := s.LayerStore() + lstore, err := s.ROLayerStore() if err != nil { return "", err } @@ -2179,7 +2234,7 @@ func (s *store) Lookup(name string) (string, error) { } func (s *store) DeleteLayer(id string) error { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return err } @@ -2273,7 +2328,7 @@ func (s *store) DeleteLayer(id string) error { } func (s *store) DeleteImage(id string, commit bool) (layers []string, err error) { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return nil, err } @@ -2411,7 +2466,7 @@ func (s *store) DeleteImage(id string, commit bool) (layers []string, err error) } func (s *store) DeleteContainer(id string) error { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return err } @@ -2502,7 +2557,7 @@ func (s *store) DeleteContainer(id string) error { } func (s *store) Delete(id string) error { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return err } @@ -2578,7 +2633,7 @@ func (s *store) Wipe() error { if err != nil { return err } - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return err } @@ -2615,7 +2670,7 @@ func (s *store) Wipe() error { } func (s *store) Status() ([][2]string, error) { - rlstore, err := s.LayerStore() + rlstore, err := s.ROLayerStore() if err != nil { return nil, err } @@ -2627,7 +2682,7 @@ func (s *store) Version() ([][2]string, error) { } func (s *store) mount(id string, options drivers.MountOpts) (string, error) { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return "", err } @@ -2696,7 +2751,7 @@ func (s *store) Mounted(id string) (int, error) { if layerID, err := s.ContainerLayerID(id); err == nil { id = layerID } - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return 0, err } @@ -2723,7 +2778,7 @@ func (s *store) Unmount(id string, force bool) (bool, error) { if layerID, err := s.ContainerLayerID(id); err == nil { id = layerID } - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return false, err } @@ -2741,7 +2796,7 @@ func (s *store) Unmount(id string, force bool) (bool, error) { } func (s *store) Changes(from, to string) ([]archive.Change, error) { - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return nil, err } @@ -2766,7 +2821,7 @@ func (s *store) Changes(from, to string) ([]archive.Change, error) { } func (s *store) DiffSize(from, to string) (int64, error) { - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return -1, err } @@ -2791,7 +2846,7 @@ func (s *store) DiffSize(from, to string) (int64, error) { } func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, error) { - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return nil, err } @@ -2826,7 +2881,7 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro } func (s *store) ApplyDiff(to string, diff io.Reader) (int64, error) { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return -1, err } @@ -2845,7 +2900,7 @@ func (s *store) ApplyDiff(to string, diff io.Reader) (int64, error) { func (s *store) layersByMappedDigest(m func(ROLayerStore, digest.Digest) ([]Layer, error), d digest.Digest) ([]Layer, error) { var layers []Layer - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return nil, err } @@ -2893,7 +2948,7 @@ func (s *store) LayersByUncompressedDigest(d digest.Digest) ([]Layer, error) { } func (s *store) LayerSize(id string) (int64, error) { - lstore, err := s.LayerStore() + lstore, err := s.ROLayerStore() if err != nil { return -1, err } @@ -2918,7 +2973,7 @@ func (s *store) LayerSize(id string) (int64, error) { } func (s *store) LayerParentOwners(id string) ([]int, []int, error) { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return nil, nil, err } @@ -2936,7 +2991,7 @@ func (s *store) LayerParentOwners(id string) ([]int, []int, error) { } func (s *store) ContainerParentOwners(id string) ([]int, []int, error) { - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return nil, nil, err } @@ -2968,8 +3023,8 @@ func (s *store) ContainerParentOwners(id string) ([]int, []int, error) { return nil, nil, ErrLayerUnknown } -func (s *store) Layers() ([]Layer, error) { - lstore, err := s.LayerStore() +func (s *store) RWLayers() ([]Layer, error) { + lstore, err := s.RWLayerStore() if err != nil { return nil, err } @@ -2981,6 +3036,23 @@ func (s *store) Layers() ([]Layer, error) { return nil, err } + return s.appendROLayers(layers) +} + +func (s *store) Layers() ([]Layer, error) { + lstore, err := s.RWLayerStore() + if err != nil { + return nil, err + } + layers, err := lstore.Layers() + if err != nil { + return nil, err + } + + return s.appendROLayers(layers) +} + +func (s *store) appendROLayers(layers []Layer) ([]Layer, error) { lstores, err := s.ROLayerStores() if err != nil { return nil, err @@ -3051,10 +3123,26 @@ func (s *store) Containers() ([]Container, error) { } func (s *store) Layer(id string) (*Layer, error) { - lstore, err := s.LayerStore() + lstore, err := s.RWLayerStore() if err != nil { return nil, err } + return s.layer(lstore, id) +} + +func (s *store) LayerMountpoint(id string) (string, error) { + lstore, err := s.RWLayerStore() + if err != nil { + return "", err + } + l, err := s.layer(lstore, id) + if err != nil { + return "", err + } + return l.MountPoint, nil +} + +func (s *store) layer(lstore ROLayerStore, id string) (layer *Layer, err error) { lstores, err := s.ROLayerStores() if err != nil { return nil, err @@ -3326,11 +3414,20 @@ func (s *store) FromContainerRunDirectory(id, file string) ([]byte, error) { return ioutil.ReadFile(filepath.Join(dir, file)) } +func (s *store) ShutdownRO() error { + s.graphLock.Lock() + defer s.graphLock.Unlock() + + err := s.graphDriver.Cleanup() + s.graphLock.Touch() + return err +} + func (s *store) Shutdown(force bool) ([]string, error) { mounted := []string{} modified := false - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return mounted, err } diff --git a/userns.go b/userns.go index 49ec544a33..6b8ca0378b 100644 --- a/userns.go +++ b/userns.go @@ -136,7 +136,7 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 { // getMaxSizeFromImage returns the maximum ID used by the specified image. // The layer stores must be already locked. func (s *store) getMaxSizeFromImage(id string, image *Image, passwdFile, groupFile string) (uint32, error) { - lstore, err := s.LayerStore() + lstore, err := s.ROLayerStore() if err != nil { return 0, err } @@ -178,7 +178,7 @@ outer: return 0, errors.Errorf("cannot find layer %q", layerName) } - rlstore, err := s.LayerStore() + rlstore, err := s.RWLayerStore() if err != nil { return 0, err }