diff --git a/go.mod b/go.mod index 296f7ded83e..c372fc0a318 100644 --- a/go.mod +++ b/go.mod @@ -121,3 +121,5 @@ require ( ) replace github.com/opencontainers/image-spec => github.com/opencontainers/image-spec v1.0.2-0.20211123152302-43a7dee1ec31 + +replace github.com/containers/storage => github.com/mtrmac/storage v0.0.0-20221018233244-2513b094bbc3 diff --git a/go.sum b/go.sum index ccf36790922..e68073af314 100644 --- a/go.sum +++ b/go.sum @@ -39,7 +39,6 @@ github.com/Azure/go-autorest/autorest/mocks v0.4.1/go.mod h1:LTp+uSrOhSkaKrUy935 github.com/Azure/go-autorest/logger v0.2.0/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZmbF5NWuPV8+WeEW8= github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/BurntSushi/toml v0.4.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= github.com/BurntSushi/toml v1.2.0 h1:Rt8g24XnyGTyglgET/PRUNlrUeu9F5L+7FilkXfZgs0= github.com/BurntSushi/toml v1.2.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= @@ -52,8 +51,8 @@ github.com/Microsoft/go-winio v0.4.16/go.mod h1:XB6nPKklQyQ7GC9LdcBEcBl8PF76WugX github.com/Microsoft/go-winio v0.4.17-0.20210211115548-6eac466e5fa3/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.4.17-0.20210324224401-5516f17a5958/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.4.17/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= -github.com/Microsoft/go-winio v0.5.0/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.5.1/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= +github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY= github.com/Microsoft/go-winio v0.6.0 h1:slsWYD/zyx7lCXoZVlvQrj0hPTM1HI4+v1sIda2yDvg= github.com/Microsoft/go-winio v0.6.0/go.mod h1:cTAf44im0RAYeL23bpB+fzCyDH2MJiz2BO69KH/soAE= github.com/Microsoft/hcsshim v0.8.6/go.mod h1:Op3hHsoHPAvb6lceZHDtd9OkTew38wNoXnJs8iY7rUg= @@ -64,7 +63,6 @@ github.com/Microsoft/hcsshim v0.8.14/go.mod h1:NtVKoYxQuTLx6gEq0L96c9Ju4JbRJ4nY2 github.com/Microsoft/hcsshim v0.8.15/go.mod h1:x38A4YbHbdxJtc0sF6oIz+RG0npwSCAvn69iY6URG00= github.com/Microsoft/hcsshim v0.8.16/go.mod h1:o5/SZqmR7x9JNKsW3pu+nqHm0MF8vbA+VxGOoXdC600= github.com/Microsoft/hcsshim v0.8.21/go.mod h1:+w2gRZ5ReXQhFOrvSQeNfhrYB/dg3oDwTOcER2fw4I4= -github.com/Microsoft/hcsshim v0.8.22/go.mod h1:91uVCVzvX2QD16sMCenoxxXo6L1wJnLMX2PSufFMtF0= github.com/Microsoft/hcsshim v0.8.23/go.mod h1:4zegtUJth7lAvFyc6cH2gGQ5B3OFQim01nnU2M8jKDg= github.com/Microsoft/hcsshim v0.9.4 h1:mnUj0ivWy6UzbB1uLFqKR6F+ZyiDc7j4iGgHTpO+5+I= github.com/Microsoft/hcsshim v0.9.4/go.mod h1:7pLA8lDk46WKDWlVsENo92gC0XFa8rbKfyFRBqxEbCc= @@ -210,7 +208,6 @@ github.com/containerd/nri v0.0.0-20201007170849-eb1350a75164/go.mod h1:+2wGSDGFY github.com/containerd/nri v0.0.0-20210316161719-dbaa18c31c14/go.mod h1:lmxnXF6oMkbqs39FiCt1s0R2HSMhcLel9vNL3m4AaeY= github.com/containerd/nri v0.1.0/go.mod h1:lmxnXF6oMkbqs39FiCt1s0R2HSMhcLel9vNL3m4AaeY= github.com/containerd/stargz-snapshotter/estargz v0.4.1/go.mod h1:x7Q9dg9QYb4+ELgxmo4gBUeJB0tl5dqH1Sdz0nJU1QM= -github.com/containerd/stargz-snapshotter/estargz v0.9.0/go.mod h1:aE5PCyhFMwR8sbrErO5eM2GcvkyXTTJremG883D4qF0= github.com/containerd/stargz-snapshotter/estargz v0.12.1 h1:+7nYmHJb0tEkcRaAW+MHqoKaJYZmkikupxCqVtmPuY0= github.com/containerd/stargz-snapshotter/estargz v0.12.1/go.mod h1:12VUuCq3qPq4y8yUW+l5w3+oXV3cx2Po3KSe/SmPGqw= github.com/containerd/ttrpc v0.0.0-20190828154514-0e0f228740de/go.mod h1:PvCDdDGpgqzQIzDW1TphrGLssLDZp2GuS+X5DkEJB8o= @@ -248,9 +245,6 @@ github.com/containers/ocicrypt v1.1.0/go.mod h1:b8AOe0YR67uU8OqfVNcznfFpAzu3rdgU github.com/containers/ocicrypt v1.1.1/go.mod h1:Dm55fwWm1YZAjYRaJ94z2mfZikIyIN4B0oB3dj3jFxY= github.com/containers/ocicrypt v1.1.6 h1:uoG52u2e91RE4UqmBICZY8dNshgfvkdl3BW6jnxiFaI= github.com/containers/ocicrypt v1.1.6/go.mod h1:WgjxPWdTJMqYMjf3M6cuIFFA1/MpyyhIM99YInA+Rvc= -github.com/containers/storage v1.37.0/go.mod h1:kqeJeS0b7DO2ZT1nVWs0XufrmPFbgV3c+Q/45RlH6r4= -github.com/containers/storage v1.43.1-0.20221017123904-a4d4fe98e37d h1:x29qOIepRBNGxdkVj3igm0X+QHAXZd+qpPivPAU7+rw= -github.com/containers/storage v1.43.1-0.20221017123904-a4d4fe98e37d/go.mod h1:3DWrMisPOdjM9rZTL3g6p1ZCXSkBqaatGPgWiXClNsc= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-iptables v0.4.5/go.mod h1:/mVI274lEDI2ns62jHCDnCyBF9Iwsmekav8Dbxlm1MU= @@ -607,6 +601,8 @@ github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjY github.com/morikuni/aec v1.0.0 h1:nP9CBfwrvYnBRgY6qfDQkygYDmYwOilePFkwzv4dU8A= github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc= github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2QJNHXfbSQ= +github.com/mtrmac/storage v0.0.0-20221018233244-2513b094bbc3 h1:Drr7EuSRyg/TJPWu1TQPTeZMGjBzBVtaogvOAi0SXME= +github.com/mtrmac/storage v0.0.0-20221018233244-2513b094bbc3/go.mod h1:3DWrMisPOdjM9rZTL3g6p1ZCXSkBqaatGPgWiXClNsc= github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= @@ -674,7 +670,6 @@ github.com/opencontainers/runtime-tools v0.9.1-0.20221014010322-58c91d646d86/go. github.com/opencontainers/selinux v1.6.0/go.mod h1:VVGKuOLlE7v4PJyT6h7mNWvq1rzqiriPsEqVhc+svHE= github.com/opencontainers/selinux v1.8.0/go.mod h1:RScLhm78qiWa2gbVCcGkC7tCGdgk3ogry1nUQF8Evvo= github.com/opencontainers/selinux v1.8.2/go.mod h1:MUIHuUEvKB1wtJjQdOyYRgOnLD2xAPP8dBsCoU0KuF8= -github.com/opencontainers/selinux v1.8.5/go.mod h1:HTvjPFoGMbpQsG886e3lQwnsRWtE4TC1OF3OUvG9FAo= github.com/opencontainers/selinux v1.9.1/go.mod h1:2i0OySw99QjzBBQByd1Gr9gSjvuho1lHsJxIJ3gGbJI= github.com/opencontainers/selinux v1.10.0/go.mod h1:2i0OySw99QjzBBQByd1Gr9gSjvuho1lHsJxIJ3gGbJI= github.com/opencontainers/selinux v1.10.2 h1:NFy2xCsjn7+WspbfZkUd5zyVeisV7VFbPSP96+8/ha4= @@ -1057,7 +1052,6 @@ golang.org/x/sys v0.0.0-20210426230700-d19ff857e887/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210820121016-41cdb8703e55/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210906170528-6f6e22806c34/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211116061358-0a5406a5449c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/vendor/github.com/containers/storage/containers.go b/vendor/github.com/containers/storage/containers.go index 6e2f7238122..2182cbb98cc 100644 --- a/vendor/github.com/containers/storage/containers.go +++ b/vendor/github.com/containers/storage/containers.go @@ -68,11 +68,24 @@ type Container struct { // rwContainerStore provides bookkeeping for information about Containers. type rwContainerStore interface { - fileBasedStore metadataStore containerBigDataStore flaggableStore + // startWriting makes sure the store is fresh, and locks it for writing. + // If this succeeds, the caller MUST call stopWriting(). + startWriting() error + + // stopWriting releases locks obtained by startWriting. + stopWriting() + + // startReading makes sure the store is fresh, and locks it for reading. + // If this succeeds, the caller MUST call stopReading(). + startReading() error + + // stopReading releases locks obtained by startReading. + stopReading() + // Create creates a container that has a specified ID (or generates a // random one if an empty value is supplied) and optional names, // based on the specified image, using the specified layer as its @@ -163,6 +176,80 @@ func (c *Container) MountOpts() []string { } } +// startWritingWithReload makes sure the store is fresh if canReload, and locks it for writing. +// If this succeeds, the caller MUST call stopWriting(). +// +// This is an internal implementation detail of containerStore construction, every other caller +// should use startWriting() instead. +func (r *containerStore) startWritingWithReload(canReload bool) error { + r.lockfile.Lock() + succeeded := false + defer func() { + if !succeeded { + r.lockfile.Unlock() + } + }() + + if canReload { + if err := r.reloadIfChanged(true); err != nil { + return err + } + } + + succeeded = true + return nil +} + +// startWriting makes sure the store is fresh, and locks it for writing. +// If this succeeds, the caller MUST call stopWriting(). +func (r *containerStore) startWriting() error { + return r.startWritingWithReload(true) +} + +// stopWriting releases locks obtained by startWriting. +func (r *containerStore) stopWriting() { + r.lockfile.Unlock() +} + +// startReading makes sure the store is fresh, and locks it for reading. +// If this succeeds, the caller MUST call stopReading(). +func (r *containerStore) startReading() error { + r.lockfile.RLock() + succeeded := false + defer func() { + if !succeeded { + r.lockfile.Unlock() + } + }() + + if err := r.reloadIfChanged(false); err != nil { + return err + } + + succeeded = true + return nil +} + +// stopReading releases locks obtained by startReading. +func (r *containerStore) stopReading() { + r.lockfile.Unlock() +} + +// reloadIfChanged reloads the contents of the store from disk if it is changed. +// +// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true +// if it is held for writing. +func (r *containerStore) reloadIfChanged(lockedForWriting bool) error { + r.loadMut.Lock() + defer r.loadMut.Unlock() + + modified, err := r.lockfile.Modified() + if err == nil && modified { + return r.load(lockedForWriting) + } + return err +} + func (r *containerStore) Containers() ([]Container, error) { containers := make([]Container, len(r.containers)) for i := range r.containers { @@ -183,7 +270,11 @@ func (r *containerStore) datapath(id, key string) string { return filepath.Join(r.datadir(id), makeBigDataBaseName(key)) } -func (r *containerStore) Load() error { +// load reloads the contents of the store from disk. +// +// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true +// if it is held for writing. +func (r *containerStore) load(lockedForWriting bool) error { needSave := false rpath := r.containerspath() data, err := os.ReadFile(rpath) @@ -216,15 +307,19 @@ func (r *containerStore) Load() error { r.bylayer = layers r.byname = names if needSave { + if !lockedForWriting { + // Eventually, the callers should be modified to retry with a write lock, instead. + return errors.New("container store is inconsistent and the current caller does not hold a write lock") + } return r.Save() } return nil } +// Save saves the contents of the store to disk. It should be called with +// the lock held, locked for writing. func (r *containerStore) Save() error { - if !r.Locked() { - return errors.New("container store is not locked") - } + r.lockfile.AssertLockedForWriting() rpath := r.containerspath() if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil { return err @@ -236,7 +331,7 @@ func (r *containerStore) Save() error { if err := ioutils.AtomicWriteFile(rpath, jdata, 0600); err != nil { return err } - return r.Touch() + return r.lockfile.Touch() } func newContainerStore(dir string) (rwContainerStore, error) { @@ -255,9 +350,11 @@ func newContainerStore(dir string) (rwContainerStore, error) { bylayer: make(map[string]*Container), byname: make(map[string]*Container), } - cstore.Lock() - defer cstore.Unlock() - if err := cstore.Load(); err != nil { + if err := cstore.startWritingWithReload(false); err != nil { + return nil, err + } + defer cstore.stopWriting() + if err := cstore.load(true); err != nil { return nil, err } return &cstore, nil @@ -595,46 +692,3 @@ func (r *containerStore) Wipe() error { } return nil } - -func (r *containerStore) Lock() { - r.lockfile.Lock() -} - -func (r *containerStore) RLock() { - r.lockfile.RLock() -} - -func (r *containerStore) Unlock() { - r.lockfile.Unlock() -} - -func (r *containerStore) Touch() error { - return r.lockfile.Touch() -} - -func (r *containerStore) Modified() (bool, error) { - return r.lockfile.Modified() -} - -func (r *containerStore) IsReadWrite() bool { - return r.lockfile.IsReadWrite() -} - -func (r *containerStore) TouchedSince(when time.Time) bool { - return r.lockfile.TouchedSince(when) -} - -func (r *containerStore) Locked() bool { - return r.lockfile.Locked() -} - -func (r *containerStore) ReloadIfChanged() error { - r.loadMut.Lock() - defer r.loadMut.Unlock() - - modified, err := r.Modified() - if err == nil && modified { - return r.Load() - } - return err -} diff --git a/vendor/github.com/containers/storage/drivers/overlay/overlay.go b/vendor/github.com/containers/storage/drivers/overlay/overlay.go index 2558dc402c6..89a197469a9 100644 --- a/vendor/github.com/containers/storage/drivers/overlay/overlay.go +++ b/vendor/github.com/containers/storage/drivers/overlay/overlay.go @@ -89,7 +89,7 @@ const ( // is true (512 is a buffer for label metadata, 128 is the // number of lowers we want to be able to use without having // to use bind mounts to get all the way to the kernel limit). - // ((idLength + len(linkDir) + 1) * maxDepth) <= (pageSize - 512) + // ((idLength + len(linkDir) + 1) * 128) <= (pageSize - 512) idLength = 26 ) diff --git a/vendor/github.com/containers/storage/images.go b/vendor/github.com/containers/storage/images.go index ce45942b696..2b6ab26f7d3 100644 --- a/vendor/github.com/containers/storage/images.go +++ b/vendor/github.com/containers/storage/images.go @@ -1,7 +1,6 @@ package storage import ( - "errors" "fmt" "os" "path/filepath" @@ -96,10 +95,16 @@ type Image struct { // roImageStore provides bookkeeping for information about Images. type roImageStore interface { - roFileBasedStore roMetadataStore roBigDataStore + // startReading makes sure the store is fresh, and locks it for reading. + // If this succeeds, the caller MUST call stopReading(). + startReading() error + + // stopReading releases locks obtained by startReading. + stopReading() + // Exists checks if there is an image with the given ID or name. Exists(id string) bool @@ -119,11 +124,17 @@ type roImageStore interface { // rwImageStore provides bookkeeping for information about Images. type rwImageStore interface { roImageStore - rwFileBasedStore rwMetadataStore rwImageBigDataStore flaggableStore + // startWriting makes sure the store is fresh, and locks it for writing. + // If this succeeds, the caller MUST call stopWriting(). + startWriting() error + + // stopWriting releases locks obtained by startWriting. + stopWriting() + // Create creates an image that has a specified ID (or a random one) and // optional names, using the specified layer as its topmost (hopefully // read-only) layer. That layer can be referenced by multiple images. @@ -142,7 +153,7 @@ type rwImageStore interface { } type imageStore struct { - lockfile Locker + lockfile Locker // lockfile.IsReadWrite can be used to distinguish between read-write and read-only image stores. dir string images []*Image idindex *truncindex.TruncIndex @@ -182,6 +193,91 @@ func copyImageSlice(slice []*Image) []*Image { return nil } +// startWritingWithReload makes sure the store is fresh if canReload, and locks it for writing. +// If this succeeds, the caller MUST call stopWriting(). +// +// This is an internal implementation detail of imageStore construction, every other caller +// should use startReading() instead. +func (r *imageStore) startWritingWithReload(canReload bool) error { + r.lockfile.Lock() + succeeded := false + defer func() { + if !succeeded { + r.lockfile.Unlock() + } + }() + + if canReload { + if err := r.reloadIfChanged(true); err != nil { + return err + } + } + + succeeded = true + return nil +} + +// startWriting makes sure the store is fresh, and locks it for writing. +// If this succeeds, the caller MUST call stopWriting(). +func (r *imageStore) startWriting() error { + return r.startWritingWithReload(false) +} + +// stopWriting releases locks obtained by startWriting. +func (r *imageStore) stopWriting() { + r.lockfile.Unlock() +} + +// startReadingWithReload makes sure the store is fresh if canReload, and locks it for reading. +// If this succeeds, the caller MUST call stopReading(). +// +// This is an internal implementation detail of imageStore construction, every other caller +// should use startReading() instead. +func (r *imageStore) startReadingWithReload(canReload bool) error { + r.lockfile.RLock() + succeeded := false + defer func() { + if !succeeded { + r.lockfile.Unlock() + } + }() + + if canReload { + if err := r.reloadIfChanged(false); err != nil { + return err + } + } + + succeeded = true + return nil +} + +// startReading makes sure the store is fresh, and locks it for reading. +// If this succeeds, the caller MUST call stopReading(). +func (r *imageStore) startReading() error { + return r.startReadingWithReload(true) +} + +// stopReading releases locks obtained by startReading. +func (r *imageStore) stopReading() { + r.lockfile.Unlock() +} + +// reloadIfChanged reloads the contents of the store from disk if it is changed. +// +// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true +// if it is held for writing. +func (r *imageStore) reloadIfChanged(lockedForWriting bool) error { + r.loadMut.Lock() + defer r.loadMut.Unlock() + + modified, err := r.lockfile.Modified() + if err == nil && modified { + return r.load(lockedForWriting) + } + return err +} + func (r *imageStore) Images() ([]Image, error) { images := make([]Image, len(r.images)) for i := range r.images { @@ -242,7 +338,11 @@ func (i *Image) recomputeDigests() error { return nil } -func (r *imageStore) Load() error { +// load reloads the contents of the store from disk. +// +// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true +// if it is held for writing. +func (r *imageStore) load(lockedForWriting bool) error { shouldSave := false rpath := r.imagespath() data, err := os.ReadFile(rpath) @@ -277,10 +377,11 @@ func (r *imageStore) Load() error { list := digests[digest] digests[digest] = append(list, image) } - image.ReadOnly = !r.IsReadWrite() + image.ReadOnly = !r.lockfile.IsReadWrite() } } - if shouldSave && (!r.IsReadWrite() || !r.Locked()) { + if shouldSave && (!r.lockfile.IsReadWrite() || !lockedForWriting) { + // Eventually, the callers should be modified to retry with a write lock if IsReadWrite && !lockedForWriting, instead. return ErrDuplicateImageNames } r.images = images @@ -294,13 +395,13 @@ func (r *imageStore) Load() error { return nil } +// Save saves the contents of the store to disk. It should be called with +// the lock held, locked for writing. func (r *imageStore) Save() error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to modify the image store at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } - if !r.Locked() { - return errors.New("image store is not locked for writing") - } + r.lockfile.AssertLockedForWriting() rpath := r.imagespath() if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil { return err @@ -312,7 +413,7 @@ func (r *imageStore) Save() error { if err := ioutils.AtomicWriteFile(rpath, jdata, 0600); err != nil { return err } - return r.Touch() + return r.lockfile.Touch() } func newImageStore(dir string) (rwImageStore, error) { @@ -331,9 +432,11 @@ func newImageStore(dir string) (rwImageStore, error) { byname: make(map[string]*Image), bydigest: make(map[digest.Digest][]*Image), } - istore.Lock() - defer istore.Unlock() - if err := istore.Load(); err != nil { + if err := istore.startWritingWithReload(false); err != nil { + return nil, err + } + defer istore.stopWriting() + if err := istore.load(true); err != nil { return nil, err } return &istore, nil @@ -352,9 +455,11 @@ func newROImageStore(dir string) (roImageStore, error) { byname: make(map[string]*Image), bydigest: make(map[digest.Digest][]*Image), } - istore.RLock() - defer istore.Unlock() - if err := istore.Load(); err != nil { + if err := istore.startReadingWithReload(false); err != nil { + return nil, err + } + defer istore.stopReading() + if err := istore.load(false); err != nil { return nil, err } return &istore, nil @@ -373,7 +478,7 @@ func (r *imageStore) lookup(id string) (*Image, bool) { } func (r *imageStore) ClearFlag(id string, flag string) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to clear flags on images at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } image, ok := r.lookup(id) @@ -385,7 +490,7 @@ func (r *imageStore) ClearFlag(id string, flag string) error { } func (r *imageStore) SetFlag(id string, flag string, value interface{}) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to set flags on images at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } image, ok := r.lookup(id) @@ -400,7 +505,7 @@ func (r *imageStore) SetFlag(id string, flag string, value interface{}) error { } func (r *imageStore) Create(id string, names []string, layer, metadata string, created time.Time, searchableDigest digest.Digest) (image *Image, err error) { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return nil, fmt.Errorf("not allowed to create new images at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } if id == "" { @@ -487,7 +592,7 @@ func (r *imageStore) Metadata(id string) (string, error) { } func (r *imageStore) SetMetadata(id, metadata string) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to modify image metadata at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } if image, ok := r.lookup(id); ok { @@ -506,7 +611,7 @@ func (i *Image) addNameToHistory(name string) { } func (r *imageStore) updateNames(id string, names []string, op updateNameOperation) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to change image name assignments at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } image, ok := r.lookup(id) @@ -533,7 +638,7 @@ func (r *imageStore) updateNames(id string, names []string, op updateNameOperati } func (r *imageStore) Delete(id string) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to delete images at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } image, ok := r.lookup(id) @@ -669,7 +774,7 @@ func (r *imageStore) SetBigData(id, key string, data []byte, digestManifest func if key == "" { return fmt.Errorf("can't set empty name for image big data item: %w", ErrInvalidBigDataName) } - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to save data items associated with images at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } image, ok := r.lookup(id) @@ -750,7 +855,7 @@ func (r *imageStore) SetBigData(id, key string, data []byte, digestManifest func } func (r *imageStore) Wipe() error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to delete images at %q: %w", r.imagespath(), ErrStoreIsReadOnly) } ids := make([]string, 0, len(r.byid)) @@ -764,46 +869,3 @@ func (r *imageStore) Wipe() error { } return nil } - -func (r *imageStore) Lock() { - r.lockfile.Lock() -} - -func (r *imageStore) RLock() { - r.lockfile.RLock() -} - -func (r *imageStore) Unlock() { - r.lockfile.Unlock() -} - -func (r *imageStore) Touch() error { - return r.lockfile.Touch() -} - -func (r *imageStore) Modified() (bool, error) { - return r.lockfile.Modified() -} - -func (r *imageStore) IsReadWrite() bool { - return r.lockfile.IsReadWrite() -} - -func (r *imageStore) TouchedSince(when time.Time) bool { - return r.lockfile.TouchedSince(when) -} - -func (r *imageStore) Locked() bool { - return r.lockfile.Locked() -} - -func (r *imageStore) ReloadIfChanged() error { - r.loadMut.Lock() - defer r.loadMut.Unlock() - - modified, err := r.Modified() - if err == nil && modified { - return r.Load() - } - return err -} diff --git a/vendor/github.com/containers/storage/layers.go b/vendor/github.com/containers/storage/layers.go index 199fb6e4ac3..3e9cf9a33c4 100644 --- a/vendor/github.com/containers/storage/layers.go +++ b/vendor/github.com/containers/storage/layers.go @@ -141,10 +141,16 @@ type DiffOptions struct { // name, and keeping track of parent-child relationships, along with a list of // all known layers. type roLayerStore interface { - roFileBasedStore roMetadataStore roLayerBigDataStore + // startReading makes sure the store is fresh, and locks it for reading. + // If this succeeds, the caller MUST call stopReading(). + startReading() error + + // stopReading releases locks obtained by startReading. + stopReading() + // Exists checks if a layer with the specified name or ID is known. Exists(id string) bool @@ -194,11 +200,17 @@ type roLayerStore interface { // all known layers. type rwLayerStore interface { roLayerStore - rwFileBasedStore rwMetadataStore flaggableStore rwLayerBigDataStore + // startWriting makes sure the store is fresh, and locks it for writing. + // If this succeeds, the caller MUST call stopWriting(). + startWriting() error + + // stopWriting releases locks obtained by startWriting. + stopWriting() + // Create creates a new layer, optionally giving it a specified ID rather than // a randomly-generated one, either inheriting data from another specified // layer or the empty base layer. The new layer can optionally be given names @@ -304,6 +316,125 @@ func copyLayer(l *Layer) *Layer { } } +// startWritingWithReload makes sure the store is fresh if canReload, and locks it for writing. +// If this succeeds, the caller MUST call stopWriting(). +// +// This is an internal implementation detail of layerStore construction, every other caller +// should use startWriting() instead. +func (r *layerStore) startWritingWithReload(canReload bool) error { + r.lockfile.Lock() + succeeded := false + defer func() { + if !succeeded { + r.lockfile.Unlock() + } + }() + + if canReload { + if err := r.reloadIfChanged(true); err != nil { + return err + } + } + + succeeded = true + return nil +} + +// startWriting makes sure the store is fresh, and locks it for writing. +// If this succeeds, the caller MUST call stopWriting(). +func (r *layerStore) startWriting() error { + return r.startWritingWithReload(false) +} + +// stopWriting releases locks obtained by startWriting. +func (r *layerStore) stopWriting() { + r.lockfile.Unlock() +} + +// startReadingWithReload makes sure the store is fresh if canReload, and locks it for reading. +// If this succeeds, the caller MUST call stopReading(). +// +// This is an internal implementation detail of layerStore construction, every other caller +// should use startReading() instead. +func (r *layerStore) startReadingWithReload(canReload bool) error { + r.lockfile.RLock() + succeeded := false + defer func() { + if !succeeded { + r.lockfile.Unlock() + } + }() + + if canReload { + if err := r.reloadIfChanged(false); err != nil { + return err + } + } + + succeeded = true + return nil +} + +// startReading makes sure the store is fresh, and locks it for reading. +// If this succeeds, the caller MUST call stopReading(). +func (r *layerStore) startReading() error { + return r.startReadingWithReload(true) +} + +// stopReading releases locks obtained by startReading. +func (r *layerStore) stopReading() { + r.lockfile.Unlock() +} + +// Modified() checks if the most recent writer was a party other than the +// last recorded writer. It should only be called with the lock held. +func (r *layerStore) Modified() (bool, error) { + var mmodified, tmodified bool + lmodified, err := r.lockfile.Modified() + if err != nil { + return lmodified, err + } + if r.lockfile.IsReadWrite() { + r.mountsLockfile.RLock() + defer r.mountsLockfile.Unlock() + mmodified, err = r.mountsLockfile.Modified() + if err != nil { + return lmodified, err + } + } + + if lmodified || mmodified { + return true, nil + } + + // If the layers.json file has been modified manually, then we have to + // reload the storage in any case. + info, err := os.Stat(r.layerspath()) + if err != nil && !os.IsNotExist(err) { + return false, fmt.Errorf("stat layers file: %w", err) + } + if info != nil { + tmodified = info.ModTime() != r.layerspathModified + } + + return tmodified, nil +} + +// reloadIfChanged reloads the contents of the store from disk if it is changed. +// +// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true +// if it is held for writing. +func (r *layerStore) reloadIfChanged(lockedForWriting bool) error { + r.loadMut.Lock() + defer r.loadMut.Unlock() + + modified, err := r.Modified() + if err == nil && modified { + return r.load(lockedForWriting) + } + return err +} + func (r *layerStore) Layers() ([]Layer, error) { layers := make([]Layer, len(r.layers)) for i := range r.layers { @@ -320,7 +451,11 @@ func (r *layerStore) layerspath() string { return filepath.Join(r.layerdir, "layers.json") } -func (r *layerStore) Load() error { +// load reloads the contents of the store from disk. +// +// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true +// if it is held for writing. +func (r *layerStore) load(lockedForWriting bool) error { shouldSave := false rpath := r.layerspath() info, err := os.Stat(rpath) @@ -341,7 +476,7 @@ func (r *layerStore) Load() error { names := make(map[string]*Layer) compressedsums := make(map[digest.Digest][]string) uncompressedsums := make(map[digest.Digest][]string) - if r.IsReadWrite() { + if r.lockfile.IsReadWrite() { selinux.ClearLabels() } if err = json.Unmarshal(data, &layers); len(data) == 0 || err == nil { @@ -365,11 +500,12 @@ func (r *layerStore) Load() error { if layer.MountLabel != "" { selinux.ReserveLabel(layer.MountLabel) } - layer.ReadOnly = !r.IsReadWrite() + layer.ReadOnly = !r.lockfile.IsReadWrite() } err = nil } - if shouldSave && (!r.IsReadWrite() || !r.Locked()) { + if shouldSave && (!r.lockfile.IsReadWrite() || !lockedForWriting) { + // Eventually, the callers should be modified to retry with a write lock if IsReadWrite && !lockedForWriting, instead. return ErrDuplicateLayerNames } r.layers = layers @@ -380,7 +516,7 @@ func (r *layerStore) Load() error { r.byuncompressedsum = uncompressedsums // Load and merge information about which layers are mounted, and where. - if r.IsReadWrite() { + if r.lockfile.IsReadWrite() { r.mountsLockfile.RLock() defer r.mountsLockfile.Unlock() if err = r.loadMounts(); err != nil { @@ -390,7 +526,7 @@ func (r *layerStore) Load() error { // Last step: as we’re writable, try to remove anything that a previous // user of this storage area marked for deletion but didn't manage to // actually delete. - if r.Locked() { + if lockedForWriting { for _, layer := range r.layers { if layer.Flags == nil { layer.Flags = make(map[string]interface{}) @@ -450,6 +586,8 @@ func (r *layerStore) loadMounts() error { return err } +// Save saves the contents of the store to disk. It should be called with +// the lock held, locked for writing. func (r *layerStore) Save() error { r.mountsLockfile.Lock() defer r.mountsLockfile.Unlock() @@ -460,12 +598,10 @@ func (r *layerStore) Save() error { } func (r *layerStore) saveLayers() error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to modify the layer store at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } - if !r.Locked() { - return errors.New("layer store is not locked for writing") - } + r.lockfile.AssertLockedForWriting() rpath := r.layerspath() if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil { return err @@ -477,16 +613,14 @@ func (r *layerStore) saveLayers() error { if err := ioutils.AtomicWriteFile(rpath, jldata, 0600); err != nil { return err } - return r.Touch() + return r.lockfile.Touch() } func (r *layerStore) saveMounts() error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to modify the layer store at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } - if !r.mountsLockfile.Locked() { - return errors.New("layer store mount information is not locked for writing") - } + r.mountsLockfile.AssertLockedForWriting() mpath := r.mountspath() if err := os.MkdirAll(filepath.Dir(mpath), 0700); err != nil { return err @@ -539,9 +673,11 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri bymount: make(map[string]*Layer), byname: make(map[string]*Layer), } - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.Load(); err != nil { + if err := rlstore.startWritingWithReload(false); err != nil { + return nil, err + } + defer rlstore.stopWriting() + if err := rlstore.load(true); err != nil { return nil, err } return &rlstore, nil @@ -562,9 +698,11 @@ func newROLayerStore(rundir string, layerdir string, driver drivers.Driver) (roL bymount: make(map[string]*Layer), byname: make(map[string]*Layer), } - rlstore.RLock() - defer rlstore.Unlock() - if err := rlstore.Load(); err != nil { + if err := rlstore.startReadingWithReload(false); err != nil { + return nil, err + } + defer rlstore.stopReading() + if err := rlstore.load(false); err != nil { return nil, err } return &rlstore, nil @@ -597,7 +735,7 @@ func (r *layerStore) Size(name string) (int64, error) { } func (r *layerStore) ClearFlag(id string, flag string) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to clear flags on layers at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } layer, ok := r.lookup(id) @@ -609,7 +747,7 @@ func (r *layerStore) ClearFlag(id string, flag string) error { } func (r *layerStore) SetFlag(id string, flag string, value interface{}) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to set flags on layers at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } layer, ok := r.lookup(id) @@ -684,7 +822,7 @@ func (r *layerStore) PutAdditionalLayer(id string, parentLayer *Layer, names []s } func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLabel string, options map[string]string, moreOptions *LayerOptions, writeable bool, flags map[string]interface{}, diff io.Reader) (*Layer, int64, error) { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return nil, -1, fmt.Errorf("not allowed to create new layers at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } if err := os.MkdirAll(r.rundir, 0700); err != nil { @@ -889,7 +1027,7 @@ func (r *layerStore) Create(id string, parent *Layer, names []string, mountLabel } func (r *layerStore) Mounted(id string) (int, error) { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return 0, fmt.Errorf("no mount information for layers at %q: %w", r.mountspath(), ErrStoreIsReadOnly) } r.mountsLockfile.RLock() @@ -919,7 +1057,7 @@ func (r *layerStore) Mount(id string, options drivers.MountOpts) (string, error) // You are not allowed to mount layers from readonly stores if they // are not mounted read/only. - if !r.IsReadWrite() && !hasReadOnlyOpt(options.Options) { + if !r.lockfile.IsReadWrite() && !hasReadOnlyOpt(options.Options) { return "", fmt.Errorf("not allowed to update mount locations for layers at %q: %w", r.mountspath(), ErrStoreIsReadOnly) } r.mountsLockfile.Lock() @@ -969,7 +1107,7 @@ func (r *layerStore) Mount(id string, options drivers.MountOpts) (string, error) } func (r *layerStore) Unmount(id string, force bool) (bool, error) { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return false, fmt.Errorf("not allowed to update mount locations for layers at %q: %w", r.mountspath(), ErrStoreIsReadOnly) } r.mountsLockfile.Lock() @@ -1007,7 +1145,7 @@ func (r *layerStore) Unmount(id string, force bool) (bool, error) { } func (r *layerStore) ParentOwners(id string) (uids, gids []int, err error) { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return nil, nil, fmt.Errorf("no mount information for layers at %q: %w", r.mountspath(), ErrStoreIsReadOnly) } r.mountsLockfile.RLock() @@ -1082,7 +1220,7 @@ func (r *layerStore) removeName(layer *Layer, name string) { } func (r *layerStore) updateNames(id string, names []string, op updateNameOperation) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to change layer name assignments at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } layer, ok := r.lookup(id) @@ -1130,7 +1268,7 @@ func (r *layerStore) SetBigData(id, key string, data io.Reader) error { if key == "" { return fmt.Errorf("can't set empty name for layer big data item: %w", ErrInvalidBigDataName) } - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to save data items associated with layers at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } layer, ok := r.lookup(id) @@ -1189,7 +1327,7 @@ func (r *layerStore) Metadata(id string) (string, error) { } func (r *layerStore) SetMetadata(id, metadata string) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to modify layer metadata at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } if layer, ok := r.lookup(id); ok { @@ -1215,7 +1353,7 @@ func layerHasIncompleteFlag(layer *Layer) bool { } func (r *layerStore) deleteInternal(id string) error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to delete layers at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } layer, ok := r.lookup(id) @@ -1346,7 +1484,7 @@ func (r *layerStore) Get(id string) (*Layer, error) { } func (r *layerStore) Wipe() error { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return fmt.Errorf("not allowed to delete layers at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } ids := make([]string, 0, len(r.byid)) @@ -1611,7 +1749,7 @@ func (r *layerStore) ApplyDiff(to string, diff io.Reader) (size int64, err error } func (r *layerStore) applyDiffWithOptions(to string, layerOptions *LayerOptions, diff io.Reader) (size int64, err error) { - if !r.IsReadWrite() { + if !r.lockfile.IsReadWrite() { return -1, fmt.Errorf("not allowed to modify layer contents at %q: %w", r.layerspath(), ErrStoreIsReadOnly) } @@ -1867,77 +2005,6 @@ func (r *layerStore) LayersByUncompressedDigest(d digest.Digest) ([]Layer, error return r.layersByDigestMap(r.byuncompressedsum, d) } -func (r *layerStore) Lock() { - r.lockfile.Lock() -} - -func (r *layerStore) RLock() { - r.lockfile.RLock() -} - -func (r *layerStore) Unlock() { - r.lockfile.Unlock() -} - -func (r *layerStore) Touch() error { - return r.lockfile.Touch() -} - -func (r *layerStore) Modified() (bool, error) { - var mmodified, tmodified bool - lmodified, err := r.lockfile.Modified() - if err != nil { - return lmodified, err - } - if r.IsReadWrite() { - r.mountsLockfile.RLock() - defer r.mountsLockfile.Unlock() - mmodified, err = r.mountsLockfile.Modified() - if err != nil { - return lmodified, err - } - } - - if lmodified || mmodified { - return true, nil - } - - // If the layers.json file has been modified manually, then we have to - // reload the storage in any case. - info, err := os.Stat(r.layerspath()) - if err != nil && !os.IsNotExist(err) { - return false, fmt.Errorf("stat layers file: %w", err) - } - if info != nil { - tmodified = info.ModTime() != r.layerspathModified - } - - return tmodified, nil -} - -func (r *layerStore) IsReadWrite() bool { - return r.lockfile.IsReadWrite() -} - -func (r *layerStore) TouchedSince(when time.Time) bool { - return r.lockfile.TouchedSince(when) -} - -func (r *layerStore) Locked() bool { - return r.lockfile.Locked() -} - -func (r *layerStore) ReloadIfChanged() error { - r.loadMut.Lock() - defer r.loadMut.Unlock() - - modified, err := r.Modified() - if err == nil && modified { - return r.Load() - } - return err -} - func closeAll(closes ...func() error) (rErr error) { for _, f := range closes { if err := f(); err != nil { diff --git a/vendor/github.com/containers/storage/pkg/lockfile/lockfile.go b/vendor/github.com/containers/storage/pkg/lockfile/lockfile.go index 7cd2051d58d..f639357f4af 100644 --- a/vendor/github.com/containers/storage/pkg/lockfile/lockfile.go +++ b/vendor/github.com/containers/storage/pkg/lockfile/lockfile.go @@ -40,8 +40,13 @@ type Locker interface { // IsReadWrite() checks if the lock file is read-write IsReadWrite() bool - // Locked() checks if lock is locked for writing by a thread in this process - Locked() bool + // AssertLocked() can be used by callers that _know_ that they hold the lock (for reading or writing), for sanity checking. + // It might do nothing at all, or it may panic if the caller is not the owner of this lock. + AssertLocked() + + // AssertLocked() can be used by callers that _know_ that they hold the lock locked for writing, for sanity checking. + // It might do nothing at all, or it may panic if the caller is not the owner of this lock for writing. + AssertLockedForWriting() } var ( diff --git a/vendor/github.com/containers/storage/pkg/lockfile/lockfile_unix.go b/vendor/github.com/containers/storage/pkg/lockfile/lockfile_unix.go index deca49f79d5..21378e94201 100644 --- a/vendor/github.com/containers/storage/pkg/lockfile/lockfile_unix.go +++ b/vendor/github.com/containers/storage/pkg/lockfile/lockfile_unix.go @@ -213,11 +213,33 @@ func (l *lockfile) Unlock() { l.stateMutex.Unlock() } -// Locked checks if lockfile is locked for writing by a thread in this process. -func (l *lockfile) Locked() bool { - l.stateMutex.Lock() - defer l.stateMutex.Unlock() - return l.locked && (l.locktype == unix.F_WRLCK) +func (l *lockfile) AssertLocked() { + // DO NOT provide a variant that returns the value of l.locked. + // + // If the caller does not hold the lock, l.locked might nevertheless be true because another goroutine does hold it, and + // we can’t tell the difference. + // + // Hence, this “AssertLocked” method, which exists only for sanity checks. + + // Don’t even bother with l.stateMutex: The caller is expected to hold the lock, and in that case l.locked is constant true + // with no possible writers. + // If the caller does not hold the lock, we are violating the locking/memory model anyway, and accessing the data + // without the lock is more efficient for callers, and potentially more visible to lock analysers for incorrect callers. + if !l.locked { + panic("internal error: lock is not held by the expected owner") + } +} + +func (l *lockfile) AssertLockedForWriting() { + // DO NOT provide a variant that returns the current lock state. + // + // The same caveats as for AssertLocked apply equally. + + l.AssertLocked() + // Like AssertLocked, don’t even bother with l.stateMutex. + if l.locktype != unix.F_WRLCK { + panic("internal error: lock is not held for writing") + } } // Touch updates the lock file with the UID of the user. @@ -246,7 +268,7 @@ func (l *lockfile) Modified() (bool, error) { panic("attempted to check last-writer in lockfile without locking it first") } defer l.stateMutex.Unlock() - currentLW := make([]byte, len(l.lw)) + currentLW := make([]byte, lastWriterIDSize) n, err := unix.Pread(int(l.fd), currentLW, 0) if err != nil { return true, err diff --git a/vendor/github.com/containers/storage/pkg/lockfile/lockfile_windows.go b/vendor/github.com/containers/storage/pkg/lockfile/lockfile_windows.go index 9849f94dee7..cc898d82b55 100644 --- a/vendor/github.com/containers/storage/pkg/lockfile/lockfile_windows.go +++ b/vendor/github.com/containers/storage/pkg/lockfile/lockfile_windows.go @@ -47,8 +47,23 @@ func (l *lockfile) Unlock() { l.mu.Unlock() } -func (l *lockfile) Locked() bool { - return l.locked +func (l *lockfile) AssertLocked() { + // DO NOT provide a variant that returns the value of l.locked. + // + // If the caller does not hold the lock, l.locked might nevertheless be true because another goroutine does hold it, and + // we can’t tell the difference. + // + // Hence, this “AssertLocked” method, which exists only for sanity checks. + if !l.locked { + panic("internal error: lock is not held by the expected owner") + } +} + +func (l *lockfile) AssertLockedForWriting() { + // DO NOT provide a variant that returns the current lock state. + // + // The same caveats as for AssertLocked apply equally. + l.AssertLocked() // The current implementation does not distinguish between read and write locks. } func (l *lockfile) Modified() (bool, error) { diff --git a/vendor/github.com/containers/storage/store.go b/vendor/github.com/containers/storage/store.go index 1c646f325a1..656b9b4e475 100644 --- a/vendor/github.com/containers/storage/store.go +++ b/vendor/github.com/containers/storage/store.go @@ -50,36 +50,6 @@ var ( storesLock sync.Mutex ) -// roFileBasedStore wraps up the methods of the various types of file-based -// data stores that we implement which are needed for both read-only and -// read-write files. -type roFileBasedStore interface { - Locker - - // Load reloads the contents of the store from disk. It should be called - // with the lock held. - Load() error - - // ReloadIfChanged reloads the contents of the store from disk if it is changed. - ReloadIfChanged() error -} - -// rwFileBasedStore wraps up the methods of various types of file-based data -// stores that we implement using read-write files. -type rwFileBasedStore interface { - // Save saves the contents of the store to disk. It should be called with - // the lock held, and Touch() should be called afterward before releasing the - // lock. - Save() error -} - -// fileBasedStore wraps up the common methods of various types of file-based -// data stores that we implement. -type fileBasedStore interface { - roFileBasedStore - rwFileBasedStore -} - // roMetadataStore wraps a method for reading metadata associated with an ID. type roMetadataStore interface { // Metadata reads metadata associated with an item with the specified ID. @@ -694,10 +664,8 @@ func GetStore(options types.StoreOptions) (Store, error) { if err := os.MkdirAll(options.GraphRoot, 0700); err != nil { return nil, err } - for _, subdir := range []string{"mounts", "tmp", options.GraphDriverName} { - if err := os.MkdirAll(filepath.Join(options.GraphRoot, subdir), 0700); err != nil { - return nil, err - } + if err := os.MkdirAll(filepath.Join(options.GraphRoot, options.GraphDriverName), 0700); err != nil { + return nil, err } graphLock, err := GetLockfile(filepath.Join(options.GraphRoot, "storage.lock")) @@ -991,11 +959,10 @@ func (s *store) readAllLayerStores(fn func(store roLayerStore) (bool, error)) (b } for _, s := range layerStores { store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startReading(); err != nil { return true, err } + defer store.stopReading() if done, err := fn(store); done { return true, err } @@ -1012,11 +979,10 @@ func (s *store) writeToLayerStore(fn func(store rwLayerStore) error) error { return err } - store.Lock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startWriting(); err != nil { return err } + defer store.stopWriting() return fn(store) } @@ -1079,11 +1045,10 @@ func (s *store) readAllImageStores(fn func(store roImageStore) (bool, error)) (b } for _, s := range ImageStores { store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startReading(); err != nil { return true, err } + defer store.stopReading() if done, err := fn(store); done { return true, err } @@ -1100,11 +1065,10 @@ func (s *store) writeToImageStore(fn func(store rwImageStore) error) error { return err } - store.Lock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startWriting(); err != nil { return err } + defer store.stopWriting() return fn(store) } @@ -1126,11 +1090,10 @@ func (s *store) writeToContainerStore(fn func(store rwContainerStore) error) err return err } - store.Lock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startWriting(); err != nil { return err } + defer store.stopWriting() return fn(store) } @@ -1151,21 +1114,18 @@ func (s *store) writeToAllStores(fn func(rlstore rwLayerStore, ristore rwImageSt return err } - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startWriting(); err != nil { return err } - ristore.Lock() - defer ristore.Unlock() - if err := ristore.ReloadIfChanged(); err != nil { + defer rlstore.stopWriting() + if err := ristore.startWriting(); err != nil { return err } - rcstore.Lock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + defer ristore.stopWriting() + if err := rcstore.startWriting(); err != nil { return err } + defer rcstore.stopWriting() return fn(rlstore, ristore, rcstore) } @@ -1197,16 +1157,14 @@ func (s *store) PutLayer(id, parent string, names []string, mountLabel string, w if err != nil { return nil, -1, err } - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startWriting(); err != nil { return nil, -1, err } - rcstore.Lock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + defer rlstore.stopWriting() + if err := rcstore.startWriting(); err != nil { return nil, -1, err } + defer rcstore.stopWriting() if options == nil { options = &LayerOptions{} } @@ -1223,11 +1181,10 @@ func (s *store) PutLayer(id, parent string, names []string, mountLabel string, w for _, l := range append([]roLayerStore{rlstore}, rlstores...) { lstore := l if lstore != rlstore { - lstore.RLock() - defer lstore.Unlock() - if err := lstore.ReloadIfChanged(); err != nil { + if err := lstore.startReading(); err != nil { return nil, -1, err } + defer lstore.stopReading() } if l, err := lstore.Get(parent); err == nil && l != nil { ilayer = l @@ -1293,12 +1250,10 @@ func (s *store) CreateImage(id string, names []string, layer, metadata string, o var ilayer *Layer for _, s := range layerStores { store := s - store.RLock() - defer store.Unlock() - err := store.ReloadIfChanged() - if err != nil { + if err := store.startReading(); err != nil { return nil, err } + defer store.stopReading() ilayer, err = store.Get(layer) if err == nil { break @@ -1481,34 +1436,36 @@ func (s *store) CreateContainer(id string, names []string, image, layer, metadat if err != nil { return nil, err } - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startWriting(); err != nil { return nil, err } + defer rlstore.stopWriting() for _, s := range lstores { store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startReading(); err != nil { return nil, err } + store.stopReading() } - for _, s := range append([]roImageStore{istore}, istores...) { - store := s - if store == istore { - store.Lock() - } else { - store.RLock() - } - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { - return nil, err - } - cimage, err = store.Get(image) - if err == nil { - imageHomeStore = store - break + if err := istore.startWriting(); err != nil { + return nil, err + } + defer istore.stopWriting() + cimage, err = istore.Get(image) + if err == nil { + imageHomeStore = istore + } else { + for _, s := range istores { + store := s + if err := store.startReading(); err != nil { + return nil, err + } + defer store.stopReading() + cimage, err = store.Get(image) + if err == nil { + imageHomeStore = store + break + } } } if cimage == nil { @@ -1546,11 +1503,10 @@ func (s *store) CreateContainer(id string, names []string, image, layer, metadat } } } else { - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startWriting(); err != nil { return nil, err } + defer rlstore.stopWriting() if !options.HostUIDMapping && len(options.UIDMap) == 0 { uidMap = s.uidMap } @@ -1671,11 +1627,10 @@ func (s *store) Metadata(id string) (string, error) { if err != nil { return "", err } - cstore.RLock() - defer cstore.Unlock() - if err := cstore.ReloadIfChanged(); err != nil { + if err := cstore.startReading(); err != nil { return "", err } + defer cstore.stopReading() if cstore.Exists(id) { return cstore.Metadata(id) } @@ -1818,11 +1773,10 @@ func (s *store) ImageSize(id string) (int64, error) { } for _, s := range layerStores { store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startReading(); err != nil { return -1, err } + defer store.stopReading() } imageStores, err := s.allImageStores() @@ -1834,11 +1788,10 @@ func (s *store) ImageSize(id string) (int64, error) { var image *Image for _, s := range imageStores { store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startReading(); err != nil { return -1, err } + defer store.stopReading() if image, err = store.Get(id); err == nil { imageStore = store break @@ -1918,11 +1871,10 @@ func (s *store) ContainerSize(id string) (int64, error) { } for _, s := range layerStores { store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startReading(); err != nil { return -1, err } + defer store.stopReading() } // Get the location of the container directory and container run directory. @@ -1940,11 +1892,10 @@ func (s *store) ContainerSize(id string) (int64, error) { if err != nil { return -1, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return -1, err } + defer rcstore.stopReading() // Read the container record. container, err := rcstore.Get(id) @@ -2002,11 +1953,10 @@ func (s *store) ListContainerBigData(id string) ([]string, error) { return nil, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return nil, err } + defer rcstore.stopReading() return rcstore.BigDataNames(id) } @@ -2016,11 +1966,10 @@ func (s *store) ContainerBigDataSize(id, key string) (int64, error) { if err != nil { return -1, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return -1, err } + defer rcstore.stopReading() return rcstore.BigDataSize(id, key) } @@ -2029,11 +1978,10 @@ func (s *store) ContainerBigDataDigest(id, key string) (digest.Digest, error) { if err != nil { return "", err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return "", err } + defer rcstore.stopReading() return rcstore.BigDataDigest(id, key) } @@ -2042,11 +1990,10 @@ func (s *store) ContainerBigData(id, key string) ([]byte, error) { if err != nil { return nil, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return nil, err } + defer rcstore.stopReading() return rcstore.BigData(id, key) } @@ -2083,16 +2030,11 @@ func (s *store) Exists(id string) bool { if err != nil { return false } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return false } - if rcstore.Exists(id) { - return true - } - - return false + defer rcstore.stopReading() + return rcstore.Exists(id) } func dedupeNames(names []string) []string { @@ -2138,11 +2080,10 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e if err != nil { return err } - ristore.Lock() - defer ristore.Unlock() - if err := ristore.ReloadIfChanged(); err != nil { + if err := ristore.startWriting(); err != nil { return err } + defer ristore.stopWriting() if ristore.Exists(id) { return ristore.updateNames(id, deduped, op) } @@ -2154,20 +2095,16 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e } for _, s := range ristores { store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { + if err := store.startReading(); err != nil { return err } + defer store.stopReading() if i, err := store.Get(id); err == nil { if len(deduped) > 1 { // Do not want to create image name in R/W storage deduped = deduped[1:] } _, err := ristore.Create(id, deduped, i.TopLayer, i.Metadata, i.Created, i.Digest) - if err == nil { - return ristore.Save() - } return err } } @@ -2213,11 +2150,10 @@ func (s *store) Names(id string) ([]string, error) { if err != nil { return nil, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return nil, err } + defer rcstore.stopReading() if c, err := rcstore.Get(id); c != nil && err == nil { return c.Names, nil } @@ -2251,11 +2187,10 @@ func (s *store) Lookup(name string) (string, error) { if err != nil { return "", err } - cstore.RLock() - defer cstore.Unlock() - if err := cstore.ReloadIfChanged(); err != nil { + if err := cstore.startReading(); err != nil { return "", err } + defer cstore.stopReading() if c, err := cstore.Get(name); c != nil && err == nil { return c.ID, nil } @@ -2566,11 +2501,10 @@ func (s *store) mount(id string, options drivers.MountOpts) (string, error) { s.graphLock.Lock() defer s.graphLock.Unlock() - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startWriting(); err != nil { return "", err } + defer rlstore.stopWriting() modified, err := s.graphLock.Modified() if err != nil { @@ -2646,11 +2580,10 @@ func (s *store) Mounted(id string) (int, error) { if err != nil { return 0, err } - rlstore.RLock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startReading(); err != nil { return 0, err } + defer rlstore.stopReading() return rlstore.Mounted(id) } @@ -2738,9 +2671,7 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro for _, s := range layerStores { store := s - store.RLock() - if err := store.ReloadIfChanged(); err != nil { - store.Unlock() + if err := store.startReading(); err != nil { return nil, err } if store.Exists(to) { @@ -2748,15 +2679,15 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro if rc != nil && err == nil { wrapped := ioutils.NewReadCloserWrapper(rc, func() error { err := rc.Close() - store.Unlock() + store.stopReading() return err }) return wrapped, nil } - store.Unlock() + store.stopReading() return rc, err } - store.Unlock() + store.stopReading() } return nil, ErrLayerUnknown } @@ -2870,11 +2801,10 @@ func (s *store) LayerParentOwners(id string) ([]int, []int, error) { if err != nil { return nil, nil, err } - rlstore.RLock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startReading(); err != nil { return nil, nil, err } + defer rlstore.stopReading() if rlstore.Exists(id) { return rlstore.ParentOwners(id) } @@ -2890,16 +2820,14 @@ func (s *store) ContainerParentOwners(id string) ([]int, []int, error) { if err != nil { return nil, nil, err } - rlstore.RLock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startReading(); err != nil { return nil, nil, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + defer rlstore.stopReading() + if err := rcstore.startReading(); err != nil { return nil, nil, err } + defer rcstore.stopReading() container, err := rcstore.Get(id) if err != nil { return nil, nil, err @@ -2946,11 +2874,10 @@ func (s *store) Containers() ([]Container, error) { return nil, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return nil, err } + defer rcstore.stopReading() return rcstore.Containers() } @@ -3014,11 +2941,10 @@ func (al *additionalLayer) PutAs(id, parent string, names []string) (*Layer, err if err != nil { return nil, err } - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startWriting(); err != nil { return nil, err } + defer rlstore.stopWriting() rlstores, err := al.s.getROLayerStores() if err != nil { return nil, err @@ -3028,11 +2954,10 @@ func (al *additionalLayer) PutAs(id, parent string, names []string) (*Layer, err if parent != "" { for _, lstore := range append([]roLayerStore{rlstore}, rlstores...) { if lstore != rlstore { - lstore.RLock() - defer lstore.Unlock() - if err := lstore.ReloadIfChanged(); err != nil { + if err := lstore.startReading(); err != nil { return nil, err } + defer lstore.stopReading() } parentLayer, err = lstore.Get(parent) if err == nil { @@ -3111,11 +3036,10 @@ func (s *store) Container(id string) (*Container, error) { if err != nil { return nil, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return nil, err } + defer rcstore.stopReading() return rcstore.Get(id) } @@ -3125,11 +3049,10 @@ func (s *store) ContainerLayerID(id string) (string, error) { if err != nil { return "", err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return "", err } + defer rcstore.stopReading() container, err := rcstore.Get(id) if err != nil { return "", err @@ -3146,11 +3069,10 @@ func (s *store) ContainerByLayer(id string) (*Container, error) { if err != nil { return nil, err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return nil, err } + defer rcstore.stopReading() containerList, err := rcstore.Containers() if err != nil { return nil, err @@ -3169,11 +3091,10 @@ func (s *store) ContainerDirectory(id string) (string, error) { if err != nil { return "", err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return "", err } + defer rcstore.stopReading() id, err = rcstore.Lookup(id) if err != nil { @@ -3194,11 +3115,10 @@ func (s *store) ContainerRunDirectory(id string) (string, error) { return "", err } - rcstore.RLock() - defer rcstore.Unlock() - if err := rcstore.ReloadIfChanged(); err != nil { + if err := rcstore.startReading(); err != nil { return "", err } + defer rcstore.stopReading() id, err = rcstore.Lookup(id) if err != nil { @@ -3264,11 +3184,10 @@ func (s *store) Shutdown(force bool) ([]string, error) { s.graphLock.Lock() defer s.graphLock.Unlock() - rlstore.Lock() - defer rlstore.Unlock() - if err := rlstore.ReloadIfChanged(); err != nil { + if err := rlstore.startWriting(); err != nil { return nil, err } + defer rlstore.stopWriting() layers, err := rlstore.Layers() if err != nil { diff --git a/vendor/modules.txt b/vendor/modules.txt index 087cda33c67..06e8e407f30 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -200,7 +200,7 @@ github.com/containers/ocicrypt/keywrap/pkcs7 github.com/containers/ocicrypt/spec github.com/containers/ocicrypt/utils github.com/containers/ocicrypt/utils/keyprovider -# github.com/containers/storage v1.43.1-0.20221017123904-a4d4fe98e37d +# github.com/containers/storage v1.43.1-0.20221017123904-a4d4fe98e37d => github.com/mtrmac/storage v0.0.0-20221018233244-2513b094bbc3 ## explicit; go 1.17 github.com/containers/storage github.com/containers/storage/drivers @@ -813,3 +813,4 @@ gopkg.in/yaml.v3 ## explicit; go 1.12 k8s.io/klog # github.com/opencontainers/image-spec => github.com/opencontainers/image-spec v1.0.2-0.20211123152302-43a7dee1ec31 +# github.com/containers/storage => github.com/mtrmac/storage v0.0.0-20221018233244-2513b094bbc3