Remove SQL state locking and rely on sqlite locking

Also renames some parameters from locksDir -> lockDir for
consistency.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

5 files changed, 19 insertions, 45 deletions

diff --git a/libpod/container.go b/libpod/container.go
index f93b214b9..4f8c148d1 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -338,7 +338,7 @@ func (c *Container) syncContainer() error {
 }
 
 // Make a new container
-func newContainer(rspec *spec.Spec, logDir string) (*Container, error) {
+func newContainer(rspec *spec.Spec, lockDir string) (*Container, error) {
 	if rspec == nil {
 		return nil, errors.Wrapf(ErrInvalidArg, "must provide a valid runtime spec to create container")
 	}
@@ -356,7 +356,7 @@ func newContainer(rspec *spec.Spec, logDir string) (*Container, error) {
 	ctr.config.CreatedTime = time.Now()
 
 	// Path our lock file will reside at
-	lockPath := filepath.Join(logDir, ctr.config.ID)
+	lockPath := filepath.Join(lockDir, ctr.config.ID)
 	// Ensure there is no conflict - file does not exist
 	_, err := os.Stat(lockPath)
 	if err == nil || !os.IsNotExist(err) {
diff --git a/libpod/runtime.go b/libpod/runtime.go
index 616641fc1..ac7db51ca 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -25,7 +25,7 @@ type Runtime struct {
 	storageService *storageService
 	imageContext   *types.SystemContext
 	ociRuntime     *OCIRuntime
-	locksDir       string
+	lockDir        string
 	valid          bool
 	lock           sync.RWMutex
 }
@@ -138,15 +138,15 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) {
 	}
 
 	// Make a directory to hold container lockfiles
-	lockPath := filepath.Join(runtime.config.StaticDir, "lock")
-	if err := os.MkdirAll(lockPath, 0755); err != nil {
+	lockDir := filepath.Join(runtime.config.StaticDir, "lock")
+	if err := os.MkdirAll(lockDir, 0755); err != nil {
 		// The directory is allowed to exist
 		if !os.IsExist(err) {
 			return nil, errors.Wrapf(err, "error creating runtime lockfiles directory %s",
-				lockPath)
+				lockDir)
 		}
 	}
-	runtime.locksDir = lockPath
+	runtime.lockDir = lockDir
 
 	// Make the per-boot files directory if it does not exist
 	if err := os.MkdirAll(runtime.config.TmpDir, 0755); err != nil {
@@ -166,7 +166,6 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) {
 		runtime.state = state
 	} else {
 		dbPath := filepath.Join(runtime.config.StaticDir, "state.sql")
-		lockPath := filepath.Join(runtime.config.TmpDir, "state.lck")
 		specsDir := filepath.Join(runtime.config.StaticDir, "ocispec")
 
 		// Make a directory to hold JSON versions of container OCI specs
@@ -178,7 +177,7 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) {
 			}
 		}
 
-		state, err := NewSQLState(dbPath, lockPath, specsDir, runtime.locksDir, runtime)
+		state, err := NewSQLState(dbPath, specsDir, runtime.lockDir, runtime)
 		if err != nil {
 			return nil, err
 		}
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 793bb3e65..f314b302f 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -30,7 +30,7 @@ func (r *Runtime) NewContainer(spec *spec.Spec, options ...CtrCreateOption) (c *
 		return nil, ErrRuntimeStopped
 	}
 
-	ctr, err := newContainer(spec, r.locksDir)
+	ctr, err := newContainer(spec, r.lockDir)
 	if err != nil {
 		return nil, err
 	}
diff --git a/libpod/sql_state.go b/libpod/sql_state.go
index 8640ca9c3..cdf8dd707 100644
--- a/libpod/sql_state.go
+++ b/libpod/sql_state.go
@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"os"
 
-	"github.com/containers/storage"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
@@ -22,25 +21,17 @@ const DBSchema = 2
 type SQLState struct {
 	db       *sql.DB
 	specsDir string
-	locksDir string
+	lockDir  string
 	runtime  *Runtime
-	lock     storage.Locker
 	valid    bool
 }
 
 // NewSQLState initializes a SQL-backed state, created the database if necessary
-func NewSQLState(dbPath, lockPath, specsDir, locksDir string, runtime *Runtime) (State, error) {
+func NewSQLState(dbPath, specsDir, lockDir string, runtime *Runtime) (State, error) {
 	state := new(SQLState)
 
 	state.runtime = runtime
 
-	// Make our lock file
-	lock, err := storage.GetLockfile(lockPath)
-	if err != nil {
-		return nil, errors.Wrapf(err, "error creating lockfile for state")
-	}
-	state.lock = lock
-
 	// Make the directory that will hold JSON copies of container runtime specs
 	if err := os.MkdirAll(specsDir, 0750); err != nil {
 		// The directory is allowed to exist
@@ -51,17 +42,13 @@ func NewSQLState(dbPath, lockPath, specsDir, locksDir string, runtime *Runtime)
 	state.specsDir = specsDir
 
 	// Make the directory that will hold container lockfiles
-	if err := os.MkdirAll(locksDir, 0750); err != nil {
+	if err := os.MkdirAll(lockDir, 0750); err != nil {
 		// The directory is allowed to exist
 		if !os.IsExist(err) {
-			return nil, errors.Wrapf(err, "error creating lockfiles dir %s", locksDir)
+			return nil, errors.Wrapf(err, "error creating lockfiles dir %s", lockDir)
 		}
 	}
-	state.locksDir = locksDir
-
-	// Acquire the lock while we open the database and perform initial setup
-	state.lock.Lock()
-	defer state.lock.Unlock()
+	state.lockDir = lockDir
 
 	// TODO add a separate temporary database for per-boot container
 	// state
@@ -97,9 +84,6 @@ func NewSQLState(dbPath, lockPath, specsDir, locksDir string, runtime *Runtime)
 
 // Close the state's database connection
 func (s *SQLState) Close() error {
-	s.lock.Lock()
-	defer s.lock.Unlock()
-
 	if !s.valid {
 		return ErrDBClosed
 	}
@@ -140,7 +124,7 @@ func (s *SQLState) Container(id string) (*Container, error) {
 
 	row := s.db.QueryRow(query, id)
 
-	ctr, err := ctrFromScannable(row, s.runtime, s.specsDir, s.locksDir)
+	ctr, err := ctrFromScannable(row, s.runtime, s.specsDir, s.lockDir)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error retrieving container %s from database", id)
 	}
@@ -187,7 +171,7 @@ func (s *SQLState) LookupContainer(idOrName string) (*Container, error) {
 		}
 
 		var err error
-		ctr, err = ctrFromScannable(rows, s.runtime, s.specsDir, s.locksDir)
+		ctr, err = ctrFromScannable(rows, s.runtime, s.specsDir, s.lockDir)
 		if err != nil {
 			return nil, errors.Wrapf(err, "error retrieving container %s from database", idOrName)
 		}
@@ -260,9 +244,6 @@ func (s *SQLState) AddContainer(ctr *Container) (err error) {
 		return errors.Wrapf(err, "error marshaling container %s labels to JSON", ctr.ID())
 	}
 
-	s.lock.Lock()
-	defer s.lock.Unlock()
-
 	tx, err := s.db.Begin()
 	if err != nil {
 		return errors.Wrapf(err, "error beginning database transaction")
@@ -441,9 +422,6 @@ func (s *SQLState) SaveContainer(ctr *Container) error {
                           Pid=?
                        WHERE Id=?;`
 
-	s.lock.Lock()
-	defer s.lock.Unlock()
-
 	if !s.valid {
 		return ErrDBClosed
 	}
@@ -501,9 +479,6 @@ func (s *SQLState) RemoveContainer(ctr *Container) error {
 		removeState = "DELETE FROM containerState WHERE ID=?;"
 	)
 
-	s.lock.Lock()
-	defer s.lock.Unlock()
-
 	if !s.valid {
 		return ErrDBClosed
 	}
@@ -586,7 +561,7 @@ func (s *SQLState) AllContainers() ([]*Container, error) {
 	containers := []*Container{}
 
 	for rows.Next() {
-		ctr, err := ctrFromScannable(rows, s.runtime, s.specsDir, s.locksDir)
+		ctr, err := ctrFromScannable(rows, s.runtime, s.specsDir, s.lockDir)
 		if err != nil {
 			return nil, err
 		}
diff --git a/libpod/sql_state_internal.go b/libpod/sql_state_internal.go
index e4af280ae..dfcd97c63 100644
--- a/libpod/sql_state_internal.go
+++ b/libpod/sql_state_internal.go
@@ -268,7 +268,7 @@ type scannable interface {
 }
 
 // Read a single container from a single row result in the database
-func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, locksDir string) (*Container, error) {
+func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, lockDir string) (*Container, error) {
 	var (
 		id                 string
 		name               string
@@ -387,7 +387,7 @@ func ctrFromScannable(row scannable, runtime *Runtime, specsDir string, locksDir
 	ctr.runtime = runtime
 
 	// Ensure the lockfile exists
-	lockPath := filepath.Join(locksDir, id)
+	lockPath := filepath.Join(lockDir, id)
 	_, err = os.Stat(lockPath)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error performing stat on container %s lockfile", id)