From 871c19a9030c68d86f723cbd3538d0f2352425c2 Mon Sep 17 00:00:00 2001 From: Gustav Paul Date: Fri, 9 Aug 2019 11:25:40 +0200 Subject: [PATCH] enable lockfile by default --- README.md | 16 +++++++++------- cmd/csilvm/csilvm.go | 4 ++-- pkg/lvm/lock.go | 13 +++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 80059ae0..6ab07bc2 100644 --- a/README.md +++ b/README.md @@ -186,16 +186,18 @@ It is expected that the CO will connect to the plugin through the unix socket an ### Locking -If the `-lockfile` parameter is specified or the `CSILVM_LOCKFILE_PATH` environment variable is set, -an exclusive `flock()` is held on that file whenever any lvm2 command-line utility is invoked. This prevents -multiple CSILVM processes from performing concurrent lvm operations that can -lead to deadlocks in the underlying lvm2 implementation. For example, +Any command-line invocations executed by the `csilvm` process first acquires a +lock on a lockfile. The path to the lockfile can be overridden using the +`-lockfile=` option or the `CSILVM_LOCKFILE_PATH` environment variable. The +locking behaviour can be disabled by setting the `-lockfile=` option to the +empty string. The purpose of locking around command-line invocations is to +prevent multiple `csilvm` processes from executing concurrent `lvm2` commands. +This works around deadlocks in lvm2. For example, https://jira.mesosphere.com/browse/DCOS_OSS-5434 and https://github.com/lvmteam/lvm2/issues/23. -By default no lock file is specified and no inter-process synchronization is -performed. You are encouraged to enable locking by setting the `-lockfile=` -option, for example `-lockfile=/run/csilvm.lock`. +By default the lock file is created at `/run/csilvm.lock` so it is assumed that +the `/run` directory exists and is writable by the `csilvm` process. ### Logging diff --git a/cmd/csilvm/csilvm.go b/cmd/csilvm/csilvm.go index 9dfe8e26..d848a1b0 100644 --- a/cmd/csilvm/csilvm.go +++ b/cmd/csilvm/csilvm.go @@ -59,7 +59,7 @@ func main() { var probeModulesF stringsFlag flag.Var(&probeModulesF, "probe-module", "Probe checks that the kernel module is loaded") nodeIDF := flag.String("node-id", "", "The node ID reported via the CSI Node gRPC service") - lockFilePathF := flag.String("lockfile", "", "The path to the lock file used to prevent concurrent lvm invocation by multiple csilvm instances") + lockFilePathF := flag.String("lockfile", "/run/csilvm.lock", "The path to the lock file used to prevent concurrent lvm invocation by multiple csilvm instances") // Metrics-related flags statsdUDPHostEnvVarF := flag.String("statsd-udp-host-env-var", "", "The name of the environment variable containing the host where a statsd service is listening for stats over UDP") statsdUDPPortEnvVarF := flag.String("statsd-udp-port-env-var", "", "The name of the environment variable containing the port where a statsd service is listening for stats over UDP") @@ -95,7 +95,7 @@ func main() { // Unlink the domain socket in case it is left lying around from a // previous run. err return is not really interesting because it is // normal for this to fail if the process is starting for the first time. - logger.Printf("Unlinking %s", sock) + logger.Printf("Unlinking socket file: %q", sock) syscall.Unlink(sock) // Setup socket listener lis, err := net.Listen("unix", sock) diff --git a/pkg/lvm/lock.go b/pkg/lvm/lock.go index 46a9018d..90a20ac6 100644 --- a/pkg/lvm/lock.go +++ b/pkg/lvm/lock.go @@ -1,6 +1,8 @@ package lvm import ( + "fmt" + "github.com/gofrs/flock" ) @@ -13,5 +15,16 @@ var lvmlock *flock.Flock // - https://jira.mesosphere.com/browse/DCOS_OSS-5434 // - https://github.com/lvmteam/lvm2/issues/23 func SetLockFilePath(filepath string) { + log.Printf("using lock file at %q", filepath) lvmlock = flock.New(filepath) + log.Printf("checking if lock can be acquired") + err := lvmlock.Lock() + if err != nil { + panic(fmt.Sprintf("cannot acquire lock: %v", err)) + } + log.Printf("lock acquired, releasing lock") + if err := lvmlock.Unlock(); err != nil { + panic(fmt.Sprintf("cannot release lock: %v", err)) + } + log.Printf("configured lock file") }