Merge pull request containers#2350 from mheon/lock_renumber

Add lock renumbering

cmd/podman/cliconfig/config.go

@@ -548,3 +548,7 @@ type SystemPruneValues struct {
 	Force  bool
 	Volume bool
 }
+
+type SystemRenumberValues struct {
+	PodmanCommand
+}

cmd/podman/commands.go

@@ -131,5 +131,6 @@ func getTrustSubCommands() []*cobra.Command {
 func getSystemSubCommands() []*cobra.Command {
 	return []*cobra.Command{
 		_pruneSystemCommand,
+		_renumberCommand,
 	}
 }

cmd/podman/libpodruntime/runtime.go

@@ -8,8 +8,17 @@ import (
 	"github.com/pkg/errors"
 )
 
+// GetRuntimeRenumber gets a libpod runtime that will perform a lock renumber
+func GetRuntimeRenumber(c *cliconfig.PodmanCommand) (*libpod.Runtime, error) {
+	return getRuntime(c, true)
+}
+
 // GetRuntime generates a new libpod runtime configured by command line options
 func GetRuntime(c *cliconfig.PodmanCommand) (*libpod.Runtime, error) {
+	return getRuntime(c, false)
+}
+
+func getRuntime(c *cliconfig.PodmanCommand, renumber bool) (*libpod.Runtime, error) {
 	options := []libpod.RuntimeOption{}
 
 	storageOpts, volumePath, err := util.GetDefaultStoreOptions()

cmd/podman/system_renumber.go

@@ -0,0 +1,49 @@
+package main
+
+import (
+	"github.com/containers/libpod/cmd/podman/cliconfig"
+	"github.com/containers/libpod/cmd/podman/libpodruntime"
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+)
+
+var (
+	renumberCommand     cliconfig.SystemRenumberValues
+	renumberDescription = `
+        podman system renumber
+
+        Migrate lock numbers to handle a change in maximum number of locks.
+        Mandatory after the number of locks in libpod.conf is changed.
+`
+
+	_renumberCommand = &cobra.Command{
+		Use:   "renumber",
+		Short: "Migrate lock numbers",
+		Long:  renumberDescription,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			renumberCommand.InputArgs = args
+			renumberCommand.GlobalFlags = MainGlobalOpts
+			return renumberCmd(&renumberCommand)
+		},
+	}
+)
+
+func init() {
+	renumberCommand.Command = _renumberCommand
+	renumberCommand.SetUsageTemplate(UsageTemplate())
+}
+
+func renumberCmd(c *cliconfig.SystemRenumberValues) error {
+	// We need to pass one extra option to NewRuntime.
+	// This will inform the OCI runtime to start a renumber.
+	// That's controlled by the last argument to GetRuntime.
+	r, err := libpodruntime.GetRuntimeRenumber(&c.PodmanCommand)
+	if err != nil {
+		return errors.Wrapf(err, "error renumbering locks")
+	}
+	if err := r.Shutdown(false); err != nil {
+		return err
+	}
+
+	return nil
+}

docs/podman-system-renumber.1.md

@@ -0,0 +1,29 @@
+% podman-system-renumber(1) podman
+
+## NAME
+podman\-system\-renumber - Renumber container locks
+
+## SYNOPSIS
+** podman system renumber**
+
+## DESCRIPTION
+** podman system renumber** renumbers locks used by containers and pods.
+
+Each Podman container and pod is allocated a lock at creation time, up to a maximum number controlled by the **num_locks** parameter in **libpod.conf**.
+
+When all available locks are exhausted, no further containers and pods can be created until some existing containers and pods are removed. This can be avoided by increasing the number of locks available via modifying **libpod.conf** and subsequently running **podman system renumber** to prepare the new locks (and reallocate lock numbers to fit the new struct).
+
+**podman system renumber** must be called after any changes to **num_locks** - failure to do so will result in errors starting Podman as the number of locks available conflicts with the configured number of locks.
+
+**podman system renumber** can also be used to migrate 1.0 and earlier versions of Podman, which used a different locking scheme, to the new locking model. It is not strictly required to do this, but it is highly recommended to do so as deadlocks can occur otherwise.
+
+If possible, avoid calling **podman system renumber** while there are other Podman processes running.
+
+## SYNOPSIS
+**podman system renumber**
+
+## SEE ALSO
+`podman(1)`, `libpod.conf(5)`
+
+# HISTORY
+February 2019, Originally compiled by Matt Heon (mheon at redhat dot com)

libpod/boltdb_state.go

@@ -783,6 +783,94 @@ func (s *BoltState) AllContainers() ([]*Container, error) {
 	return ctrs, nil
 }
 
+// RewriteContainerConfig rewrites a container's configuration.
+// WARNING: This function is DANGEROUS. Do not use without reading the full
+// comment on this function in state.go.
+func (s *BoltState) RewriteContainerConfig(ctr *Container, newCfg *ContainerConfig) error {
+	if !s.valid {
+		return ErrDBClosed
+	}
+
+	if !ctr.valid {
+		return ErrCtrRemoved
+	}
+
+	newCfgJSON, err := json.Marshal(newCfg)
+	if err != nil {
+		return errors.Wrapf(err, "error marshalling new configuration JSON for container %s", ctr.ID())
+	}
+
+	db, err := s.getDBCon()
+	if err != nil {
+		return err
+	}
+	defer s.closeDBCon(db)
+
+	err = db.Update(func(tx *bolt.Tx) error {
+		ctrBkt, err := getCtrBucket(tx)
+		if err != nil {
+			return err
+		}
+
+		ctrDB := ctrBkt.Bucket([]byte(ctr.ID()))
+		if ctrDB == nil {
+			ctr.valid = false
+			return errors.Wrapf(ErrNoSuchCtr, "no container with ID %s found in DB", ctr.ID())
+		}
+
+		if err := ctrDB.Put(configKey, newCfgJSON); err != nil {
+			return errors.Wrapf(err, "error updating container %s config JSON", ctr.ID())
+		}
+
+		return nil
+	})
+	return err
+}
+
+// RewritePodConfig rewrites a pod's configuration.
+// WARNING: This function is DANGEROUS. Do not use without reading the full
+// comment on this function in state.go.
+func (s *BoltState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error {
+	if !s.valid {
+		return ErrDBClosed
+	}
+
+	if !pod.valid {
+		return ErrPodRemoved
+	}
+
+	newCfgJSON, err := json.Marshal(newCfg)
+	if err != nil {
+		return errors.Wrapf(err, "error marshalling new configuration JSON for container %s", pod.ID())
+	}
+
+	db, err := s.getDBCon()
+	if err != nil {
+		return err
+	}
+	defer s.closeDBCon(db)
+
+	err = db.Update(func(tx *bolt.Tx) error {
+		podBkt, err := getPodBucket(tx)
+		if err != nil {
+			return err
+		}
+
+		podDB := podBkt.Bucket([]byte(pod.ID()))
+		if podDB == nil {
+			pod.valid = false
+			return errors.Wrapf(ErrNoSuchPod, "no pod with ID %s found in DB", pod.ID())
+		}
+
+		if err := podDB.Put(configKey, newCfgJSON); err != nil {
+			return errors.Wrapf(err, "error updating pod %s config JSON", pod.ID())
+		}
+
+		return nil
+	})
+	return err
+}
+
 // Pod retrieves a pod given its full ID
 func (s *BoltState) Pod(id string) (*Pod, error) {
 	if id == "" {
@@ -1281,10 +1369,6 @@ func (s *BoltState) RemoveVolume(volume *Volume) error {
 		return ErrDBClosed
 	}
 
-	if !volume.valid {
-		return ErrVolumeRemoved
-	}
-
 	volName := []byte(volume.Name())
 
 	db, err := s.getDBCon()

libpod/boltdb_state_internal.go

@@ -348,13 +348,6 @@ func (s *BoltState) getVolumeFromDB(name []byte, volume *Volume, volBkt *bolt.Bu
 		return errors.Wrapf(err, "error unmarshalling volume %s config from DB", string(name))
 	}
 
-	// Get the lock
-	lock, err := s.runtime.lockManager.RetrieveLock(volume.config.LockID)
-	if err != nil {
-		return errors.Wrapf(err, "error retrieving lockfile for volume %s", string(name))
-	}
-	volume.lock = lock
-
 	volume.runtime = s.runtime
 	volume.valid = true
 

libpod/in_memory_state.go

@@ -378,6 +378,58 @@ func (s *InMemoryState) ContainerInUse(ctr *Container) ([]string, error) {
 	return arr, nil
 }
 
+// AllContainers retrieves all containers from the state
+func (s *InMemoryState) AllContainers() ([]*Container, error) {
+	ctrs := make([]*Container, 0, len(s.containers))
+	for _, ctr := range s.containers {
+		if s.namespace == "" || ctr.config.Namespace == s.namespace {
+			ctrs = append(ctrs, ctr)
+		}
+	}
+
+	return ctrs, nil
+}
+
+// RewriteContainerConfig rewrites a container's configuration.
+// This function is DANGEROUS, even with an in-memory state.
+// Please read the full comment on it in state.go before using it.
+func (s *InMemoryState) RewriteContainerConfig(ctr *Container, newCfg *ContainerConfig) error {
+	if !ctr.valid {
+		return ErrCtrRemoved
+	}
+
+	// If the container does not exist, return error
+	stateCtr, ok := s.containers[ctr.ID()]
+	if !ok {
+		ctr.valid = false
+		return errors.Wrapf(ErrNoSuchCtr, "container with ID %s not found in state", ctr.ID())
+	}
+
+	stateCtr.config = newCfg
+
+	return nil
+}
+
+// RewritePodConfig rewrites a pod's configuration.
+// This function is DANGEROUS, even with in-memory state.
+// Please read the full comment on it in state.go before using it.
+func (s *InMemoryState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error {
+	if !pod.valid {
+		return ErrPodRemoved
+	}
+
+	// If the pod does not exist, return error
+	statePod, ok := s.pods[pod.ID()]
+	if !ok {
+		pod.valid = false
+		return errors.Wrapf(ErrNoSuchPod, "pod with ID %s not found in state", pod.ID())
+	}
+
+	statePod.config = newCfg
+
+	return nil
+}
+
 // Volume retrieves a volume from its full name
 func (s *InMemoryState) Volume(name string) (*Volume, error) {
 	if name == "" {
@@ -486,18 +538,6 @@ func (s *InMemoryState) AllVolumes() ([]*Volume, error) {
 	return allVols, nil
 }
 
-// AllContainers retrieves all containers from the state
-func (s *InMemoryState) AllContainers() ([]*Container, error) {
-	ctrs := make([]*Container, 0, len(s.containers))
-	for _, ctr := range s.containers {
-		if s.namespace == "" || ctr.config.Namespace == s.namespace {
-			ctrs = append(ctrs, ctr)
-		}
-	}
-
-	return ctrs, nil
-}
-
 // Pod retrieves a pod from the state from its full ID
 func (s *InMemoryState) Pod(id string) (*Pod, error) {
 	if id == "" {

libpod/lock/in_memory_locks.go

@@ -89,3 +89,14 @@ func (m *InMemoryManager) RetrieveLock(id uint32) (Locker, error) {
 
 	return m.locks[id], nil
 }
+
+// FreeAllLocks frees all locks.
+// This function is DANGEROUS. Please read the full comment in locks.go before
+// trying to use it.
+func (m *InMemoryManager) FreeAllLocks() error {
+	for _, lock := range m.locks {
+		lock.allocated = false
+	}
+
+	return nil
+}

libpod/lock/lock.go

@@ -24,6 +24,20 @@ type Manager interface {
 	// The underlying lock MUST be the same as another other lock with the
 	// same UUID.
 	RetrieveLock(id uint32) (Locker, error)
+	// PLEASE READ FULL DESCRIPTION BEFORE USING.
+	// FreeAllLocks frees all allocated locks, in preparation for lock
+	// reallocation.
+	// As this deallocates all presently-held locks, this can be very
+	// dangerous - if there are other processes running that might be
+	// attempting to allocate new locks and free existing locks, we may
+	// encounter races leading to an inconsistent state.
+	// (This is in addition to the fact that FreeAllLocks instantly makes
+	// the state inconsistent simply by using it, and requires a full
+	// lock renumbering to restore consistency!).
+	// In short, this should only be used as part of unit tests, or lock
+	// renumbering, where reasonable guarantees about other processes can be
+	// made.
+	FreeAllLocks() error
 }
 
 // Locker is similar to sync.Locker, but provides a method for freeing the lock