Merge pull request #712 from darkowlzz/concurrent-vm-creation

Fix concurrent VM creation due to busy device error
weaveworks · Oct 12, 2020 · 6ce24e1 · 6ce24e1
2 parents 1127008 + 63cd55c
commit 6ce24e1
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 8 deletions.
diff --git a/cmd/ignite-spawn/ignite-spawn.go b/cmd/ignite-spawn/ignite-spawn.go
@@ -10,7 +10,7 @@ import (
 	"github.com/weaveworks/ignite/pkg/apis/ignite/scheme"
 	"github.com/weaveworks/ignite/pkg/constants"
 	"github.com/weaveworks/ignite/pkg/container"
-	dmcleanup "github.com/weaveworks/ignite/pkg/dmlegacy/cleanup"
+	"github.com/weaveworks/ignite/pkg/dmlegacy"
 	"github.com/weaveworks/ignite/pkg/prometheus"
 	"github.com/weaveworks/ignite/pkg/util"
 	patchutil "github.com/weaveworks/libgitops/pkg/util/patch"
@@ -60,7 +60,7 @@ func StartVM(vm *api.VM) (err error) {
 	defer util.DeferErr(&err, func() error { return patchStopped(vm) })
 
 	// Remove the snapshot overlay post-run, which also removes the detached backing loop devices
-	defer util.DeferErr(&err, func() error { return dmcleanup.DeactivateSnapshot(vm) })
+	defer util.DeferErr(&err, func() error { return dmlegacy.DeactivateSnapshot(vm) })
 
 	// Remove the Prometheus socket post-run
 	defer util.DeferErr(&err, func() error { return os.Remove(metricsSocket) })

diff --git a/pkg/dmlegacy/cleanup/deactivate.go → pkg/dmlegacy/deactivate.go b/pkg/dmlegacy/cleanup/deactivate.go → pkg/dmlegacy/deactivate.go
@@ -1,12 +1,33 @@
-package cleanup
+package dmlegacy
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/nightlyone/lockfile"
+
 	api "github.com/weaveworks/ignite/pkg/apis/ignite"
 	"github.com/weaveworks/ignite/pkg/util"
 )
 
 // DeactivateSnapshot deactivates the snapshot by removing it with dmsetup
 func DeactivateSnapshot(vm *api.VM) error {
+	// Global lock path.
+	glpath := filepath.Join(os.TempDir(), snapshotLockFileName)
+
+	// Create a lockfile and obtain a lock.
+	lock, err := lockfile.New(glpath)
+	if err != nil {
+		err = fmt.Errorf("failed to create lockfile: %w", err)
+		return err
+	}
+	if err = obtainLock(lock); err != nil {
+		return err
+	}
+	// Release the lock at the end.
+	defer util.DeferErr(&err, lock.Unlock)
+
 	dmArgs := []string{
 		"remove",
 		"--verifyudev", // if udevd is not running, dmsetup will manage the device node in /dev/mapper
@@ -21,6 +42,6 @@ func DeactivateSnapshot(vm *api.VM) error {
 		dmArgs = append(dmArgs, baseDev)
 	}
 
-	_, err := util.ExecuteCommand("dmsetup", dmArgs...)
+	_, err = util.ExecuteCommand("dmsetup", dmArgs...)
 	return err
 }
diff --git a/pkg/dmlegacy/snapshot.go b/pkg/dmlegacy/snapshot.go
@@ -41,6 +41,10 @@ func ActivateSnapshot(vm *api.VM) (devicePath string, err error) {
 	// Serialize this operation by creating a global lock file when creating a
 	// loop device and release the lock after setting up device mapper using the
 	// loop device.
+	// Also, concurrent interactions with device mapper results in the error:
+	//   Device or resource busy
+	// Serializing the interaction with device mapper helps avoid issues due to
+	// race conditions.
 
 	// Global lock path.
 	glpath := filepath.Join(os.TempDir(), snapshotLockFileName)

diff --git a/pkg/dmlegacy/vm.go b/pkg/dmlegacy/vm.go
@@ -10,10 +10,10 @@ import (
 	"path/filepath"
 
 	log "github.com/sirupsen/logrus"
+
 	api "github.com/weaveworks/ignite/pkg/apis/ignite"
 	meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
 	"github.com/weaveworks/ignite/pkg/constants"
-	"github.com/weaveworks/ignite/pkg/dmlegacy/cleanup"
 	"github.com/weaveworks/ignite/pkg/operations/lookup"
 	"github.com/weaveworks/ignite/pkg/providers"
 	"github.com/weaveworks/ignite/pkg/util"
@@ -87,7 +87,7 @@ func copyToOverlay(vm *api.VM) (err error) {
 	if err != nil {
 		return
 	}
-	defer util.DeferErr(&err, func() error { return cleanup.DeactivateSnapshot(vm) })
+	defer util.DeferErr(&err, func() error { return DeactivateSnapshot(vm) })
 
 	mp, err := util.Mount(vm.SnapshotDev())
 	if err != nil {

diff --git a/pkg/operations/remove.go b/pkg/operations/remove.go
@@ -8,7 +8,7 @@ import (
 	api "github.com/weaveworks/ignite/pkg/apis/ignite"
 	meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
 	"github.com/weaveworks/ignite/pkg/client"
-	"github.com/weaveworks/ignite/pkg/dmlegacy/cleanup"
+	"github.com/weaveworks/ignite/pkg/dmlegacy"
 	"github.com/weaveworks/ignite/pkg/logs"
 	"github.com/weaveworks/ignite/pkg/providers"
 	"github.com/weaveworks/ignite/pkg/runtime"
@@ -50,7 +50,7 @@ func CleanupVM(vm *api.VM) error {
 	// After removing the VM container, if the Snapshot Device is still there, clean up
 	if _, err := os.Stat(vm.SnapshotDev()); err == nil {
 		// try remove it again with DeactivateSnapshot
-		if err := cleanup.DeactivateSnapshot(vm); err != nil {
+		if err := dmlegacy.DeactivateSnapshot(vm); err != nil {
 			return err
 		}
 	}