Lock the cache dir for doing operations

This first implementation is far from ideal. To make it better
requires refactoring the code, since the cache logic is all over
the place. In any case, I think this is better than nothing.

I do not use 'github.com/containers/storage/pkg/lockfile' because
it does not allow to acquire an exclusive lock and after releasing
it, to acquire a shared one. After refactoring I'll try it again.

Signed-off-by: German Maglione <[email protected]>

Author: germag

cmd/list.go

@@ -1,14 +1,15 @@
 package cmd
 
 import (
-	"github.com/sirupsen/logrus"
 	"os"
 
 	"gitlab.com/bootc-org/podman-bootc/pkg/config"
 	"gitlab.com/bootc-org/podman-bootc/pkg/user"
+	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 	"gitlab.com/bootc-org/podman-bootc/pkg/vm"
 
 	"github.com/containers/common/pkg/report"
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -83,13 +84,20 @@ func getVMInfo(user user.User, libvirtUri string, imageId string) (*vm.BootcVMCo
 		ImageID:    imageId,
 		User:       user,
 		LibvirtUri: libvirtUri,
+		Locking:    utils.Shared,
 	})
 
 	if err != nil {
 		return nil, err
 	}
 
-	defer bootcVM.CloseConnection()
+	// Let's be explicit instead of relying on the defer exec order
+	defer func() {
+		bootcVM.CloseConnection()
+		if err := bootcVM.Unlock(); err != nil {
+			logrus.Warningf("unable to unlock VM %s: %v", imageId, err)
+		}
+	}()
 
 	cfg, err := bootcVM.GetConfig()
 	if err != nil {

cmd/rm.go

@@ -6,6 +6,7 @@ import (
 
 	"gitlab.com/bootc-org/podman-bootc/pkg/config"
 	"gitlab.com/bootc-org/podman-bootc/pkg/user"
+	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 	"gitlab.com/bootc-org/podman-bootc/pkg/vm"
 
 	"github.com/sirupsen/logrus"
@@ -60,12 +61,19 @@ func prune(id string) error {
 		ImageID:    id,
 		LibvirtUri: config.LibvirtUri,
 		User:       user,
+		Locking:    utils.Exclusive,
 	})
 	if err != nil {
 		return fmt.Errorf("unable to get VM %s: %v", id, err)
 	}
 
-	defer bootcVM.CloseConnection()
+	// Let's be explicit instead of relying on the defer exec order
+	defer func() {
+		bootcVM.CloseConnection()
+		if err := bootcVM.Unlock(); err != nil {
+			logrus.Warningf("unable to unlock VM %s: %v", id, err)
+		}
+	}()
 
 	if force {
 		err := forceKillVM(bootcVM)

cmd/run.go

@@ -116,13 +116,20 @@ func doRun(flags *cobra.Command, args []string) error {
 		ImageID:    bootcDisk.GetImageId(),
 		User:       user,
 		LibvirtUri: config.LibvirtUri,
+		Locking:    utils.Shared,
 	})
 
 	if err != nil {
 		return fmt.Errorf("unable to initialize VM: %w", err)
 	}
 
-	defer bootcVM.CloseConnection()
+	// Let's be explicit instead of relying on the defer exec order
+	defer func() {
+		bootcVM.CloseConnection()
+		if err := bootcVM.Unlock(); err != nil {
+			logrus.Warningf("unable to unlock VM %s: %v", bootcDisk.GetImageId(), err)
+		}
+	}()
 
 	cmd := args[1:]
 	err = bootcVM.Run(vm.RunVMParameters{

cmd/ssh.go

@@ -6,6 +6,7 @@ import (
 	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 	"gitlab.com/bootc-org/podman-bootc/pkg/vm"
 
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -35,13 +36,20 @@ func doSsh(_ *cobra.Command, args []string) error {
 		ImageID:    id,
 		User:       user,
 		LibvirtUri: config.LibvirtUri,
+		Locking:    utils.Shared,
 	})
 
 	if err != nil {
 		return err
 	}
 
-	defer vm.CloseConnection()
+	// Let's be explicit instead of relying on the defer exec order
+	defer func() {
+		vm.CloseConnection()
+		if err := vm.Unlock(); err != nil {
+			logrus.Warningf("unable to unlock VM %s: %v", id, err)
+		}
+	}()
 
 	err = vm.SetUser(sshUser)
 	if err != nil {

cmd/stop.go

@@ -3,8 +3,10 @@ package cmd
 import (
 	"gitlab.com/bootc-org/podman-bootc/pkg/config"
 	"gitlab.com/bootc-org/podman-bootc/pkg/user"
+	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 	"gitlab.com/bootc-org/podman-bootc/pkg/vm"
 
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -31,10 +33,19 @@ func doStop(_ *cobra.Command, args []string) (err error) {
 		ImageID:    id,
 		LibvirtUri: config.LibvirtUri,
 		User:       user,
+		Locking:    utils.Exclusive,
 	})
 	if err != nil {
 		return err
 	}
-	defer bootcVM.CloseConnection()
+
+	// Let's be explicit instead of relying on the defer exec order
+	defer func() {
+		bootcVM.CloseConnection()
+		if err := bootcVM.Unlock(); err != nil {
+			logrus.Warningf("unable to unlock VM %s: %v", id, err)
+		}
+	}()
+
 	return bootcVM.Delete()
 }

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/containers/common v0.58.1
 	github.com/containers/podman/v5 v5.0.1
 	github.com/docker/go-units v0.5.0
+	github.com/gofrs/flock v0.8.1
 	github.com/onsi/ginkgo/v2 v2.17.1
 	github.com/onsi/gomega v1.32.0
 	github.com/opencontainers/runtime-spec v1.2.0

go.sum

@@ -229,6 +229,8 @@ github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MG
 github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/godbus/dbus/v5 v5.1.1-0.20230522191255-76236955d466 h1:sQspH8M4niEijh3PFscJRLDnkL547IeP7kpPe3uUhEg=
 github.com/godbus/dbus/v5 v5.1.1-0.20230522191255-76236955d466/go.mod h1:ZiQxhyQ+bbbfxUKVvjfO498oPYvtYhZzycal3G/NHmU=
+github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=
+github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=

pkg/bootc/bootc_disk.go

@@ -13,6 +13,7 @@ import (
 
 	"gitlab.com/bootc-org/podman-bootc/pkg/config"
 	"gitlab.com/bootc-org/podman-bootc/pkg/user"
+	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 
 	"github.com/containers/podman/v5/pkg/bindings/containers"
 	"github.com/containers/podman/v5/pkg/bindings/images"
@@ -105,6 +106,21 @@ func (p *BootcDisk) Install(quiet bool) (err error) {
 
 	// Create VM cache dir; one per oci bootc image
 	p.Directory = filepath.Join(p.User.CacheDir(), p.ImageId)
+	lock := utils.NewCacheLock(p.User.RunDir(), p.Directory)
+	locked, err := lock.TryLock(utils.Exclusive)
+	if err != nil {
+		return fmt.Errorf("error locking the VM cache path: %w", err)
+	}
+	if !locked {
+		return fmt.Errorf("unable to lock the VM cache path")
+	}
+
+	defer func() {
+		if err := lock.Unlock(); err != nil {
+			logrus.Errorf("unable to unlock VM %s: %v", p.ImageId, err)
+		}
+	}()
+
 	if err := os.MkdirAll(p.Directory, os.ModePerm); err != nil {
 		return fmt.Errorf("error while making bootc disk directory: %w", err)
 	}

pkg/utils/locks.go

@@ -0,0 +1,41 @@
+package utils
+
+import (
+	"path/filepath"
+
+	"github.com/gofrs/flock"
+)
+
+type AccessMode uint
+
+const (
+	Exclusive AccessMode = iota
+	Shared
+)
+
+type CacheLock struct {
+	inner *flock.Flock
+}
+
+// NewCacheLock  returns a new instance of *CacheLock. It takes the path to the VM cache dir.
+func NewCacheLock(lockDir, cacheDir string) CacheLock {
+	imageLongID := filepath.Base(cacheDir)
+	cacheDirLockFile := filepath.Join(lockDir, imageLongID+".lock")
+	return CacheLock{inner: flock.New(cacheDirLockFile)}
+}
+
+// TryLock takes an exclusive or shared lock, based on the parameter mode.
+// The lock is non-blocking, if we are unable to lock the cache directory,
+// the function will return false instead of waiting for the lock.
+func (l CacheLock) TryLock(mode AccessMode) (bool, error) {
+	if mode == Exclusive {
+		return l.inner.TryLock()
+	} else {
+		return l.inner.TryRLock()
+	}
+}
+
+// Unlock unlocks the cache lock.
+func (l CacheLock) Unlock() error {
+	return l.inner.Unlock()
+}

pkg/vm/vm.go

@@ -3,6 +3,7 @@ package vm
 import (
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -14,12 +15,15 @@ import (
 	"gitlab.com/bootc-org/podman-bootc/pkg/bootc"
 	"gitlab.com/bootc-org/podman-bootc/pkg/config"
 	"gitlab.com/bootc-org/podman-bootc/pkg/user"
+	"gitlab.com/bootc-org/podman-bootc/pkg/utils"
 
 	"github.com/docker/go-units"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh"
 )
 
+var ErrVMInUse = errors.New("VM already in use")
+
 // GetVMCachePath returns the path to the VM cache directory
 func GetVMCachePath(imageId string, user user.User) (path string, err error) {
 	files, err := os.ReadDir(user.CacheDir())
@@ -45,6 +49,7 @@ type NewVMParameters struct {
 	ImageID    string
 	User       user.User //user who is running the podman bootc command
 	LibvirtUri string    //linux only
+	Locking    utils.AccessMode
 }
 
 type RunVMParameters struct {
@@ -71,6 +76,7 @@ type BootcVM interface {
 	GetConfig() (*BootcVMConfig, error)
 	CloseConnection()
 	PrintConsole() error
+	Unlock() error
 }
 
 type BootcVMCommon struct {
@@ -92,6 +98,7 @@ type BootcVMCommon struct {
 	cloudInitDir  string
 	cloudInitArgs string
 	bootcDisk     bootc.BootcDisk
+	cacheDirLock  utils.CacheLock
 }
 
 type BootcVMConfig struct {
@@ -274,3 +281,31 @@ func (b *BootcVMCommon) tmpFileInjectSshKeyEnc() (string, error) {
 	tmpFileCmdEnc := base64.StdEncoding.EncodeToString([]byte(tmpFileCmd))
 	return tmpFileCmdEnc, nil
 }
+
+func lockVM(params NewVMParameters, cacheDir string) (utils.CacheLock, error) {
+	lock := utils.NewCacheLock(params.User.RunDir(), cacheDir)
+	locked, err := lock.TryLock(params.Locking)
+	if err != nil {
+		return lock, fmt.Errorf("unable to lock the VM cache path: %w", err)
+	}
+
+	if !locked {
+		return lock, ErrVMInUse
+	}
+
+	cacheDirExists, err := utils.FileExists(cacheDir)
+	if err != nil {
+		if err := lock.Unlock(); err != nil {
+			logrus.Debugf("unlock failed: %v", err)
+		}
+		return lock, fmt.Errorf("unable to check cache path: %w", err)
+	}
+	if !cacheDirExists {
+		if err := lock.Unlock(); err != nil {
+			logrus.Debugf("unlock failed: %v", err)
+		}
+		return lock, fmt.Errorf("'%s' does not exists", params.ImageID)
+	}
+
+	return lock, nil
+}