diff options
| -rw-r--r-- | libpod/boltdb_state_internal.go | 5 | ||||
| -rw-r--r-- | libpod/container.go | 5 | ||||
| -rw-r--r-- | libpod/container_easyjson.go | 12 | ||||
| -rw-r--r-- | libpod/container_internal.go | 13 | ||||
| -rw-r--r-- | libpod/runtime.go | 11 | ||||
| -rw-r--r-- | libpod/runtime_ctr.go | 27 | ||||
| -rw-r--r-- | libpod/runtime_pod_linux.go | 5 |
7 files changed, 59 insertions, 19 deletions
diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 06f8dcb24..238bfa161 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -288,10 +288,9 @@ func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt. } // Get the lock - lockPath := filepath.Join(s.runtime.lockDir, string(id)) - lock, err := storage.GetLockfile(lockPath) + lock, err := s.runtime.lockManager.RetrieveLock(ctr.config.LockID) if err != nil { - return errors.Wrapf(err, "error retrieving lockfile for container %s", string(id)) + return errors.Wrapf(err, "error retrieving lock for container %s", string(id)) } ctr.lock = lock diff --git a/libpod/container.go b/libpod/container.go index b4190344a..4e5088b32 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -9,6 +9,7 @@ import ( "github.com/containernetworking/cni/pkg/types" cnitypes "github.com/containernetworking/cni/pkg/types/current" + "github.com/containers/libpod/libpod/lock" "github.com/containers/libpod/pkg/namespaces" "github.com/containers/storage" "github.com/cri-o/ocicni/pkg/ocicni" @@ -122,7 +123,7 @@ type Container struct { batched bool valid bool - lock storage.Locker + lock lock.Locker runtime *Runtime rootlessSlirpSyncR *os.File @@ -211,6 +212,8 @@ type Config struct { Pod string `json:"pod,omitempty"` // Namespace the container is in Namespace string `json:"namespace,omitempty"` + // ID of this container's lock + LockID uint32 `json:"lockID"` // TODO consider breaking these subsections up into smaller structs diff --git a/libpod/container_easyjson.go b/libpod/container_easyjson.go index 299a645e1..ae0972cf6 100644 --- a/libpod/container_easyjson.go +++ b/libpod/container_easyjson.go @@ -1275,6 +1275,8 @@ func easyjson1dbef17bDecodeGithubComContainersLibpodLibpod2(in *jlexer.Lexer, ou out.Pod = string(in.String()) case "namespace": out.Namespace = string(in.String()) + case "lockID": + out.LockID = uint32(in.Uint32()) case "idMappingsOptions": easyjson1dbef17bDecodeGithubComContainersLibpodVendorGithubComContainersStorage(in, &out.IDMappings) case "rootfsImageID": @@ -1778,6 +1780,16 @@ func easyjson1dbef17bEncodeGithubComContainersLibpodLibpod2(out *jwriter.Writer, } out.String(string(in.Namespace)) } + { + const prefix string = ",\"lockID\":" + if first { + first = false + out.RawString(prefix[1:]) + } else { + out.RawString(prefix) + } + out.Uint32(uint32(in.LockID)) + } if true { const prefix string = ",\"idMappingsOptions\":" if first { diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 6c027f59a..856313b9d 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -401,7 +401,10 @@ func resetState(state *containerState) error { return nil } -// Refresh refreshes the container's state after a restart +// Refresh refreshes the container's state after a restart. +// Refresh cannot perform any operations that would lock another container. +// We cannot guarantee any other container has a valid lock at the time it is +// running. func (c *Container) refresh() error { // Don't need a full sync, but we do need to update from the database to // pick up potentially-missing container state @@ -447,6 +450,14 @@ func (c *Container) refresh() error { c.state.DestinationRunDir = filepath.Join(c.state.UserNSRoot, "rundir") } + // We need to pick up a new lock + lock, err := c.runtime.lockManager.AllocateLock() + if err != nil { + return errors.Wrapf(err, "error acquiring lock for container %s", c.ID()) + } + c.lock = lock + c.config.LockID = c.lock.ID() + if err := c.save(); err != nil { return errors.Wrapf(err, "error refreshing state for container %s", c.ID()) } diff --git a/libpod/runtime.go b/libpod/runtime.go index 238a7a9db..7c665bbc2 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -830,19 +830,22 @@ func (r *Runtime) refresh(alivePath string) error { if err != nil { return errors.Wrapf(err, "error retrieving all pods from state") } + // No locks are taken during pod and container refresh. + // Furthermore, the pod and container refresh() functions are not + // allowed to take locks themselves. + // We cannot assume that any pod or container has a valid lock until + // after this function has returned. + // The runtime alive lock should suffice to provide mutual exclusion + // until this has run. for _, ctr := range ctrs { - ctr.lock.Lock() if err := ctr.refresh(); err != nil { logrus.Errorf("Error refreshing container %s: %v", ctr.ID(), err) } - ctr.lock.Unlock() } for _, pod := range pods { - pod.lock.Lock() if err := pod.refresh(); err != nil { logrus.Errorf("Error refreshing pod %s: %v", pod.ID(), err) } - pod.lock.Unlock() } // Create a file indicating the runtime is alive and ready diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index a9095c6d1..eb78e7d7d 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -9,7 +9,6 @@ import ( "time" "github.com/containers/libpod/pkg/rootless" - "github.com/containers/storage" "github.com/containers/storage/pkg/stringid" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" @@ -61,15 +60,6 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. ctr.state.BindMounts = make(map[string]string) - // Path our lock file will reside at - lockPath := filepath.Join(r.lockDir, ctr.config.ID) - // Grab a lockfile at the given path - lock, err := storage.GetLockfile(lockPath) - if err != nil { - return nil, errors.Wrapf(err, "error creating lockfile for new container") - } - ctr.lock = lock - ctr.config.StopTimeout = CtrRemoveTimeout // Set namespace based on current runtime namespace @@ -85,6 +75,14 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. } } + // Allocate a lock for the container + lock, err := r.lockManager.AllocateLock() + if err != nil { + return nil, errors.Wrapf(err, "error allocating lock for new container") + } + ctr.lock = lock + ctr.config.LockID = c.lock.ID() + ctr.valid = true ctr.state.State = ContainerStateConfigured @@ -379,6 +377,15 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) } } + // Deallocate the container's lock + if err := c.lock.Free(); err != nil { + if cleanupErr == nil { + cleanupErr = err + } else { + logrus.Errorf("free container lock: %v", err) + } + } + return cleanupErr } diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index 3d6fad52f..95246449a 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -239,6 +239,11 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool) return err } } + + // Free the container's lock + if err := ctr.lock.Free(); err != nil { + return err + } } // Remove containers from the state |