Skip to content

Commit

Permalink
Lock the mounts list with its own lockfile
Browse files Browse the repository at this point in the history 
Separate loading and saving the mountpoints.json table out of the main
layer load/save paths so that they can be called independently, so that
we can mount and unmount layers (which requires that we update that
information) when the layer list itself may only be held with a read
lock.

The new loadMounts() and saveMounts() methods need to be called only for
read-write layer stores.  Callers that just refer to the mount
information can take a read lock on the mounts information, but callers
that modify the mount information need to acquire a write lock.

Break the unwritten "stores don't manage their own locks" rule and have
the layer store handle managing the lock for the mountpoints list, with
the understanding that the layer store's lock will always have been
acquired before we try to take the mounts lock.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
  • Loading branch information
@nalind
nalind committed Feb 26, 2019
1 parent 45c0592 commit 0183a29
Showing 1 changed file with 141 additions and 46 deletions.
187 changes: 141 additions & 46 deletions layers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ type LayerStore interface {

type layerStore struct {
lockfile Locker
mountsLockfile Locker
rundir string
driver drivers.Driver
layerdir string
Expand Down Expand Up @@ -291,7 +292,6 @@ func (r *layerStore) Load() error {
idlist := []string{}
ids := make(map[string]*Layer)
names := make(map[string]*Layer)
mounts := make(map[string]*Layer)
compressedsums := make(map[digest.Digest][]string)
uncompressedsums := make(map[digest.Digest][]string)
if r.lockfile.IsReadWrite() {
Expand Down Expand Up @@ -319,35 +319,25 @@ func (r *layerStore) Load() error {
label.ReserveLabel(layer.MountLabel)
}
}
err = nil
}
if shouldSave && (!r.IsReadWrite() || !r.Locked()) {
return ErrDuplicateLayerNames
}
mpath := r.mountspath()
data, err = ioutil.ReadFile(mpath)
if err != nil && !os.IsNotExist(err) {
return err
}
layerMounts := []layerMountPoint{}
if err = json.Unmarshal(data, &layerMounts); len(data) == 0 || err == nil {
for _, mount := range layerMounts {
if mount.MountPoint != "" {
if layer, ok := ids[mount.ID]; ok {
mounts[mount.MountPoint] = layer
layer.MountPoint = mount.MountPoint
layer.MountCount = mount.MountCount
}
}
}
}
r.layers = layers
r.idindex = truncindex.NewTruncIndex(idlist)
r.byid = ids
r.byname = names
r.bymount = mounts
r.bycompressedsum = compressedsums
r.byuncompressedsum = uncompressedsums
err = nil
// Load and merge information about which layers are mounted, and where.
if r.IsReadWrite() {
r.mountsLockfile.RLock()
defer r.mountsLockfile.Unlock()
if err = r.loadMounts(); err != nil {
return err
}
}
// Last step: if 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.
Expand All @@ -373,6 +363,30 @@ func (r *layerStore) Load() error {
return err
}

func (r *layerStore) loadMounts() error {
mounts := make(map[string]*Layer)
mpath := r.mountspath()
data, err := ioutil.ReadFile(mpath)
if err != nil && !os.IsNotExist(err) {
return err
}
layerMounts := []layerMountPoint{}
if err = json.Unmarshal(data, &layerMounts); len(data) == 0 || err == nil {
for _, mount := range layerMounts {
if mount.MountPoint != "" {
if layer, ok := r.lookup(mount.ID); ok {
mounts[mount.MountPoint] = layer
layer.MountPoint = mount.MountPoint
layer.MountCount = mount.MountCount
}
}
}
err = nil
}
r.bymount = mounts
return err
}

func (r *layerStore) Save() error {
if !r.IsReadWrite() {
return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to modify the layer store at %q", r.layerspath())
Expand All @@ -388,6 +402,25 @@ func (r *layerStore) Save() error {
if err != nil {
return err
}
if err := ioutils.AtomicWriteFile(rpath, jldata, 0600); err != nil {
return err
}
if !r.IsReadWrite() {
return nil
}
r.mountsLockfile.Lock()
defer r.mountsLockfile.Unlock()
defer r.mountsLockfile.Touch()
return r.saveMounts()
}

func (r *layerStore) saveMounts() error {
if !r.IsReadWrite() {
return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to modify the layer store at %q", r.layerspath())
}
if !r.mountsLockfile.Locked() {
return errors.New("layer store mount information is not locked for writing")
}
mpath := r.mountspath()
if err := os.MkdirAll(filepath.Dir(mpath), 0700); err != nil {
return err
Expand All @@ -406,11 +439,10 @@ func (r *layerStore) Save() error {
if err != nil {
return err
}
if err := ioutils.AtomicWriteFile(rpath, jldata, 0600); err != nil {
if err = ioutils.AtomicWriteFile(mpath, jmdata, 0600); err != nil {
return err
}
defer r.Touch()
return ioutils.AtomicWriteFile(mpath, jmdata, 0600)
return r.loadMounts()
}

func newLayerStore(rundir string, layerdir string, driver drivers.Driver, uidMap, gidMap []idtools.IDMap) (LayerStore, error) {
Expand All @@ -426,16 +458,21 @@ func newLayerStore(rundir string, layerdir string, driver drivers.Driver, uidMap
}
lockfile.Lock()
defer lockfile.Unlock()
mountsLockfile, err := GetLockfile(filepath.Join(rundir, "mountpoints.lock"))
if err != nil {
return nil, err
}
rlstore := layerStore{
lockfile: lockfile,
driver: driver,
rundir: rundir,
layerdir: layerdir,
byid: make(map[string]*Layer),
bymount: make(map[string]*Layer),
byname: make(map[string]*Layer),
uidMap: copyIDMap(uidMap),
gidMap: copyIDMap(gidMap),
lockfile: lockfile,
mountsLockfile: mountsLockfile,
driver: driver,
rundir: rundir,
layerdir: layerdir,
byid: make(map[string]*Layer),
bymount: make(map[string]*Layer),
byname: make(map[string]*Layer),
uidMap: copyIDMap(uidMap),
gidMap: copyIDMap(gidMap),
}
if err := rlstore.Load(); err != nil {
return nil, err
Expand All @@ -451,13 +488,14 @@ func newROLayerStore(rundir string, layerdir string, driver drivers.Driver) (ROL
lockfile.Lock()
defer lockfile.Unlock()
rlstore := layerStore{
lockfile: lockfile,
driver: driver,
rundir: rundir,
layerdir: layerdir,
byid: make(map[string]*Layer),
bymount: make(map[string]*Layer),
byname: make(map[string]*Layer),
lockfile: lockfile,
mountsLockfile: nil,
driver: driver,
rundir: rundir,
layerdir: layerdir,
byid: make(map[string]*Layer),
bymount: make(map[string]*Layer),
byname: make(map[string]*Layer),
}
if err := rlstore.Load(); err != nil {
return nil, err
Expand Down Expand Up @@ -673,6 +711,16 @@ func (r *layerStore) Create(id string, parent *Layer, names []string, mountLabel
}

func (r *layerStore) Mounted(id string) (int, error) {
if !r.IsReadWrite() {
return 0, errors.Wrapf(ErrStoreIsReadOnly, "no mount information for layers at %q", r.mountspath())
}
r.mountsLockfile.RLock()
defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil {
if err = r.loadMounts(); err != nil {
return 0, err
}
}
layer, ok := r.lookup(id)
if !ok {
return 0, ErrLayerUnknown
Expand All @@ -684,13 +732,21 @@ func (r *layerStore) Mount(id string, options drivers.MountOpts) (string, error)
if !r.IsReadWrite() {
return "", errors.Wrapf(ErrStoreIsReadOnly, "not allowed to update mount locations for layers at %q", r.mountspath())
}
r.mountsLockfile.Lock()
defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil {
if err = r.loadMounts(); err != nil {
return "", err
}
}
defer r.mountsLockfile.Touch()
layer, ok := r.lookup(id)
if !ok {
return "", ErrLayerUnknown
}
if layer.MountCount > 0 {
layer.MountCount++
return layer.MountPoint, r.Save()
return layer.MountPoint, r.saveMounts()
}
if options.MountLabel == "" {
options.MountLabel = layer.MountLabel
Expand All @@ -709,7 +765,7 @@ func (r *layerStore) Mount(id string, options drivers.MountOpts) (string, error)
layer.MountPoint = filepath.Clean(mountpoint)
layer.MountCount++
r.bymount[layer.MountPoint] = layer
err = r.Save()
err = r.saveMounts()
}
return mountpoint, err
}
Expand All @@ -718,6 +774,14 @@ func (r *layerStore) Unmount(id string, force bool) (bool, error) {
if !r.IsReadWrite() {
return false, errors.Wrapf(ErrStoreIsReadOnly, "not allowed to update mount locations for layers at %q", r.mountspath())
}
r.mountsLockfile.Lock()
defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil {
if err = r.loadMounts(); err != nil {
return false, err
}
}
defer r.mountsLockfile.Touch()
layer, ok := r.lookup(id)
if !ok {
layerByMount, ok := r.bymount[filepath.Clean(id)]
Expand All @@ -731,7 +795,7 @@ func (r *layerStore) Unmount(id string, force bool) (bool, error) {
}
if layer.MountCount > 1 {
layer.MountCount--
return true, r.Save()
return true, r.saveMounts()
}
err := r.driver.Put(id)
if err == nil || os.IsNotExist(err) {
Expand All @@ -740,12 +804,22 @@ func (r *layerStore) Unmount(id string, force bool) (bool, error) {
}
layer.MountCount--
layer.MountPoint = ""
return false, r.Save()
return false, r.saveMounts()
}
return true, err
}

func (r *layerStore) ParentOwners(id string) (uids, gids []int, err error) {
if !r.IsReadWrite() {
return nil, nil, errors.Wrapf(ErrStoreIsReadOnly, "no mount information for layers at %q", r.mountspath())
}
r.mountsLockfile.RLock()
defer r.mountsLockfile.Unlock()
if modified, err := r.mountsLockfile.Modified(); modified || err != nil {
if err = r.loadMounts(); err != nil {
return nil, nil, err
}
}
layer, ok := r.lookup(id)
if !ok {
return nil, nil, ErrLayerUnknown
Expand Down Expand Up @@ -865,12 +939,20 @@ func (r *layerStore) Delete(id string) error {
// The layer may already have been explicitly unmounted, but if not, we
// should try to clean that up before we start deleting anything at the
// driver level.
for layer.MountCount > 0 {
mountCount, err := r.Mounted(id)
if err != nil {
return errors.Wrapf(err, "error checking if layer %q is still mounted", id)
}
for mountCount > 0 {
if _, err := r.Unmount(id, false); err != nil {
return err
}
mountCount, err = r.Mounted(id)
if err != nil {
return errors.Wrapf(err, "error checking if layer %q is still mounted", id)
}
}
err := r.driver.Remove(id)
err = r.driver.Remove(id)
if err == nil {
os.Remove(r.tspath(id))
delete(r.byid, id)
Expand Down Expand Up @@ -1236,7 +1318,20 @@ func (r *layerStore) Touch() error {
}

func (r *layerStore) Modified() (bool, error) {
return r.lockfile.Modified()
var mmodified 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
}
}
return lmodified || mmodified, nil
}

func (r *layerStore) IsReadWrite() bool {
Expand Down

0 comments on commit 0183a29

Please sign in to comment.