From deea99442453eb890d63ccd223c93029b3e930f7 Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Mon, 23 Mar 2015 12:07:01 +0100 Subject: [PATCH] lock: convert DirLock to handle also locks on regular files. This patch renames DirLock to FileLock (where a file can be a regular file or a directory), adds a new LockType type to specify if the lock must be on a directory or on a regular file and checks if the file is valid for the requested type. --- cas/db.go | 4 +- .../ipam/static/backend/disk/backend.go | 4 +- pkg/lock/dir.go | 114 +++++++++++------- pkg/lock/dir_test.go | 27 +++-- rkt/containers.go | 20 +-- rkt/containers_test.go | 2 +- 6 files changed, 97 insertions(+), 74 deletions(-) diff --git a/cas/db.go b/cas/db.go index f32ee88..d62d476 100644 --- a/cas/db.go +++ b/cas/db.go @@ -31,7 +31,7 @@ const ( type DB struct { dbdir string - lock *lock.DirLock + lock *lock.FileLock sqldb *sql.DB } @@ -47,7 +47,7 @@ func (db *DB) Open() error { if db.lock != nil { panic("cas db lock already gained") } - dl, err := lock.ExclusiveLock(db.dbdir) + dl, err := lock.ExclusiveLock(db.dbdir, lock.Dir) if err != nil { return err } diff --git a/networking/ipam/static/backend/disk/backend.go b/networking/ipam/static/backend/disk/backend.go index 726e970..c945fc4 100644 --- a/networking/ipam/static/backend/disk/backend.go +++ b/networking/ipam/static/backend/disk/backend.go @@ -12,7 +12,7 @@ import ( var defaultDataDir = "/var/lib/rkt/networks" type Store struct { - lock.DirLock + lock.FileLock dataDir string } @@ -22,7 +22,7 @@ func New(network string) (*Store, error) { return nil, err } - lk, err := lock.NewLock(dir) + lk, err := lock.NewLock(dir, lock.Dir) if err != nil { return nil, err } diff --git a/pkg/lock/dir.go b/pkg/lock/dir.go index 5129ffa..265a276 100644 --- a/pkg/lock/dir.go +++ b/pkg/lock/dir.go @@ -13,7 +13,7 @@ // limitations under the License. // Package lock implements simple locking primitives on a -// directory using flock +// regular file or directory using flock package lock import ( @@ -22,22 +22,30 @@ import ( ) var ( - ErrLocked = errors.New("directory already locked") - ErrNotExist = errors.New("directory does not exist") + ErrLocked = errors.New("file already locked") + ErrNotExist = errors.New("file does not exist") ErrPermission = errors.New("permission denied") + ErrNotRegular = errors.New("not a regular file") ) -// DirLock represents a lock on a directory -type DirLock struct { - dir string - fd int +// FileLock represents a lock on a regular file or a directory +type FileLock struct { + path string + fd int } -// TryExclusiveLock takes an exclusive lock on a directory without blocking. -// This is idempotent when the DirLock already represents an exclusive lock, +type LockType int + +const ( + Dir LockType = iota + RegFile +) + +// TryExclusiveLock takes an exclusive lock without blocking. +// This is idempotent when the Lock already represents an exclusive lock, // and tries promote a shared lock to exclusive atomically. -// It will return ErrLocked if any lock is already held on the directory. -func (l *DirLock) TryExclusiveLock() error { +// It will return ErrLocked if any lock is already held. +func (l *FileLock) TryExclusiveLock() error { err := syscall.Flock(l.fd, syscall.LOCK_EX|syscall.LOCK_NB) if err == syscall.EWOULDBLOCK { err = ErrLocked @@ -45,10 +53,10 @@ func (l *DirLock) TryExclusiveLock() error { return err } -// TryExclusiveLock takes an exclusive lock on a directory without blocking. -// It will return ErrLocked if any lock is already held on the directory. -func TryExclusiveLock(dir string) (*DirLock, error) { - l, err := NewLock(dir) +// TryExclusiveLock takes an exclusive lock on a file/directory without blocking. +// It will return ErrLocked if any lock is already held on the file/directory. +func TryExclusiveLock(path string, lockType LockType) (*FileLock, error) { + l, err := NewLock(path, lockType) if err != nil { return nil, err } @@ -59,18 +67,18 @@ func TryExclusiveLock(dir string) (*DirLock, error) { return l, err } -// ExclusiveLock takes an exclusive lock on a directory. -// This is idempotent when the DirLock already represents an exclusive lock, +// ExclusiveLock takes an exclusive lock. +// This is idempotent when the Lock already represents an exclusive lock, // and promotes a shared lock to exclusive atomically. -// It will block if an exclusive lock is already held on the directory. -func (l *DirLock) ExclusiveLock() error { +// It will block if an exclusive lock is already held. +func (l *FileLock) ExclusiveLock() error { return syscall.Flock(l.fd, syscall.LOCK_EX) } -// ExclusiveLock takes an exclusive lock on a directory. -// It will block if an exclusive lock is already held on the directory. -func ExclusiveLock(dir string) (*DirLock, error) { - l, err := NewLock(dir) +// ExclusiveLock takes an exclusive lock on a file/directory. +// It will block if an exclusive lock is already held on the file/directory. +func ExclusiveLock(path string, lockType LockType) (*FileLock, error) { + l, err := NewLock(path, lockType) if err == nil { err = l.ExclusiveLock() } @@ -80,11 +88,11 @@ func ExclusiveLock(dir string) (*DirLock, error) { return l, nil } -// TrySharedLock takes a co-operative (shared) lock on a directory without blocking. -// This is idempotent when the DirLock already represents a shared lock, +// TrySharedLock takes a co-operative (shared) lock without blocking. +// This is idempotent when the Lock already represents a shared lock, // and tries demote an exclusive lock to shared atomically. -// It will return ErrLocked if an exclusive lock already exists on the directory. -func (l *DirLock) TrySharedLock() error { +// It will return ErrLocked if an exclusive lock already exists. +func (l *FileLock) TrySharedLock() error { err := syscall.Flock(l.fd, syscall.LOCK_SH|syscall.LOCK_NB) if err == syscall.EWOULDBLOCK { err = ErrLocked @@ -92,10 +100,10 @@ func (l *DirLock) TrySharedLock() error { return err } -// TrySharedLock takes a co-operative (shared) lock on a directory without blocking. -// It will return ErrLocked if an exclusive lock already exists on the directory. -func TrySharedLock(dir string) (*DirLock, error) { - l, err := NewLock(dir) +// TrySharedLock takes a co-operative (shared) lock on a file/directory without blocking. +// It will return ErrLocked if an exclusive lock already exists on the file/directory. +func TrySharedLock(path string, lockType LockType) (*FileLock, error) { + l, err := NewLock(path, lockType) if err != nil { return nil, err } @@ -106,18 +114,18 @@ func TrySharedLock(dir string) (*DirLock, error) { return l, nil } -// SharedLock takes a co-operative (shared) lock on a directory. -// This is idempotent when the DirLock already represents a shared lock, +// SharedLock takes a co-operative (shared) lock on. +// This is idempotent when the Lock already represents a shared lock, // and demotes an exclusive lock to shared atomically. -// It will block if an exclusive lock is already held on the directory. -func (l *DirLock) SharedLock() error { +// It will block if an exclusive lock is already held. +func (l *FileLock) SharedLock() error { return syscall.Flock(l.fd, syscall.LOCK_SH) } -// SharedLock takes a co-operative (shared) lock on a directory. -// It will block if an exclusive lock is already held on the directory. -func SharedLock(dir string) (*DirLock, error) { - l, err := NewLock(dir) +// SharedLock takes a co-operative (shared) lock on a file/directory. +// It will block if an exclusive lock is already held on the file/directory. +func SharedLock(path string, lockType LockType) (*FileLock, error) { + l, err := NewLock(path, lockType) if err != nil { return nil, err } @@ -129,12 +137,12 @@ func SharedLock(dir string) (*DirLock, error) { } // Unlock unlocks the lock -func (l *DirLock) Unlock() error { +func (l *FileLock) Unlock() error { return syscall.Flock(l.fd, syscall.LOCK_UN) } // Fd returns the lock's file descriptor, or an error if the lock is closed -func (l *DirLock) Fd() (int, error) { +func (l *FileLock) Fd() (int, error) { var err error if l.fd == -1 { err = errors.New("lock closed") @@ -143,17 +151,21 @@ func (l *DirLock) Fd() (int, error) { } // Close closes the lock which implicitly unlocks it as well -func (l *DirLock) Close() error { +func (l *FileLock) Close() error { fd := l.fd l.fd = -1 return syscall.Close(fd) } -// NewLock opens a new lock on a directory without acquisition -func NewLock(dir string) (*DirLock, error) { - l := &DirLock{dir: dir, fd: -1} +// NewLock opens a new lock on a file without acquisition +func NewLock(path string, lockType LockType) (*FileLock, error) { + l := &FileLock{path: path, fd: -1} - lfd, err := syscall.Open(l.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_CLOEXEC, 0) + mode := syscall.O_RDONLY | syscall.O_CLOEXEC + if lockType == Dir { + mode |= syscall.O_DIRECTORY + } + lfd, err := syscall.Open(l.path, mode, 0) if err != nil { if err == syscall.ENOENT { err = ErrNotExist @@ -164,5 +176,15 @@ func NewLock(dir string) (*DirLock, error) { } l.fd = lfd + var stat syscall.Stat_t + err = syscall.Fstat(lfd, &stat) + if err != nil { + return nil, err + } + // Check if the file is a regular file + if lockType == RegFile && !(stat.Mode&syscall.S_IFMT == syscall.S_IFREG) { + return nil, ErrNotRegular + } + return l, nil } diff --git a/pkg/lock/dir_test.go b/pkg/lock/dir_test.go index e0baf81..fb86626 100644 --- a/pkg/lock/dir_test.go +++ b/pkg/lock/dir_test.go @@ -28,10 +28,11 @@ func TestNewLock(t *testing.T) { defer os.Remove(f.Name()) f.Close() - l, err := NewLock(f.Name()) - if err == nil || l != nil { - t.Fatal("expected error creating lock on file") + l, err := NewLock(f.Name(), RegFile) + if err != nil { + t.Fatalf("error creating NewFileLock: %v", err) } + l.Close() d, err := ioutil.TempDir("", "") if err != nil { @@ -39,7 +40,7 @@ func TestNewLock(t *testing.T) { } defer os.Remove(d) - l, err = NewLock(d) + l, err = NewLock(d, Dir) if err != nil { t.Fatalf("error creating NewLock: %v", err) } @@ -53,7 +54,7 @@ func TestNewLock(t *testing.T) { t.Fatalf("error removing tmpdir: %v", err) } - l, err = NewLock(d) + l, err = NewLock(d, Dir) if err == nil { t.Fatalf("expected error creating lock on nonexistent path") } @@ -67,7 +68,7 @@ func TestExclusiveLock(t *testing.T) { defer os.Remove(dir) // Set up the initial exclusive lock - l, err := ExclusiveLock(dir) + l, err := ExclusiveLock(dir, Dir) if err != nil { t.Fatalf("error creating lock: %v", err) } @@ -79,7 +80,7 @@ func TestExclusiveLock(t *testing.T) { } // Now try another exclusive lock, should fail - _, err = TryExclusiveLock(dir) + _, err = TryExclusiveLock(dir, Dir) if err == nil { t.Fatalf("expected err trying exclusive lock") } @@ -91,7 +92,7 @@ func TestExclusiveLock(t *testing.T) { } // Now another exclusive lock should succeed - _, err = TryExclusiveLock(dir) + _, err = TryExclusiveLock(dir, Dir) if err != nil { t.Fatalf("error creating lock: %v", err) } @@ -105,7 +106,7 @@ func TestSharedLock(t *testing.T) { defer os.Remove(dir) // Set up the initial shared lock - l1, err := SharedLock(dir) + l1, err := SharedLock(dir, Dir) if err != nil { t.Fatalf("error creating new shared lock: %v", err) } @@ -116,17 +117,17 @@ func TestSharedLock(t *testing.T) { } // Subsequent shared locks should succeed - l2, err := TrySharedLock(dir) + l2, err := TrySharedLock(dir, Dir) if err != nil { t.Fatalf("error creating shared lock: %v", err) } - l3, err := TrySharedLock(dir) + l3, err := TrySharedLock(dir, Dir) if err != nil { t.Fatalf("error creating shared lock: %v", err) } // But an exclusive lock should fail - _, err = TryExclusiveLock(dir) + _, err = TryExclusiveLock(dir, Dir) if err == nil { t.Fatal("expected exclusive lock to fail") } @@ -148,7 +149,7 @@ func TestSharedLock(t *testing.T) { } // Now try an exclusive lock, should succeed - _, err = TryExclusiveLock(dir) + _, err = TryExclusiveLock(dir, Dir) if err != nil { t.Fatalf("error creating lock: %v", err) } diff --git a/rkt/containers.go b/rkt/containers.go index 7c23642..9f142c4 100644 --- a/rkt/containers.go +++ b/rkt/containers.go @@ -37,7 +37,7 @@ import ( // see Documentation/container-lifecycle.md for some explanation type container struct { - *lock.DirLock + *lock.FileLock uuid *types.UUID createdByMe bool // true if we're the creator of this container (only the creator can xToPrepare or xToRun directly from preparing) nets []netinfo.NetInfo // list of networks (name, IP, iface) this container is using @@ -150,7 +150,7 @@ func newContainer() (*container, error) { return nil, err } - c.DirLock, err = lock.NewLock(c.embryoPath()) + c.FileLock, err = lock.NewLock(c.embryoPath(), lock.Dir) if err != nil { os.Remove(c.embryoPath()) return nil, err @@ -184,27 +184,27 @@ func getContainer(uuid string) (*container, error) { c.uuid = u // we try open the container in all possible directories, in the same order the states occur - l, err := lock.NewLock(c.embryoPath()) + l, err := lock.NewLock(c.embryoPath(), lock.Dir) if err == nil { c.isEmbryo = true } else if err == lock.ErrNotExist { - l, err = lock.NewLock(c.preparePath()) + l, err = lock.NewLock(c.preparePath(), lock.Dir) if err == nil { // treat as aborted prepare until lock is tested c.isAbortedPrepare = true } else if err == lock.ErrNotExist { - l, err = lock.NewLock(c.preparedPath()) + l, err = lock.NewLock(c.preparedPath(), lock.Dir) if err == nil { c.isPrepared = true } else if err == lock.ErrNotExist { - l, err = lock.NewLock(c.runPath()) + l, err = lock.NewLock(c.runPath(), lock.Dir) if err == nil { // treat as exited until lock is tested c.isExited = true } else if err == lock.ErrNotExist { - l, err = lock.NewLock(c.exitedGarbagePath()) + l, err = lock.NewLock(c.exitedGarbagePath(), lock.Dir) if err == lock.ErrNotExist { - l, err = lock.NewLock(c.garbagePath()) + l, err = lock.NewLock(c.garbagePath(), lock.Dir) if err == nil { c.isGarbage = true } else { @@ -250,7 +250,7 @@ func getContainer(uuid string) (*container, error) { } } - c.DirLock = l + c.FileLock = l if c.isRunning() { cfd, err := c.Fd() @@ -504,7 +504,7 @@ func listContainersFromDir(cdir string) ([]string, error) { } // refreshState() updates the cached members of c to reflect current reality -// assumes c.DirLock is currently unlocked, and always returns with it unlocked. +// assumes c.FileLock is currently unlocked, and always returns with it unlocked. func (c *container) refreshState() error { // TODO(vc): this overlaps substantially with newContainer(), could probably unify. c.isEmbryo = false diff --git a/rkt/containers_test.go b/rkt/containers_test.go index 00ff222..020ca4f 100644 --- a/rkt/containers_test.go +++ b/rkt/containers_test.go @@ -133,7 +133,7 @@ func TestWalkContainers(t *testing.T) { } if !ct.exited || ct.deleting { // acquire lock to simulate running and deleting containers - l, err := lock.ExclusiveLock(cp) + l, err := lock.ExclusiveLock(cp, lock.Dir) if err != nil { t.Fatalf("error locking container: %v", err) }