Skip to content

Commit

Permalink
overlay: lock staging directories
Browse files Browse the repository at this point in the history
lock any staging directory while it is being used so that another
process cannot delete it.

Now the Cleanup() function deletes only the staging directories that
are not locked by any other user.

Closes: #1915

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
  • Loading branch information
giuseppe committed May 2, 2024
1 parent acdb1ae commit ff3eccc
Showing 1 changed file with 70 additions and 4 deletions.
74 changes: 70 additions & 4 deletions drivers/overlay/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/containers/storage/pkg/fsutils"
"github.com/containers/storage/pkg/idmap"
"github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/mount"
"github.com/containers/storage/pkg/parsers"
"github.com/containers/storage/pkg/system"
Expand Down Expand Up @@ -127,6 +128,8 @@ type Driver struct {
usingMetacopy bool
usingComposefs bool

stagingDirsLocks map[string]*lockfile.LockFile

supportsIDMappedMounts *bool
}

Expand Down Expand Up @@ -460,6 +463,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error)
supportsVolatile: supportsVolatile,
usingComposefs: opts.useComposefs,
options: *opts,
stagingDirsLocks: make(map[string]*lockfile.LockFile),
}

d.naiveDiff = graphdriver.NewNaiveDiffDriver(d, graphdriver.NewNaiveLayerIDMapUpdater(d))
Expand Down Expand Up @@ -880,10 +884,43 @@ func (d *Driver) Metadata(id string) (map[string]string, error) {
// is being shutdown. For now, we just have to unmount the bind mount
// we had created.
func (d *Driver) Cleanup() error {
_ = os.RemoveAll(filepath.Join(d.home, stagingDir))
anyPresent := d.pruneStagingDirectories()
if anyPresent {
return nil
}
return mount.Unmount(d.home)
}

// pruneStagingDirectories cleans up any staging directory that was leaked.
// It returns whether any staging directory is still present.
func (d *Driver) pruneStagingDirectories() bool {
for _, lock := range d.stagingDirsLocks {
lock.Unlock()
}
d.stagingDirsLocks = make(map[string]*lockfile.LockFile)

anyPresent := false

dirs, err := os.ReadDir(filepath.Join(d.home, stagingDir))
if err == nil {
for _, dir := range dirs {
stagingDirToRemove := filepath.Join(d.home, stagingDir, dir.Name())
lock, err := lockfile.GetLockFile(filepath.Join(stagingDirToRemove, "staging.lock"))
if err != nil {
anyPresent = true
continue
}
if err := lock.TryLock(); err != nil {
anyPresent = true
continue
}
_ = os.RemoveAll(stagingDirToRemove)
lock.Unlock()
}
}
return anyPresent
}

// LookupAdditionalLayer looks up additional layer store by the specified
// digest and ref and returns an object representing that layer.
// This API is experimental and can be changed without bumping the major version number.
Expand Down Expand Up @@ -2029,7 +2066,14 @@ func (d *Driver) DiffGetter(id string) (graphdriver.FileGetCloser, error) {

// CleanupStagingDirectory cleanups the staging directory.
func (d *Driver) CleanupStagingDirectory(stagingDirectory string) error {
return os.RemoveAll(filepath.Dir(stagingDirectory))
parentStagingDir := filepath.Dir(stagingDirectory)

if lock, ok := d.stagingDirsLocks[parentStagingDir]; ok {
delete(d.stagingDirsLocks, parentStagingDir)
lock.Unlock()
}

return os.RemoveAll(parentStagingDir)
}

func supportsDataOnlyLayersCached(home, runhome string) (bool, error) {
Expand All @@ -2051,7 +2095,7 @@ func supportsDataOnlyLayersCached(home, runhome string) (bool, error) {
}

// ApplyDiff applies the changes in the new layer using the specified function
func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.ApplyDiffWithDifferOpts, differ graphdriver.Differ) (output graphdriver.DriverWithDifferOutput, err error) {
func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.ApplyDiffWithDifferOpts, differ graphdriver.Differ) (output graphdriver.DriverWithDifferOutput, errRet error) {
var idMappings *idtools.IDMappings
if options != nil {
idMappings = options.Mappings
Expand Down Expand Up @@ -2080,6 +2124,19 @@ func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.App
if err := os.Mkdir(applyDir, perms); err != nil {
return graphdriver.DriverWithDifferOutput{}, err
}

lock, err := lockfile.GetLockFile(filepath.Join(layerDir, "staging.lock"))
if err != nil {
return graphdriver.DriverWithDifferOutput{}, err
}
defer func() {
if errRet != nil {
delete(d.stagingDirsLocks, layerDir)
lock.Unlock()
}
}()
d.stagingDirsLocks[layerDir] = lock
lock.Lock()
} else {
var err error
applyDir, err = d.getDiffPath(id)
Expand Down Expand Up @@ -2113,9 +2170,18 @@ func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.App
// ApplyDiffFromStagingDirectory applies the changes using the specified staging directory.
func (d *Driver) ApplyDiffFromStagingDirectory(id, parent string, diffOutput *graphdriver.DriverWithDifferOutput, options *graphdriver.ApplyDiffWithDifferOpts) error {
stagingDirectory := diffOutput.Target
if filepath.Dir(filepath.Dir(stagingDirectory)) != d.getStagingDir(id) {
parentStagingDir := filepath.Dir(stagingDirectory)
if filepath.Dir(parentStagingDir) != d.getStagingDir(id) {
return fmt.Errorf("%q is not a staging directory", stagingDirectory)
}

defer func() {
if lock, ok := d.stagingDirsLocks[parentStagingDir]; ok {
delete(d.stagingDirsLocks, parentStagingDir)
lock.Unlock()
}
}()

diffPath, err := d.getDiffPath(id)
if err != nil {
return err
Expand Down

0 comments on commit ff3eccc

Please sign in to comment.