Merge pull request #16581 from mtrmac/modified-test

Update c/storage after https://github.com/containers/storage/pull/1436
This commit is contained in:
OpenShift Merge Robot 2022-12-05 13:16:57 -05:00 committed by GitHub
commit c942f77887
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 531 additions and 210 deletions

2
go.mod
View File

@ -17,7 +17,7 @@ require (
github.com/containers/image/v5 v5.23.1-0.20221130170538-333c50e3eac8 github.com/containers/image/v5 v5.23.1-0.20221130170538-333c50e3eac8
github.com/containers/ocicrypt v1.1.6 github.com/containers/ocicrypt v1.1.6
github.com/containers/psgo v1.8.0 github.com/containers/psgo v1.8.0
github.com/containers/storage v1.44.1-0.20221121144727-71fd3e87df7a github.com/containers/storage v1.44.1-0.20221201083122-c5a80ad65f42
github.com/coreos/go-systemd/v22 v22.5.0 github.com/coreos/go-systemd/v22 v22.5.0
github.com/coreos/stream-metadata-go v0.0.0-20210225230131-70edb9eb47b3 github.com/coreos/stream-metadata-go v0.0.0-20210225230131-70edb9eb47b3
github.com/cyphar/filepath-securejoin v0.2.3 github.com/cyphar/filepath-securejoin v0.2.3

4
go.sum
View File

@ -281,8 +281,8 @@ github.com/containers/psgo v1.8.0 h1:2loGekmGAxM9ir5OsXWEfGwFxorMPYnc6gEDsGFQvhY
github.com/containers/psgo v1.8.0/go.mod h1:T8ZxnX3Ur4RvnhxFJ7t8xJ1F48RhiZB4rSrOaR/qGHc= github.com/containers/psgo v1.8.0/go.mod h1:T8ZxnX3Ur4RvnhxFJ7t8xJ1F48RhiZB4rSrOaR/qGHc=
github.com/containers/storage v1.37.0/go.mod h1:kqeJeS0b7DO2ZT1nVWs0XufrmPFbgV3c+Q/45RlH6r4= github.com/containers/storage v1.37.0/go.mod h1:kqeJeS0b7DO2ZT1nVWs0XufrmPFbgV3c+Q/45RlH6r4=
github.com/containers/storage v1.43.0/go.mod h1:uZ147thiIFGdVTjMmIw19knttQnUCl3y9zjreHrg11s= github.com/containers/storage v1.43.0/go.mod h1:uZ147thiIFGdVTjMmIw19knttQnUCl3y9zjreHrg11s=
github.com/containers/storage v1.44.1-0.20221121144727-71fd3e87df7a h1:Kds8yAenoKQ7d95T+2oOfnLJpxPAwG9grUf0lIA4JJs= github.com/containers/storage v1.44.1-0.20221201083122-c5a80ad65f42 h1:lba+h0VcMGvO/C4Q+oMhGxpgajzyQifbcedOYQNVRx8=
github.com/containers/storage v1.44.1-0.20221121144727-71fd3e87df7a/go.mod h1:pYkSXaKIGAuEQmIf/melI5wbS/JBM++6Xp4JuVTqY7U= github.com/containers/storage v1.44.1-0.20221201083122-c5a80ad65f42/go.mod h1:pYkSXaKIGAuEQmIf/melI5wbS/JBM++6Xp4JuVTqY7U=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= 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/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-iptables v0.4.5/go.mod h1:/mVI274lEDI2ns62jHCDnCyBF9Iwsmekav8Dbxlm1MU= github.com/coreos/go-iptables v0.4.5/go.mod h1:/mVI274lEDI2ns62jHCDnCyBF9Iwsmekav8Dbxlm1MU=

View File

@ -1884,7 +1884,7 @@ func (c *Container) cleanup(ctx context.Context) error {
if hoststFile, ok := c.state.BindMounts[config.DefaultHostsFile]; ok { if hoststFile, ok := c.state.BindMounts[config.DefaultHostsFile]; ok {
if _, err := os.Stat(hoststFile); err == nil { if _, err := os.Stat(hoststFile); err == nil {
// we cannot use the dependency container lock due ABBA deadlocks // we cannot use the dependency container lock due ABBA deadlocks
if lock, err := lockfile.GetLockfile(hoststFile); err == nil { if lock, err := lockfile.GetLockFile(hoststFile); err == nil {
lock.Lock() lock.Lock()
// make sure to ignore ENOENT error in case the netns container was cleaned up before this one // make sure to ignore ENOENT error in case the netns container was cleaned up before this one
if err := etchosts.Remove(hoststFile, getLocalhostHostEntry(c)); err != nil && !errors.Is(err, os.ErrNotExist) { if err := etchosts.Remove(hoststFile, getLocalhostHostEntry(c)); err != nil && !errors.Is(err, os.ErrNotExist) {

View File

@ -1685,7 +1685,7 @@ func (c *Container) makeBindMounts() error {
hostsPath, exists := bindMounts[config.DefaultHostsFile] hostsPath, exists := bindMounts[config.DefaultHostsFile]
if !c.config.UseImageHosts && exists { if !c.config.UseImageHosts && exists {
// we cannot use the dependency container lock due ABBA deadlocks in cleanup() // we cannot use the dependency container lock due ABBA deadlocks in cleanup()
lock, err := lockfile.GetLockfile(hostsPath) lock, err := lockfile.GetLockFile(hostsPath)
if err != nil { if err != nil {
return fmt.Errorf("failed to lock hosts file: %w", err) return fmt.Errorf("failed to lock hosts file: %w", err)
} }

View File

@ -45,7 +45,7 @@ func newLogFileEventer(options EventerOptions) (*EventLogFile, error) {
// Writes to the log file // Writes to the log file
func (e EventLogFile) Write(ee Event) error { func (e EventLogFile) Write(ee Event) error {
// We need to lock events file // We need to lock events file
lock, err := lockfile.GetLockfile(e.options.LogFilePath + ".lock") lock, err := lockfile.GetLockFile(e.options.LogFilePath + ".lock")
if err != nil { if err != nil {
return err return err
} }

View File

@ -7,7 +7,7 @@ import (
"strconv" "strconv"
"syscall" "syscall"
"github.com/containers/storage" "github.com/containers/storage/pkg/lockfile"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
) )
@ -150,7 +150,7 @@ func (locks *FileLocks) LockFileLock(lck uint32) error {
return fmt.Errorf("locks have already been closed: %w", syscall.EINVAL) return fmt.Errorf("locks have already been closed: %w", syscall.EINVAL)
} }
l, err := storage.GetLockfile(locks.getLockPath(lck)) l, err := lockfile.GetLockFile(locks.getLockPath(lck))
if err != nil { if err != nil {
return fmt.Errorf("acquiring lock: %w", err) return fmt.Errorf("acquiring lock: %w", err)
} }
@ -164,7 +164,7 @@ func (locks *FileLocks) UnlockFileLock(lck uint32) error {
if !locks.valid { if !locks.valid {
return fmt.Errorf("locks have already been closed: %w", syscall.EINVAL) return fmt.Errorf("locks have already been closed: %w", syscall.EINVAL)
} }
l, err := storage.GetLockfile(locks.getLockPath(lck)) l, err := lockfile.GetLockFile(locks.getLockPath(lck))
if err != nil { if err != nil {
return fmt.Errorf("acquiring lock: %w", err) return fmt.Errorf("acquiring lock: %w", err)
} }

View File

@ -451,7 +451,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
if len(rm) > 0 { if len(rm) > 0 {
// make sure to lock this file to prevent concurrent writes when // make sure to lock this file to prevent concurrent writes when
// this is used a net dependency container // this is used a net dependency container
lock, err := lockfile.GetLockfile(file) lock, err := lockfile.GetLockFile(file)
if err != nil { if err != nil {
return fmt.Errorf("failed to lock hosts file: %w", err) return fmt.Errorf("failed to lock hosts file: %w", err)
} }
@ -591,7 +591,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, netOpts types.PerNe
if file, ok := c.state.BindMounts[config.DefaultHostsFile]; ok { if file, ok := c.state.BindMounts[config.DefaultHostsFile]; ok {
// make sure to lock this file to prevent concurrent writes when // make sure to lock this file to prevent concurrent writes when
// this is used a net dependency container // this is used a net dependency container
lock, err := lockfile.GetLockfile(file) lock, err := lockfile.GetLockFile(file)
if err != nil { if err != nil {
return fmt.Errorf("failed to lock hosts file: %w", err) return fmt.Errorf("failed to lock hosts file: %w", err)
} }

View File

@ -75,7 +75,7 @@ type LinkStatistics64 struct {
type RootlessNetNS struct { type RootlessNetNS struct {
dir string dir string
Lock lockfile.Locker Lock *lockfile.LockFile
} }
// getPath will join the given path to the rootless netns dir // getPath will join the given path to the rootless netns dir

View File

@ -53,7 +53,7 @@ const (
type RootlessNetNS struct { type RootlessNetNS struct {
ns ns.NetNS ns ns.NetNS
dir string dir string
Lock lockfile.Locker Lock *lockfile.LockFile
} }
// getPath will join the given path to the rootless netns dir // getPath will join the given path to the rootless netns dir
@ -333,7 +333,7 @@ func (r *Runtime) GetRootlessNetNs(new bool) (*RootlessNetNS, error) {
runDir := r.config.Engine.TmpDir runDir := r.config.Engine.TmpDir
lfile := filepath.Join(runDir, "rootless-netns.lock") lfile := filepath.Join(runDir, "rootless-netns.lock")
lock, err := lockfile.GetLockfile(lfile) lock, err := lockfile.GetLockFile(lfile)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to get rootless-netns lockfile: %w", err) return nil, fmt.Errorf("failed to get rootless-netns lockfile: %w", err)
} }

View File

@ -15,7 +15,7 @@ import (
type RootlessNetNS struct { type RootlessNetNS struct {
dir string dir string
Lock lockfile.Locker Lock *lockfile.LockFile
} }
// ocicniPortsToNetTypesPorts convert the old port format to the new one // ocicniPortsToNetTypesPorts convert the old port format to the new one

View File

@ -14,6 +14,7 @@ import (
"github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/rootless"
"github.com/containers/podman/v4/pkg/util" "github.com/containers/podman/v4/pkg/util"
"github.com/containers/storage" "github.com/containers/storage"
"github.com/containers/storage/pkg/lockfile"
stypes "github.com/containers/storage/types" stypes "github.com/containers/storage/types"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
) )
@ -35,7 +36,7 @@ func (r *Runtime) removeAllDirs() error {
// TODO: maybe want a helper for getting the path? This is duped from // TODO: maybe want a helper for getting the path? This is duped from
// runtime.go // runtime.go
runtimeAliveLock := filepath.Join(r.config.Engine.TmpDir, "alive.lck") runtimeAliveLock := filepath.Join(r.config.Engine.TmpDir, "alive.lck")
aliveLock, err := storage.GetLockfile(runtimeAliveLock) aliveLock, err := lockfile.GetLockFile(runtimeAliveLock)
if err != nil { if err != nil {
logrus.Errorf("Lock runtime alive lock %s: %v", runtimeAliveLock, err) logrus.Errorf("Lock runtime alive lock %s: %v", runtimeAliveLock, err)
} else { } else {

View File

@ -35,6 +35,7 @@ import (
"github.com/containers/podman/v4/pkg/util" "github.com/containers/podman/v4/pkg/util"
"github.com/containers/podman/v4/utils" "github.com/containers/podman/v4/utils"
"github.com/containers/storage" "github.com/containers/storage"
"github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/unshare" "github.com/containers/storage/pkg/unshare"
"github.com/docker/docker/pkg/namesgenerator" "github.com/docker/docker/pkg/namesgenerator"
spec "github.com/opencontainers/runtime-spec/specs-go" spec "github.com/opencontainers/runtime-spec/specs-go"
@ -539,7 +540,7 @@ func makeRuntime(runtime *Runtime) (retErr error) {
// This check must be locked to prevent races // This check must be locked to prevent races
runtimeAliveLock := filepath.Join(runtime.config.Engine.TmpDir, "alive.lck") runtimeAliveLock := filepath.Join(runtime.config.Engine.TmpDir, "alive.lck")
runtimeAliveFile := filepath.Join(runtime.config.Engine.TmpDir, "alive") runtimeAliveFile := filepath.Join(runtime.config.Engine.TmpDir, "alive")
aliveLock, err := storage.GetLockfile(runtimeAliveLock) aliveLock, err := lockfile.GetLockFile(runtimeAliveLock)
if err != nil { if err != nil {
return fmt.Errorf("acquiring runtime init lock: %w", err) return fmt.Errorf("acquiring runtime init lock: %w", err)
} }

View File

@ -29,7 +29,7 @@ func TryJoinPauseProcess(pausePidPath string) (bool, int, error) {
} }
// It could not join the pause process, let's lock the file before trying to delete it. // It could not join the pause process, let's lock the file before trying to delete it.
pidFileLock, err := lockfile.GetLockfile(pausePidPath) pidFileLock, err := lockfile.GetLockFile(pausePidPath)
if err != nil { if err != nil {
// The file was deleted by another process. // The file was deleted by another process.
if os.IsNotExist(err) { if os.IsNotExist(err) {

View File

@ -23,7 +23,7 @@ import (
"github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/rootless"
"github.com/containers/podman/v4/pkg/util" "github.com/containers/podman/v4/pkg/util"
. "github.com/containers/podman/v4/test/utils" . "github.com/containers/podman/v4/test/utils"
"github.com/containers/storage" "github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/reexec" "github.com/containers/storage/pkg/reexec"
"github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/stringid"
jsoniter "github.com/json-iterator/go" jsoniter "github.com/json-iterator/go"
@ -402,9 +402,9 @@ func processTestResult(f GinkgoTestDescription) {
testResultsMutex.Unlock() testResultsMutex.Unlock()
} }
func GetPortLock(port string) storage.Locker { func GetPortLock(port string) *lockfile.LockFile {
lockFile := filepath.Join(LockTmpDir, port) lockFile := filepath.Join(LockTmpDir, port)
lock, err := storage.GetLockfile(lockFile) lock, err := lockfile.GetLockFile(lockFile)
if err != nil { if err != nil {
fmt.Println(err) fmt.Println(err)
os.Exit(1) os.Exit(1)

View File

@ -10,6 +10,7 @@ import (
"github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/ioutils"
"github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/stringid"
"github.com/containers/storage/pkg/truncindex" "github.com/containers/storage/pkg/truncindex"
digest "github.com/opencontainers/go-digest" digest "github.com/opencontainers/go-digest"
@ -140,9 +141,10 @@ type rwContainerStore interface {
} }
type containerStore struct { type containerStore struct {
lockfile Locker lockfile *lockfile.LockFile
dir string dir string
jsonPath [numContainerLocationIndex]string jsonPath [numContainerLocationIndex]string
lastWrite lockfile.LastWrite
containers []*Container containers []*Container
idindex *truncindex.TruncIndex idindex *truncindex.TruncIndex
byid map[string]*Container byid map[string]*Container
@ -262,7 +264,7 @@ func (r *containerStore) startReading() error {
r.lockfile.Lock() r.lockfile.Lock()
unlockFn = r.lockfile.Unlock unlockFn = r.lockfile.Unlock
if _, err := r.load(true); err != nil { if _, err := r.reloadIfChanged(true); err != nil {
return err return err
} }
unlockFn() unlockFn()
@ -295,19 +297,20 @@ func (r *containerStore) stopReading() {
// if it is held for writing. // if it is held for writing.
// //
// If !lockedForWriting and this function fails, the return value indicates whether // If !lockedForWriting and this function fails, the return value indicates whether
// load() with lockedForWriting could succeed. In that case the caller MUST // reloadIfChanged() with lockedForWriting could succeed.
// call load(), not reloadIfChanged() (because the “if changed” state will not
// be detected again).
func (r *containerStore) reloadIfChanged(lockedForWriting bool) (bool, error) { func (r *containerStore) reloadIfChanged(lockedForWriting bool) (bool, error) {
r.loadMut.Lock() r.loadMut.Lock()
defer r.loadMut.Unlock() defer r.loadMut.Unlock()
modified, err := r.lockfile.Modified() lastWrite, modified, err := r.lockfile.ModifiedSince(r.lastWrite)
if err != nil { if err != nil {
return false, err return false, err
} }
if modified { if modified {
return r.load(lockedForWriting) if tryLockedForWriting, err := r.load(lockedForWriting); err != nil {
return tryLockedForWriting, err // r.lastWrite is unchanged, so we will load the next time again.
}
r.lastWrite = lastWrite
} }
return false, nil return false, nil
} }
@ -363,6 +366,9 @@ func (r *containerStore) datapath(id, key string) string {
// load reloads the contents of the store from disk. // load reloads the contents of the store from disk.
// //
// Most callers should call reloadIfChanged() instead, to avoid overhead and to correctly
// manage r.lastWrite.
//
// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true // The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true
// if it is held for writing. // if it is held for writing.
// //
@ -469,7 +475,12 @@ func (r *containerStore) save(saveLocations containerLocations) error {
return err return err
} }
} }
return r.lockfile.Touch() lw, err := r.lockfile.RecordWrite()
if err != nil {
return err
}
r.lastWrite = lw
return nil
} }
func (r *containerStore) saveFor(modifiedContainer *Container) error { func (r *containerStore) saveFor(modifiedContainer *Container) error {
@ -487,7 +498,7 @@ func newContainerStore(dir string, runDir string, transient bool) (rwContainerSt
} }
volatileDir = runDir volatileDir = runDir
} }
lockfile, err := GetLockfile(filepath.Join(volatileDir, "containers.lock")) lockfile, err := lockfile.GetLockFile(filepath.Join(volatileDir, "containers.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -507,6 +518,10 @@ func newContainerStore(dir string, runDir string, transient bool) (rwContainerSt
if err := cstore.startWritingWithReload(false); err != nil { if err := cstore.startWritingWithReload(false); err != nil {
return nil, err return nil, err
} }
cstore.lastWrite, err = cstore.lockfile.GetLastWrite()
if err != nil {
return nil, err
}
defer cstore.stopWriting() defer cstore.stopWriting()
if _, err := cstore.load(true); err != nil { if _, err := cstore.load(true); err != nil {
return nil, err return nil, err

View File

@ -9,6 +9,7 @@ import (
"time" "time"
"github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/ioutils"
"github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/stringid"
"github.com/containers/storage/pkg/stringutils" "github.com/containers/storage/pkg/stringutils"
"github.com/containers/storage/pkg/truncindex" "github.com/containers/storage/pkg/truncindex"
@ -156,14 +157,15 @@ type rwImageStore interface {
} }
type imageStore struct { type imageStore struct {
lockfile Locker // lockfile.IsReadWrite can be used to distinguish between read-write and read-only image stores. lockfile *lockfile.LockFile // lockfile.IsReadWrite can be used to distinguish between read-write and read-only image stores.
dir string dir string
images []*Image lastWrite lockfile.LastWrite
idindex *truncindex.TruncIndex images []*Image
byid map[string]*Image idindex *truncindex.TruncIndex
byname map[string]*Image byid map[string]*Image
bydigest map[digest.Digest][]*Image byname map[string]*Image
loadMut sync.Mutex bydigest map[digest.Digest][]*Image
loadMut sync.Mutex
} }
func copyImage(i *Image) *Image { func copyImage(i *Image) *Image {
@ -255,7 +257,7 @@ func (r *imageStore) startReadingWithReload(canReload bool) error {
r.lockfile.Lock() r.lockfile.Lock()
unlockFn = r.lockfile.Unlock unlockFn = r.lockfile.Unlock
if _, err := r.load(true); err != nil { if _, err := r.reloadIfChanged(true); err != nil {
return err return err
} }
unlockFn() unlockFn()
@ -295,19 +297,20 @@ func (r *imageStore) stopReading() {
// if it is held for writing. // if it is held for writing.
// //
// If !lockedForWriting and this function fails, the return value indicates whether // If !lockedForWriting and this function fails, the return value indicates whether
// retrying with lockedForWriting could succeed. In that case the caller MUST // reloadIfChanged() with lockedForWriting could succeed.
// call load(), not reloadIfChanged() (because the “if changed” state will not
// be detected again).
func (r *imageStore) reloadIfChanged(lockedForWriting bool) (bool, error) { func (r *imageStore) reloadIfChanged(lockedForWriting bool) (bool, error) {
r.loadMut.Lock() r.loadMut.Lock()
defer r.loadMut.Unlock() defer r.loadMut.Unlock()
modified, err := r.lockfile.Modified() lastWrite, modified, err := r.lockfile.ModifiedSince(r.lastWrite)
if err != nil { if err != nil {
return false, err return false, err
} }
if modified { if modified {
return r.load(lockedForWriting) if tryLockedForWriting, err := r.load(lockedForWriting); err != nil {
return tryLockedForWriting, err // r.lastWrite is unchanged, so we will load the next time again.
}
r.lastWrite = lastWrite
} }
return false, nil return false, nil
} }
@ -374,6 +377,9 @@ func (i *Image) recomputeDigests() error {
// load reloads the contents of the store from disk. // load reloads the contents of the store from disk.
// //
// Most callers should call reloadIfChanged() instead, to avoid overhead and to correctly
// manage r.lastWrite.
//
// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true // The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true
// if it is held for writing. // if it is held for writing.
// //
@ -457,14 +463,19 @@ func (r *imageStore) Save() error {
if err := ioutils.AtomicWriteFile(rpath, jdata, 0600); err != nil { if err := ioutils.AtomicWriteFile(rpath, jdata, 0600); err != nil {
return err return err
} }
return r.lockfile.Touch() lw, err := r.lockfile.RecordWrite()
if err != nil {
return err
}
r.lastWrite = lw
return nil
} }
func newImageStore(dir string) (rwImageStore, error) { func newImageStore(dir string) (rwImageStore, error) {
if err := os.MkdirAll(dir, 0700); err != nil { if err := os.MkdirAll(dir, 0700); err != nil {
return nil, err return nil, err
} }
lockfile, err := GetLockfile(filepath.Join(dir, "images.lock")) lockfile, err := lockfile.GetLockFile(filepath.Join(dir, "images.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -480,6 +491,10 @@ func newImageStore(dir string) (rwImageStore, error) {
return nil, err return nil, err
} }
defer istore.stopWriting() defer istore.stopWriting()
istore.lastWrite, err = istore.lockfile.GetLastWrite()
if err != nil {
return nil, err
}
if _, err := istore.load(true); err != nil { if _, err := istore.load(true); err != nil {
return nil, err return nil, err
} }
@ -487,7 +502,7 @@ func newImageStore(dir string) (rwImageStore, error) {
} }
func newROImageStore(dir string) (roImageStore, error) { func newROImageStore(dir string) (roImageStore, error) {
lockfile, err := GetROLockfile(filepath.Join(dir, "images.lock")) lockfile, err := lockfile.GetROLockFile(filepath.Join(dir, "images.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -503,6 +518,10 @@ func newROImageStore(dir string) (roImageStore, error) {
return nil, err return nil, err
} }
defer istore.stopReading() defer istore.stopReading()
istore.lastWrite, err = istore.lockfile.GetLastWrite()
if err != nil {
return nil, err
}
if _, err := istore.load(false); err != nil { if _, err := istore.load(false); err != nil {
return nil, err return nil, err
} }

View File

@ -18,6 +18,7 @@ import (
"github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/archive"
"github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/ioutils"
"github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/mount" "github.com/containers/storage/pkg/mount"
"github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/stringid"
"github.com/containers/storage/pkg/system" "github.com/containers/storage/pkg/system"
@ -301,12 +302,14 @@ type rwLayerStore interface {
} }
type layerStore struct { type layerStore struct {
lockfile Locker lockfile *lockfile.LockFile
mountsLockfile Locker mountsLockfile *lockfile.LockFile
rundir string rundir string
jsonPath [numLayerLocationIndex]string jsonPath [numLayerLocationIndex]string
driver drivers.Driver driver drivers.Driver
layerdir string layerdir string
lastWrite lockfile.LastWrite
mountsLastWrite lockfile.LastWrite // Only valid if lockfile.IsReadWrite()
layers []*Layer layers []*Layer
idindex *truncindex.TruncIndex idindex *truncindex.TruncIndex
byid map[string]*Layer byid map[string]*Layer
@ -418,7 +421,7 @@ func (r *layerStore) startReadingWithReload(canReload bool) error {
r.lockfile.Lock() r.lockfile.Lock()
unlockFn = r.lockfile.Unlock unlockFn = r.lockfile.Unlock
if _, err := r.load(true); err != nil { if _, err := r.reloadIfChanged(true); err != nil {
return err return err
} }
unlockFn() unlockFn()
@ -447,25 +450,17 @@ func (r *layerStore) stopReading() {
r.lockfile.Unlock() r.lockfile.Unlock()
} }
// Modified() checks if the most recent writer was a party other than the // layersModified() checks if the most recent writer to r.jsonPath[] was a party other than the
// last recorded writer. It should only be called with the lock held. // last recorded writer. If so, it returns a lockfile.LastWrite value to record on a successful
func (r *layerStore) Modified() (bool, error) { // reload.
var mmodified bool // It should only be called with the lock held.
lmodified, err := r.lockfile.Modified() func (r *layerStore) layersModified() (lockfile.LastWrite, bool, error) {
lastWrite, modified, err := r.lockfile.ModifiedSince(r.lastWrite)
if err != nil { if err != nil {
return lmodified, err return lockfile.LastWrite{}, modified, err
} }
if r.lockfile.IsReadWrite() { if modified {
r.mountsLockfile.RLock() return lastWrite, true, nil
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 or container-layers.json has been // If the layers.json file or container-layers.json has been
@ -474,14 +469,15 @@ func (r *layerStore) Modified() (bool, error) {
for locationIndex := 0; locationIndex < numLayerLocationIndex; locationIndex++ { for locationIndex := 0; locationIndex < numLayerLocationIndex; locationIndex++ {
info, err := os.Stat(r.jsonPath[locationIndex]) info, err := os.Stat(r.jsonPath[locationIndex])
if err != nil && !os.IsNotExist(err) { if err != nil && !os.IsNotExist(err) {
return false, fmt.Errorf("stat layers file: %w", err) return lockfile.LastWrite{}, false, fmt.Errorf("stat layers file: %w", err)
} }
if info != nil && info.ModTime() != r.layerspathsModified[locationIndex] { if info != nil && info.ModTime() != r.layerspathsModified[locationIndex] {
return true, nil // In this case the LastWrite value is equal to r.lastWrite; writing it back doesnt hurt.
return lastWrite, true, nil
} }
} }
return false, nil return lockfile.LastWrite{}, false, nil
} }
// reloadIfChanged reloads the contents of the store from disk if it is changed. // reloadIfChanged reloads the contents of the store from disk if it is changed.
@ -490,23 +486,50 @@ func (r *layerStore) Modified() (bool, error) {
// if it is held for writing. // if it is held for writing.
// //
// If !lockedForWriting and this function fails, the return value indicates whether // If !lockedForWriting and this function fails, the return value indicates whether
// retrying with lockedForWriting could succeed. In that case the caller MUST // reloadIfChanged() with lockedForWriting could succeed.
// call load(), not reloadIfChanged() (because the “if changed” state will not
// be detected again).
func (r *layerStore) reloadIfChanged(lockedForWriting bool) (bool, error) { func (r *layerStore) reloadIfChanged(lockedForWriting bool) (bool, error) {
r.loadMut.Lock() r.loadMut.Lock()
defer r.loadMut.Unlock() defer r.loadMut.Unlock()
modified, err := r.Modified() lastWrite, layersModified, err := r.layersModified()
if err != nil { if err != nil {
return false, err return false, err
} }
if modified { if layersModified {
return r.load(lockedForWriting) // r.load also reloads mounts data; so, on this path, we dont need to call reloadMountsIfChanged.
if tryLockedForWriting, err := r.load(lockedForWriting); err != nil {
return tryLockedForWriting, err // r.lastWrite is unchanged, so we will load the next time again.
}
r.lastWrite = lastWrite
return false, nil
}
if r.lockfile.IsReadWrite() {
r.mountsLockfile.RLock()
defer r.mountsLockfile.Unlock()
if err := r.reloadMountsIfChanged(); err != nil {
return false, err
}
} }
return false, nil return false, nil
} }
// reloadMountsIfChanged reloads the contents of mountsPath from disk if it is changed.
//
// The caller must hold r.mountsLockFile for reading or writing.
func (r *layerStore) reloadMountsIfChanged() error {
lastWrite, modified, err := r.mountsLockfile.ModifiedSince(r.mountsLastWrite)
if err != nil {
return err
}
if modified {
if err = r.loadMounts(); err != nil {
return err
}
r.mountsLastWrite = lastWrite
}
return nil
}
func (r *layerStore) Layers() ([]Layer, error) { func (r *layerStore) Layers() ([]Layer, error) {
layers := make([]Layer, len(r.layers)) layers := make([]Layer, len(r.layers))
for i := range r.layers { for i := range r.layers {
@ -548,6 +571,11 @@ func (r *layerStore) mountspath() string {
// load reloads the contents of the store from disk. // load reloads the contents of the store from disk.
// //
// Most callers should call reloadIfChanged() instead, to avoid overhead and to correctly
// manage r.lastWrite.
//
// As a side effect, this sets r.mountsLastWrite.
//
// The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true // The caller must hold r.lockfile for reading _or_ writing; lockedForWriting is true
// if it is held for writing. // if it is held for writing.
// //
@ -652,9 +680,17 @@ func (r *layerStore) load(lockedForWriting bool) (bool, error) {
if r.lockfile.IsReadWrite() { if r.lockfile.IsReadWrite() {
r.mountsLockfile.RLock() r.mountsLockfile.RLock()
defer r.mountsLockfile.Unlock() defer r.mountsLockfile.Unlock()
// We need to reload mounts unconditionally, becuause by creating r.layers from scratch, we have discarded the previous
// information, if any. So, obtain a fresh mountsLastWrite value so that we dont unnecessarily reload the data
// afterwards.
mountsLastWrite, err := r.mountsLockfile.GetLastWrite()
if err != nil {
return false, err
}
if err := r.loadMounts(); err != nil { if err := r.loadMounts(); err != nil {
return false, err return false, err
} }
r.mountsLastWrite = mountsLastWrite
} }
if errorToResolveBySaving != nil { if errorToResolveBySaving != nil {
@ -779,8 +815,12 @@ func (r *layerStore) saveLayers(saveLocations layerLocations) error {
if err := ioutils.AtomicWriteFileWithOpts(rpath, jldata, 0600, opts); err != nil { if err := ioutils.AtomicWriteFileWithOpts(rpath, jldata, 0600, opts); err != nil {
return err return err
} }
return r.lockfile.Touch()
} }
lw, err := r.lockfile.RecordWrite()
if err != nil {
return err
}
r.lastWrite = lw
return nil return nil
} }
@ -810,9 +850,11 @@ func (r *layerStore) saveMounts() error {
if err = ioutils.AtomicWriteFile(mpath, jmdata, 0600); err != nil { if err = ioutils.AtomicWriteFile(mpath, jmdata, 0600); err != nil {
return err return err
} }
if err := r.mountsLockfile.Touch(); err != nil { lw, err := r.mountsLockfile.RecordWrite()
if err != nil {
return err return err
} }
r.mountsLastWrite = lw
return r.loadMounts() return r.loadMounts()
} }
@ -828,11 +870,11 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri
// layers.json might be used externally as a read-only layer (using e.g. // layers.json might be used externally as a read-only layer (using e.g.
// additionalimagestores), and that would look for the lockfile in the // additionalimagestores), and that would look for the lockfile in the
// same directory // same directory
lockfile, err := GetLockfile(filepath.Join(layerdir, "layers.lock")) lockFile, err := lockfile.GetLockFile(filepath.Join(layerdir, "layers.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
mountsLockfile, err := GetLockfile(filepath.Join(rundir, "mountpoints.lock")) mountsLockfile, err := lockfile.GetLockFile(filepath.Join(rundir, "mountpoints.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -841,7 +883,7 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri
volatileDir = rundir volatileDir = rundir
} }
rlstore := layerStore{ rlstore := layerStore{
lockfile: lockfile, lockfile: lockFile,
mountsLockfile: mountsLockfile, mountsLockfile: mountsLockfile,
driver: driver, driver: driver,
rundir: rundir, rundir: rundir,
@ -858,6 +900,12 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri
return nil, err return nil, err
} }
defer rlstore.stopWriting() defer rlstore.stopWriting()
lw, err := rlstore.lockfile.GetLastWrite()
if err != nil {
return nil, err
}
rlstore.lastWrite = lw
// rlstore.mountsLastWrite is initialized inside rlstore.load().
if _, err := rlstore.load(true); err != nil { if _, err := rlstore.load(true); err != nil {
return nil, err return nil, err
} }
@ -865,7 +913,7 @@ func (s *store) newLayerStore(rundir string, layerdir string, driver drivers.Dri
} }
func newROLayerStore(rundir string, layerdir string, driver drivers.Driver) (roLayerStore, error) { func newROLayerStore(rundir string, layerdir string, driver drivers.Driver) (roLayerStore, error) {
lockfile, err := GetROLockfile(filepath.Join(layerdir, "layers.lock")) lockfile, err := lockfile.GetROLockFile(filepath.Join(layerdir, "layers.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -887,6 +935,11 @@ func newROLayerStore(rundir string, layerdir string, driver drivers.Driver) (roL
return nil, err return nil, err
} }
defer rlstore.stopReading() defer rlstore.stopReading()
lw, err := rlstore.lockfile.GetLastWrite()
if err != nil {
return nil, err
}
rlstore.lastWrite = lw
if _, err := rlstore.load(false); err != nil { if _, err := rlstore.load(false); err != nil {
return nil, err return nil, err
} }
@ -1218,10 +1271,8 @@ func (r *layerStore) Mounted(id string) (int, error) {
} }
r.mountsLockfile.RLock() r.mountsLockfile.RLock()
defer r.mountsLockfile.Unlock() defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil { if err := r.reloadMountsIfChanged(); err != nil {
if err = r.loadMounts(); err != nil { return 0, err
return 0, err
}
} }
layer, ok := r.lookup(id) layer, ok := r.lookup(id)
if !ok { if !ok {
@ -1248,10 +1299,8 @@ func (r *layerStore) Mount(id string, options drivers.MountOpts) (string, error)
} }
r.mountsLockfile.Lock() r.mountsLockfile.Lock()
defer r.mountsLockfile.Unlock() defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil { if err := r.reloadMountsIfChanged(); err != nil {
if err = r.loadMounts(); err != nil { return "", err
return "", err
}
} }
layer, ok := r.lookup(id) layer, ok := r.lookup(id)
if !ok { if !ok {
@ -1298,10 +1347,8 @@ func (r *layerStore) Unmount(id string, force bool) (bool, error) {
} }
r.mountsLockfile.Lock() r.mountsLockfile.Lock()
defer r.mountsLockfile.Unlock() defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil { if err := r.reloadMountsIfChanged(); err != nil {
if err = r.loadMounts(); err != nil { return false, err
return false, err
}
} }
layer, ok := r.lookup(id) layer, ok := r.lookup(id)
if !ok { if !ok {
@ -1336,10 +1383,8 @@ func (r *layerStore) ParentOwners(id string) (uids, gids []int, err error) {
} }
r.mountsLockfile.RLock() r.mountsLockfile.RLock()
defer r.mountsLockfile.Unlock() defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil { if err := r.reloadMountsIfChanged(); err != nil {
if err = r.loadMounts(); err != nil { return nil, nil, err
return nil, nil, err
}
} }
layer, ok := r.lookup(id) layer, ok := r.lookup(id)
if !ok { if !ok {

View File

@ -4,12 +4,15 @@ import (
"github.com/containers/storage/pkg/lockfile" "github.com/containers/storage/pkg/lockfile"
) )
type Locker = lockfile.Locker // Deprecated: Use lockfile.*LockFile.
type Locker = lockfile.Locker //lint:ignore SA1019 // lockfile.Locker is deprecated
// Deprecated: Use lockfile.GetLockFile.
func GetLockfile(path string) (lockfile.Locker, error) { func GetLockfile(path string) (lockfile.Locker, error) {
return lockfile.GetLockfile(path) return lockfile.GetLockfile(path)
} }
// Deprecated: Use lockfile.GetROLockFile.
func GetROLockfile(path string) (lockfile.Locker, error) { func GetROLockfile(path string) (lockfile.Locker, error) {
return lockfile.GetROLockfile(path) return lockfile.GetROLockfile(path)
} }

View File

@ -10,6 +10,8 @@ import (
// A Locker represents a file lock where the file is used to cache an // A Locker represents a file lock where the file is used to cache an
// identifier of the last party that made changes to whatever's being protected // identifier of the last party that made changes to whatever's being protected
// by the lock. // by the lock.
//
// Deprecated: Refer directly to *LockFile, the provided implementation, instead.
type Locker interface { type Locker interface {
// Acquire a writer lock. // Acquire a writer lock.
// The default unix implementation panics if: // The default unix implementation panics if:
@ -28,10 +30,13 @@ type Locker interface {
// Touch records, for others sharing the lock, that the caller was the // Touch records, for others sharing the lock, that the caller was the
// last writer. It should only be called with the lock held. // last writer. It should only be called with the lock held.
//
// Deprecated: Use *LockFile.RecordWrite.
Touch() error Touch() error
// Modified() checks if the most recent writer was a party other than the // 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. // last recorded writer. It should only be called with the lock held.
// Deprecated: Use *LockFile.ModifiedSince.
Modified() (bool, error) Modified() (bool, error)
// TouchedSince() checks if the most recent writer modified the file (likely using Touch()) after the specified time. // TouchedSince() checks if the most recent writer modified the file (likely using Touch()) after the specified time.
@ -50,58 +55,76 @@ type Locker interface {
} }
var ( var (
lockfiles map[string]Locker lockFiles map[string]*LockFile
lockfilesLock sync.Mutex lockFilesLock sync.Mutex
) )
// GetLockFile opens a read-write lock file, creating it if necessary. The
// *LockFile object may already be locked if the path has already been requested
// by the current process.
func GetLockFile(path string) (*LockFile, error) {
return getLockfile(path, false)
}
// GetLockfile opens a read-write lock file, creating it if necessary. The // GetLockfile opens a read-write lock file, creating it if necessary. The
// Locker object may already be locked if the path has already been requested // Locker object may already be locked if the path has already been requested
// by the current process. // by the current process.
//
// Deprecated: Use GetLockFile
func GetLockfile(path string) (Locker, error) { func GetLockfile(path string) (Locker, error) {
return getLockfile(path, false) return GetLockFile(path)
}
// GetROLockFile opens a read-only lock file, creating it if necessary. The
// *LockFile object may already be locked if the path has already been requested
// by the current process.
func GetROLockFile(path string) (*LockFile, error) {
return getLockfile(path, true)
} }
// GetROLockfile opens a read-only lock file, creating it if necessary. The // GetROLockfile opens a read-only lock file, creating it if necessary. The
// Locker object may already be locked if the path has already been requested // Locker object may already be locked if the path has already been requested
// by the current process. // by the current process.
//
// Deprecated: Use GetROLockFile
func GetROLockfile(path string) (Locker, error) { func GetROLockfile(path string) (Locker, error) {
return getLockfile(path, true) return GetROLockFile(path)
} }
// getLockfile returns a Locker object, possibly (depending on the platform) // getLockFile returns a *LockFile object, possibly (depending on the platform)
// working inter-process, and associated with the specified path. // working inter-process, and associated with the specified path.
// //
// If ro, the lock is a read-write lock and the returned Locker should correspond to the // If ro, the lock is a read-write lock and the returned *LockFile should correspond to the
// “lock for reading” (shared) operation; otherwise, the lock is either an exclusive lock, // “lock for reading” (shared) operation; otherwise, the lock is either an exclusive lock,
// or a read-write lock and Locker should correspond to the “lock for writing” (exclusive) operation. // or a read-write lock and *LockFile should correspond to the “lock for writing” (exclusive) operation.
// //
// WARNING: // WARNING:
// - The lock may or MAY NOT be inter-process. // - The lock may or MAY NOT be inter-process.
// - There may or MAY NOT be an actual object on the filesystem created for the specified path. // - There may or MAY NOT be an actual object on the filesystem created for the specified path.
// - Even if ro, the lock MAY be exclusive. // - Even if ro, the lock MAY be exclusive.
func getLockfile(path string, ro bool) (Locker, error) { func getLockfile(path string, ro bool) (*LockFile, error) {
lockfilesLock.Lock() lockFilesLock.Lock()
defer lockfilesLock.Unlock() defer lockFilesLock.Unlock()
if lockfiles == nil { if lockFiles == nil {
lockfiles = make(map[string]Locker) lockFiles = make(map[string]*LockFile)
} }
cleanPath, err := filepath.Abs(path) cleanPath, err := filepath.Abs(path)
if err != nil { if err != nil {
return nil, fmt.Errorf("ensuring that path %q is an absolute path: %w", path, err) return nil, fmt.Errorf("ensuring that path %q is an absolute path: %w", path, err)
} }
if locker, ok := lockfiles[cleanPath]; ok { if lockFile, ok := lockFiles[cleanPath]; ok {
if ro && locker.IsReadWrite() { if ro && lockFile.IsReadWrite() {
return nil, fmt.Errorf("lock %q is not a read-only lock", cleanPath) return nil, fmt.Errorf("lock %q is not a read-only lock", cleanPath)
} }
if !ro && !locker.IsReadWrite() { if !ro && !lockFile.IsReadWrite() {
return nil, fmt.Errorf("lock %q is not a read-write lock", cleanPath) return nil, fmt.Errorf("lock %q is not a read-write lock", cleanPath)
} }
return locker, nil return lockFile, nil
} }
locker, err := createLockerForPath(cleanPath, ro) // platform-dependent locker lockFile, err := createLockFileForPath(cleanPath, ro) // platform-dependent LockFile
if err != nil { if err != nil {
return nil, err return nil, err
} }
lockfiles[cleanPath] = locker lockFiles[cleanPath] = lockFile
return locker, nil return lockFile, nil
} }

View File

@ -18,27 +18,48 @@ import (
"golang.org/x/sys/unix" "golang.org/x/sys/unix"
) )
type lockfile struct { // *LockFile represents a file lock where the file is used to cache an
// identifier of the last party that made changes to whatever's being protected
// by the lock.
//
// It MUST NOT be created manually. Use GetLockFile or GetROLockFile instead.
type LockFile struct {
// The following fields are only set when constructing *LockFile, and must never be modified afterwards.
// They are safe to access without any other locking.
file string
ro bool
// rwMutex serializes concurrent reader-writer acquisitions in the same process space // rwMutex serializes concurrent reader-writer acquisitions in the same process space
rwMutex *sync.RWMutex rwMutex *sync.RWMutex
// stateMutex is used to synchronize concurrent accesses to the state below // stateMutex is used to synchronize concurrent accesses to the state below
stateMutex *sync.Mutex stateMutex *sync.Mutex
counter int64 counter int64
file string lw LastWrite // A global value valid as of the last .Touch() or .Modified()
fd uintptr
lw []byte // "last writer"-unique value valid as of the last .Touch() or .Modified(), generated by newLastWriterID()
locktype int16 locktype int16
locked bool locked bool
ro bool // The following fields are only modified on transitions between counter == 0 / counter != 0.
// Thus, they can be safely accessed by users _that currently hold the LockFile_ without locking.
// In other cases, they need to be protected using stateMutex.
fd uintptr
}
// LastWrite is an opaque identifier of the last write to some *LockFile.
// It can be used by users of a *LockFile to determine if the lock indicates changes
// since the last check.
//
// Never construct a LastWrite manually; only accept it from *LockFile methods, and pass it back.
type LastWrite struct {
// Never modify fields of a LastWrite object; it has value semantics.
state []byte // Contents of the lock file.
} }
const lastWriterIDSize = 64 // This must be the same as len(stringid.GenerateRandomID) const lastWriterIDSize = 64 // This must be the same as len(stringid.GenerateRandomID)
var lastWriterIDCounter uint64 // Private state for newLastWriterID var lastWriterIDCounter uint64 // Private state for newLastWriterID
// newLastWriterID returns a new "last writer" ID. // newLastWrite returns a new "last write" ID.
// The value must be different on every call, and also differ from values // The value must be different on every call, and also differ from values
// generated by other processes. // generated by other processes.
func newLastWriterID() []byte { func newLastWrite() LastWrite {
// The ID is (PID, time, per-process counter, random) // The ID is (PID, time, per-process counter, random)
// PID + time represents both a unique process across reboots, // PID + time represents both a unique process across reboots,
// and a specific time within the process; the per-process counter // and a specific time within the process; the per-process counter
@ -60,7 +81,38 @@ func newLastWriterID() []byte {
panic(err) // This shouldn't happen panic(err) // This shouldn't happen
} }
return res return LastWrite{
state: res,
}
}
// newLastWriteFromData returns a LastWrite corresponding to data that came from a previous LastWrite.serialize
func newLastWriteFromData(serialized []byte) LastWrite {
if serialized == nil {
panic("newLastWriteFromData with nil data")
}
return LastWrite{
state: serialized,
}
}
// serialize returns bytes to write to the lock file to represent the specified write.
func (lw LastWrite) serialize() []byte {
if lw.state == nil {
panic("LastWrite.serialize on an uninitialized object")
}
return lw.state
}
// Equals returns true if lw matches other
func (lw LastWrite) equals(other LastWrite) bool {
if lw.state == nil {
panic("LastWrite.equals on an uninitialized object")
}
if other.state == nil {
panic("LastWrite.equals with an uninitialized counterparty")
}
return bytes.Equal(lw.state, other.state)
} }
// openLock opens the file at path and returns the corresponding file // openLock opens the file at path and returns the corresponding file
@ -84,7 +136,7 @@ func openLock(path string, ro bool) (fd int, err error) {
// the directory of the lockfile seems to be removed, try to create it // the directory of the lockfile seems to be removed, try to create it
if os.IsNotExist(err) { if os.IsNotExist(err) {
if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
return fd, fmt.Errorf("creating locker directory: %w", err) return fd, fmt.Errorf("creating lock file directory: %w", err)
} }
return openLock(path, ro) return openLock(path, ro)
@ -93,20 +145,20 @@ func openLock(path string, ro bool) (fd int, err error) {
return fd, &os.PathError{Op: "open", Path: path, Err: err} return fd, &os.PathError{Op: "open", Path: path, Err: err}
} }
// createLockerForPath returns a Locker object, possibly (depending on the platform) // createLockFileForPath returns new *LockFile object, possibly (depending on the platform)
// working inter-process and associated with the specified path. // working inter-process and associated with the specified path.
// //
// This function will be called at most once for each path value within a single process. // This function will be called at most once for each path value within a single process.
// //
// If ro, the lock is a read-write lock and the returned Locker should correspond to the // If ro, the lock is a read-write lock and the returned *LockFile should correspond to the
// “lock for reading” (shared) operation; otherwise, the lock is either an exclusive lock, // “lock for reading” (shared) operation; otherwise, the lock is either an exclusive lock,
// or a read-write lock and Locker should correspond to the “lock for writing” (exclusive) operation. // or a read-write lock and *LockFile should correspond to the “lock for writing” (exclusive) operation.
// //
// WARNING: // WARNING:
// - The lock may or MAY NOT be inter-process. // - The lock may or MAY NOT be inter-process.
// - There may or MAY NOT be an actual object on the filesystem created for the specified path. // - There may or MAY NOT be an actual object on the filesystem created for the specified path.
// - Even if ro, the lock MAY be exclusive. // - Even if ro, the lock MAY be exclusive.
func createLockerForPath(path string, ro bool) (Locker, error) { func createLockFileForPath(path string, ro bool) (*LockFile, error) {
// Check if we can open the lock. // Check if we can open the lock.
fd, err := openLock(path, ro) fd, err := openLock(path, ro)
if err != nil { if err != nil {
@ -118,19 +170,21 @@ func createLockerForPath(path string, ro bool) (Locker, error) {
if ro { if ro {
locktype = unix.F_RDLCK locktype = unix.F_RDLCK
} }
return &lockfile{ return &LockFile{
stateMutex: &sync.Mutex{}, file: path,
ro: ro,
rwMutex: &sync.RWMutex{}, rwMutex: &sync.RWMutex{},
file: path, stateMutex: &sync.Mutex{},
lw: newLastWriterID(), lw: newLastWrite(), // For compatibility, the first call of .Modified() will always report a change.
locktype: int16(locktype), locktype: int16(locktype),
locked: false, locked: false,
ro: ro}, nil }, nil
} }
// lock locks the lockfile via FCTNL(2) based on the specified type and // lock locks the lockfile via FCTNL(2) based on the specified type and
// command. // command.
func (l *lockfile) lock(lType int16) { func (l *LockFile) lock(lType int16) {
lk := unix.Flock_t{ lk := unix.Flock_t{
Type: lType, Type: lType,
Whence: int16(unix.SEEK_SET), Whence: int16(unix.SEEK_SET),
@ -168,7 +222,7 @@ func (l *lockfile) lock(lType int16) {
} }
// Lock locks the lockfile as a writer. Panic if the lock is a read-only one. // Lock locks the lockfile as a writer. Panic if the lock is a read-only one.
func (l *lockfile) Lock() { func (l *LockFile) Lock() {
if l.ro { if l.ro {
panic("can't take write lock on read-only lock file") panic("can't take write lock on read-only lock file")
} else { } else {
@ -177,12 +231,12 @@ func (l *lockfile) Lock() {
} }
// LockRead locks the lockfile as a reader. // LockRead locks the lockfile as a reader.
func (l *lockfile) RLock() { func (l *LockFile) RLock() {
l.lock(unix.F_RDLCK) l.lock(unix.F_RDLCK)
} }
// Unlock unlocks the lockfile. // Unlock unlocks the lockfile.
func (l *lockfile) Unlock() { func (l *LockFile) Unlock() {
l.stateMutex.Lock() l.stateMutex.Lock()
if !l.locked { if !l.locked {
// Panic when unlocking an unlocked lock. That's a violation // Panic when unlocking an unlocked lock. That's a violation
@ -213,7 +267,7 @@ func (l *lockfile) Unlock() {
l.stateMutex.Unlock() l.stateMutex.Unlock()
} }
func (l *lockfile) AssertLocked() { func (l *LockFile) AssertLocked() {
// DO NOT provide a variant that returns the value of l.locked. // 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 // If the caller does not hold the lock, l.locked might nevertheless be true because another goroutine does hold it, and
@ -230,7 +284,7 @@ func (l *lockfile) AssertLocked() {
} }
} }
func (l *lockfile) AssertLockedForWriting() { func (l *LockFile) AssertLockedForWriting() {
// DO NOT provide a variant that returns the current lock state. // DO NOT provide a variant that returns the current lock state.
// //
// The same caveats as for AssertLocked apply equally. // The same caveats as for AssertLocked apply equally.
@ -242,53 +296,128 @@ func (l *lockfile) AssertLockedForWriting() {
} }
} }
// Touch updates the lock file with the UID of the user. // GetLastWrite returns a LastWrite value corresponding to current state of the lock.
func (l *lockfile) Touch() error { // This is typically called before (_not after_) loading the state when initializing a consumer
// of the data protected by the lock.
// During the lifetime of the consumer, the consumer should usually call ModifiedSince instead.
//
// The caller must hold the lock (for reading or writing).
func (l *LockFile) GetLastWrite() (LastWrite, error) {
l.AssertLocked()
contents := make([]byte, lastWriterIDSize)
n, err := unix.Pread(int(l.fd), contents, 0)
if err != nil {
return LastWrite{}, err
}
// It is important to handle the partial read case, because
// the initial size of the lock file is zero, which is a valid
// state (no writes yet)
contents = contents[:n]
return newLastWriteFromData(contents), nil
}
// RecordWrite updates the lock with a new LastWrite value, and returns the new value.
//
// If this function fails, the LastWriter value of the lock is indeterminate;
// the caller should keep using the previously-recorded LastWrite value,
// and possibly detecting its own modification as an external one:
//
// lw, err := state.lock.RecordWrite()
// if err != nil { /* fail */ }
// state.lastWrite = lw
//
// The caller must hold the lock for writing.
func (l *LockFile) RecordWrite() (LastWrite, error) {
l.AssertLockedForWriting()
lw := newLastWrite()
lockContents := lw.serialize()
n, err := unix.Pwrite(int(l.fd), lockContents, 0)
if err != nil {
return LastWrite{}, err
}
if n != len(lockContents) {
return LastWrite{}, unix.ENOSPC
}
return lw, nil
}
// ModifiedSince checks if the lock has been changed since a provided LastWrite value,
// and returns the one to record instead.
//
// If ModifiedSince reports no modification, the previous LastWrite value
// is still valid and can continue to be used.
//
// If this function fails, the LastWriter value of the lock is indeterminate;
// the caller should fail and keep using the previously-recorded LastWrite value,
// so that it continues failing until the situation is resolved. Similarly,
// it should only update the recorded LastWrite value after processing the update:
//
// lw2, modified, err := state.lock.ModifiedSince(state.lastWrite)
// if err != nil { /* fail */ }
// state.lastWrite = lw2
// if modified {
// if err := reload(); err != nil { /* fail */ }
// state.lastWrite = lw2
// }
//
// The caller must hold the lock (for reading or writing).
func (l *LockFile) ModifiedSince(previous LastWrite) (LastWrite, bool, error) {
l.AssertLocked()
currentLW, err := l.GetLastWrite()
if err != nil {
return LastWrite{}, false, err
}
modified := !previous.equals(currentLW)
return currentLW, modified, nil
}
// Touch updates the lock file with to record that the current lock holder has modified the lock-protected data.
//
// Deprecated: Use *LockFile.RecordWrite.
func (l *LockFile) Touch() error {
lw, err := l.RecordWrite()
if err != nil {
return err
}
l.stateMutex.Lock() l.stateMutex.Lock()
if !l.locked || (l.locktype != unix.F_WRLCK) { if !l.locked || (l.locktype != unix.F_WRLCK) {
panic("attempted to update last-writer in lockfile without the write lock") panic("attempted to update last-writer in lockfile without the write lock")
} }
defer l.stateMutex.Unlock() defer l.stateMutex.Unlock()
l.lw = newLastWriterID() l.lw = lw
n, err := unix.Pwrite(int(l.fd), l.lw, 0)
if err != nil {
return err
}
if n != len(l.lw) {
return unix.ENOSPC
}
return nil return nil
} }
// Modified indicates if the lockfile has been updated since the last time it // Modified indicates if the lockfile has been updated since the last time it
// was loaded. // was loaded.
func (l *lockfile) Modified() (bool, error) { // NOTE: Unlike ModifiedSince, this returns true the first time it is called on a *LockFile.
// Callers cannot, in general, rely on this, because that might have happened for some other
// owner of the same *LockFile who created it previously.
//
// Deprecated: Use *LockFile.ModifiedSince.
func (l *LockFile) Modified() (bool, error) {
l.stateMutex.Lock() l.stateMutex.Lock()
if !l.locked { if !l.locked {
panic("attempted to check last-writer in lockfile without locking it first") panic("attempted to check last-writer in lockfile without locking it first")
} }
defer l.stateMutex.Unlock() defer l.stateMutex.Unlock()
currentLW := make([]byte, lastWriterIDSize) oldLW := l.lw
n, err := unix.Pread(int(l.fd), currentLW, 0) // Note that this is called with stateMutex held; thats fine because ModifiedSince doesnt need to lock it.
currentLW, modified, err := l.ModifiedSince(oldLW)
if err != nil { if err != nil {
return true, err return true, err
} }
// It is important to handle the partial read case, because
// the initial size of the lock file is zero, which is a valid
// state (no writes yet)
currentLW = currentLW[:n]
oldLW := l.lw
l.lw = currentLW l.lw = currentLW
return !bytes.Equal(currentLW, oldLW), nil return modified, nil
} }
// IsReadWriteLock indicates if the lock file is a read-write lock. // IsReadWriteLock indicates if the lock file is a read-write lock.
func (l *lockfile) IsReadWrite() bool { func (l *LockFile) IsReadWrite() bool {
return !l.ro return !l.ro
} }
// TouchedSince indicates if the lock file has been touched since the specified time // TouchedSince indicates if the lock file has been touched since the specified time
func (l *lockfile) TouchedSince(when time.Time) bool { func (l *LockFile) TouchedSince(when time.Time) bool {
st, err := system.Fstat(int(l.fd)) st, err := system.Fstat(int(l.fd))
if err != nil { if err != nil {
return true return true

View File

@ -9,45 +9,58 @@ import (
"time" "time"
) )
// createLockerForPath returns a Locker object, possibly (depending on the platform) // createLockFileForPath returns a *LockFile object, possibly (depending on the platform)
// working inter-process and associated with the specified path. // working inter-process and associated with the specified path.
// //
// This function will be called at most once for each path value within a single process. // This function will be called at most once for each path value within a single process.
// //
// If ro, the lock is a read-write lock and the returned Locker should correspond to the // If ro, the lock is a read-write lock and the returned *LockFile should correspond to the
// “lock for reading” (shared) operation; otherwise, the lock is either an exclusive lock, // “lock for reading” (shared) operation; otherwise, the lock is either an exclusive lock,
// or a read-write lock and Locker should correspond to the “lock for writing” (exclusive) operation. // or a read-write lock and *LockFile should correspond to the “lock for writing” (exclusive) operation.
// //
// WARNING: // WARNING:
// - The lock may or MAY NOT be inter-process. // - The lock may or MAY NOT be inter-process.
// - There may or MAY NOT be an actual object on the filesystem created for the specified path. // - There may or MAY NOT be an actual object on the filesystem created for the specified path.
// - Even if ro, the lock MAY be exclusive. // - Even if ro, the lock MAY be exclusive.
func createLockerForPath(path string, ro bool) (Locker, error) { func createLockFileForPath(path string, ro bool) (*LockFile, error) {
return &lockfile{locked: false}, nil return &LockFile{locked: false}, nil
} }
type lockfile struct { // *LockFile represents a file lock where the file is used to cache an
// identifier of the last party that made changes to whatever's being protected
// by the lock.
//
// It MUST NOT be created manually. Use GetLockFile or GetROLockFile instead.
type LockFile struct {
mu sync.Mutex mu sync.Mutex
file string file string
locked bool locked bool
} }
func (l *lockfile) Lock() { // LastWrite is an opaque identifier of the last write to some *LockFile.
// It can be used by users of a *LockFile to determine if the lock indicates changes
// since the last check.
// A default-initialized LastWrite never matches any last write, i.e. it always indicates changes.
type LastWrite struct {
// Nothing: The Windows “implementation” does not actually track writes.
}
func (l *LockFile) Lock() {
l.mu.Lock() l.mu.Lock()
l.locked = true l.locked = true
} }
func (l *lockfile) RLock() { func (l *LockFile) RLock() {
l.mu.Lock() l.mu.Lock()
l.locked = true l.locked = true
} }
func (l *lockfile) Unlock() { func (l *LockFile) Unlock() {
l.locked = false l.locked = false
l.mu.Unlock() l.mu.Unlock()
} }
func (l *lockfile) AssertLocked() { func (l *LockFile) AssertLocked() {
// DO NOT provide a variant that returns the value of l.locked. // 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 // If the caller does not hold the lock, l.locked might nevertheless be true because another goroutine does hold it, and
@ -59,24 +72,77 @@ func (l *lockfile) AssertLocked() {
} }
} }
func (l *lockfile) AssertLockedForWriting() { func (l *LockFile) AssertLockedForWriting() {
// DO NOT provide a variant that returns the current lock state. // DO NOT provide a variant that returns the current lock state.
// //
// The same caveats as for AssertLocked apply equally. // The same caveats as for AssertLocked apply equally.
l.AssertLocked() // The current implementation does not distinguish between read and write locks. l.AssertLocked() // The current implementation does not distinguish between read and write locks.
} }
func (l *lockfile) Modified() (bool, error) { // GetLastWrite() returns a LastWrite value corresponding to current state of the lock.
// This is typically called before (_not after_) loading the state when initializing a consumer
// of the data protected by the lock.
// During the lifetime of the consumer, the consumer should usually call ModifiedSince instead.
//
// The caller must hold the lock (for reading or writing) before this function is called.
func (l *LockFile) GetLastWrite() (LastWrite, error) {
l.AssertLocked()
return LastWrite{}, nil
}
// RecordWrite updates the lock with a new LastWrite value, and returns the new value.
//
// If this function fails, the LastWriter value of the lock is indeterminate;
// the caller should keep using the previously-recorded LastWrite value,
// and possibly detecting its own modification as an external one:
//
// lw, err := state.lock.RecordWrite()
// if err != nil { /* fail */ }
// state.lastWrite = lw
//
// The caller must hold the lock for writing.
func (l *LockFile) RecordWrite() (LastWrite, error) {
return LastWrite{}, nil
}
// ModifiedSince checks if the lock has been changed since a provided LastWrite value,
// and returns the one to record instead.
//
// If ModifiedSince reports no modification, the previous LastWrite value
// is still valid and can continue to be used.
//
// If this function fails, the LastWriter value of the lock is indeterminate;
// the caller should fail and keep using the previously-recorded LastWrite value,
// so that it continues failing until the situation is resolved. Similarly,
// it should only update the recorded LastWrite value after processing the update:
//
// lw2, modified, err := state.lock.ModifiedSince(state.lastWrite)
// if err != nil { /* fail */ }
// state.lastWrite = lw2
// if modified {
// if err := reload(); err != nil { /* fail */ }
// state.lastWrite = lw2
// }
//
// The caller must hold the lock (for reading or writing).
func (l *LockFile) ModifiedSince(previous LastWrite) (LastWrite, bool, error) {
return LastWrite{}, false, nil
}
// Deprecated: Use *LockFile.ModifiedSince.
func (l *LockFile) Modified() (bool, error) {
return false, nil return false, nil
} }
func (l *lockfile) Touch() error {
// Deprecated: Use *LockFile.RecordWrite.
func (l *LockFile) Touch() error {
return nil return nil
} }
func (l *lockfile) IsReadWrite() bool { func (l *LockFile) IsReadWrite() bool {
return false return false
} }
func (l *lockfile) TouchedSince(when time.Time) bool { func (l *LockFile) TouchedSince(when time.Time) bool {
stat, err := os.Stat(l.file) stat, err := os.Stat(l.file)
if err != nil { if err != nil {
return true return true

View File

@ -20,6 +20,7 @@ import (
"github.com/containers/storage/pkg/directory" "github.com/containers/storage/pkg/directory"
"github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/ioutils"
"github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/parsers" "github.com/containers/storage/pkg/parsers"
"github.com/containers/storage/pkg/stringutils" "github.com/containers/storage/pkg/stringutils"
"github.com/containers/storage/pkg/system" "github.com/containers/storage/pkg/system"
@ -579,30 +580,31 @@ type ContainerOptions struct {
} }
type store struct { type store struct {
lastLoaded time.Time lastLoaded time.Time
runRoot string runRoot string
graphLock Locker graphLock *lockfile.LockFile
usernsLock Locker usernsLock *lockfile.LockFile
graphRoot string graphRoot string
graphDriverName string graphDriverName string
graphOptions []string graphOptions []string
pullOptions map[string]string pullOptions map[string]string
uidMap []idtools.IDMap uidMap []idtools.IDMap
gidMap []idtools.IDMap gidMap []idtools.IDMap
autoUsernsUser string autoUsernsUser string
additionalUIDs *idSet // Set by getAvailableIDs() additionalUIDs *idSet // Set by getAvailableIDs()
additionalGIDs *idSet // Set by getAvailableIDs() additionalGIDs *idSet // Set by getAvailableIDs()
autoNsMinSize uint32 autoNsMinSize uint32
autoNsMaxSize uint32 autoNsMaxSize uint32
graphDriver drivers.Driver graphLockLastWrite lockfile.LastWrite
layerStore rwLayerStore graphDriver drivers.Driver
roLayerStores []roLayerStore layerStore rwLayerStore
imageStore rwImageStore roLayerStores []roLayerStore
roImageStores []roImageStore imageStore rwImageStore
containerStore rwContainerStore roImageStores []roImageStore
digestLockRoot string containerStore rwContainerStore
disableVolatile bool digestLockRoot string
transientStore bool disableVolatile bool
transientStore bool
} }
// GetStore attempts to find an already-created Store object matching the // GetStore attempts to find an already-created Store object matching the
@ -677,12 +679,12 @@ func GetStore(options types.StoreOptions) (Store, error) {
return nil, err return nil, err
} }
graphLock, err := GetLockfile(filepath.Join(options.GraphRoot, "storage.lock")) graphLock, err := lockfile.GetLockFile(filepath.Join(options.GraphRoot, "storage.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
usernsLock, err := GetLockfile(filepath.Join(options.GraphRoot, "userns.lock")) usernsLock, err := lockfile.GetLockFile(filepath.Join(options.GraphRoot, "userns.lock"))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -713,6 +715,18 @@ func GetStore(options types.StoreOptions) (Store, error) {
transientStore: options.TransientStore, transientStore: options.TransientStore,
pullOptions: options.PullOptions, pullOptions: options.PullOptions,
} }
if err := func() error { // A scope for defer
s.graphLock.Lock()
defer s.graphLock.Unlock()
lastWrite, err := s.graphLock.GetLastWrite()
if err != nil {
return err
}
s.graphLockLastWrite = lastWrite
return nil
}(); err != nil {
return nil, err
}
if err := s.load(); err != nil { if err := s.load(); err != nil {
return nil, err return nil, err
} }
@ -839,7 +853,7 @@ func (s *store) load() error {
// GetDigestLock returns a digest-specific Locker. // GetDigestLock returns a digest-specific Locker.
func (s *store) GetDigestLock(d digest.Digest) (Locker, error) { func (s *store) GetDigestLock(d digest.Digest) (Locker, error) {
return GetLockfile(filepath.Join(s.digestLockRoot, d.String())) return lockfile.GetLockFile(filepath.Join(s.digestLockRoot, d.String()))
} }
func (s *store) getGraphDriver() (drivers.Driver, error) { func (s *store) getGraphDriver() (drivers.Driver, error) {
@ -2510,10 +2524,11 @@ func (s *store) mount(id string, options drivers.MountOpts) (string, error) {
} }
defer rlstore.stopWriting() defer rlstore.stopWriting()
modified, err := s.graphLock.Modified() lastWrite, modified, err := s.graphLock.ModifiedSince(s.graphLockLastWrite)
if err != nil { if err != nil {
return "", err return "", err
} }
s.graphLockLastWrite = lastWrite
/* We need to make sure the home mount is present when the Mount is done. */ /* We need to make sure the home mount is present when the Mount is done. */
if modified { if modified {
@ -2657,10 +2672,11 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro
s.graphLock.Lock() s.graphLock.Lock()
defer s.graphLock.Unlock() defer s.graphLock.Unlock()
modified, err := s.graphLock.Modified() lastWrite, modified, err := s.graphLock.ModifiedSince(s.graphLockLastWrite)
if err != nil { if err != nil {
return nil, err return nil, err
} }
s.graphLockLastWrite = lastWrite
// We need to make sure the home mount is present when the Mount is done. // We need to make sure the home mount is present when the Mount is done.
if modified { if modified {
@ -3219,11 +3235,14 @@ func (s *store) Shutdown(force bool) ([]string, error) {
} }
if err == nil { if err == nil {
err = s.graphDriver.Cleanup() err = s.graphDriver.Cleanup()
if err2 := s.graphLock.Touch(); err2 != nil { // We dont retain the lastWrite value, and treat this update as if someone else did the .Cleanup(),
// so that we reload after a .Shutdown() the same way other processes would.
// Shutdown() is basically an error path, so reliability is more important than performance.
if _, err2 := s.graphLock.RecordWrite(); err2 != nil {
if err == nil { if err == nil {
err = err2 err = err2
} else { } else {
err = fmt.Errorf("(graphLock.Touch failed: %v) %w", err2, err) err = fmt.Errorf("(graphLock.RecordWrite failed: %v) %w", err2, err)
} }
} }
} }

2
vendor/modules.txt vendored
View File

@ -264,7 +264,7 @@ github.com/containers/psgo/internal/dev
github.com/containers/psgo/internal/host github.com/containers/psgo/internal/host
github.com/containers/psgo/internal/proc github.com/containers/psgo/internal/proc
github.com/containers/psgo/internal/process github.com/containers/psgo/internal/process
# github.com/containers/storage v1.44.1-0.20221121144727-71fd3e87df7a # github.com/containers/storage v1.44.1-0.20221201083122-c5a80ad65f42
## explicit; go 1.17 ## explicit; go 1.17
github.com/containers/storage github.com/containers/storage
github.com/containers/storage/drivers github.com/containers/storage/drivers