Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement SetLock for all virt providers #20322

Merged
merged 1 commit into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions pkg/machine/applehv/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/containers/podman/v4/pkg/strongunits"
"github.com/containers/podman/v4/pkg/util"
"github.com/containers/podman/v4/utils"
"github.com/containers/storage/pkg/lockfile"
vfConfig "github.com/crc-org/vfkit/pkg/config"
vfRest "github.com/crc-org/vfkit/pkg/rest"
"github.com/docker/go-units"
Expand Down Expand Up @@ -77,6 +78,9 @@ type MacMachine struct {
LogPath machine.VMFile
GvProxyPid machine.VMFile
GvProxySock machine.VMFile

// Used at runtime for serializing write operations
lock *lockfile.LockFile
}

// setGVProxyInfo sets the VM's gvproxy pid and socket files
Expand Down Expand Up @@ -353,6 +357,9 @@ func (m *MacMachine) Remove(name string, opts machine.RemoveOptions) (string, fu
files []string
)

m.lock.Lock()
defer m.lock.Unlock()

vmState, err := m.Vfkit.state()
if err != nil {
return "", nil, err
Expand Down Expand Up @@ -403,6 +410,10 @@ func (m *MacMachine) writeConfig() error {

func (m *MacMachine) Set(name string, opts machine.SetOptions) ([]error, error) {
var setErrors []error

m.lock.Lock()
defer m.lock.Unlock()

vmState, err := m.State(false)
if err != nil {
return nil, err
Expand Down Expand Up @@ -527,6 +538,9 @@ func (m *MacMachine) addVolumesToVfKit() error {
func (m *MacMachine) Start(name string, opts machine.StartOptions) error {
var ignitionSocket *machine.VMFile

m.lock.Lock()
defer m.lock.Unlock()

st, err := m.State(false)
if err != nil {
return err
Expand Down Expand Up @@ -680,6 +694,9 @@ func (m *MacMachine) State(_ bool) (machine.Status, error) {
}

func (m *MacMachine) Stop(name string, opts machine.StopOptions) error {
m.lock.Lock()
defer m.lock.Unlock()

vmState, err := m.State(false)
if err != nil {
return err
Expand Down Expand Up @@ -718,6 +735,13 @@ func (m *MacMachine) loadFromFile() (*MacMachine, error) {
if err := loadMacMachineFromJSON(jsonPath, &mm); err != nil {
return nil, err
}

lock, err := machine.GetLock(mm.Name, vmtype)
if err != nil {
return nil, err
}
mm.lock = lock

return &mm, nil
}

Expand Down
19 changes: 19 additions & 0 deletions pkg/machine/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"time"

"github.com/containers/storage/pkg/homedir"
"github.com/containers/storage/pkg/lockfile"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -137,6 +138,24 @@ type VM interface {
Stop(name string, opts StopOptions) error
}

func GetLock(name string, vmtype VMType) (*lockfile.LockFile, error) {
// FIXME: there's a painful amount of `GetConfDir` calls scattered
// across the code base. This should be done once and stored
// somewhere instead.
vmConfigDir, err := GetConfDir(vmtype)
if err != nil {
return nil, err
}

lockPath := filepath.Join(vmConfigDir, name+".lock")
lock, err := lockfile.GetLockFile(lockPath)
if err != nil {
return nil, fmt.Errorf("creating lockfile for VM: %w", err)
}

return lock, nil
}

type DistributionDownload interface {
HasUsableCache() (bool, error)
Get() *Download
Expand Down
23 changes: 23 additions & 0 deletions pkg/machine/hyperv/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/containers/podman/v4/pkg/strongunits"
"github.com/containers/podman/v4/pkg/util"
"github.com/containers/podman/v4/utils"
"github.com/containers/storage/pkg/lockfile"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -66,6 +67,8 @@ type HyperVMachine struct {
LastUp time.Time
// GVProxy will write its PID here
GvProxyPid machine.VMFile
// Used at runtime for serializing write operations
lock *lockfile.LockFile
}

// addNetworkAndReadySocketsToRegistry adds the Network and Ready sockets to the
Expand Down Expand Up @@ -358,6 +361,9 @@ func (m *HyperVMachine) Remove(_ string, opts machine.RemoveOptions) (string, fu
files []string
diskPath string
)
m.lock.Lock()
defer m.lock.Unlock()

vmm := hypervctl.NewVirtualMachineManager()
vm, err := vmm.GetMachine(m.Name)
if err != nil {
Expand Down Expand Up @@ -400,6 +406,10 @@ func (m *HyperVMachine) Set(name string, opts machine.SetOptions) ([]error, erro
cpuChanged, memoryChanged bool
setErrors []error
)

m.lock.Lock()
defer m.lock.Unlock()

vmm := hypervctl.NewVirtualMachineManager()
// Considering this a hard return if we cannot lookup the machine
vm, err := vmm.GetMachine(m.Name)
Expand Down Expand Up @@ -476,6 +486,9 @@ func (m *HyperVMachine) SSH(name string, opts machine.SSHOptions) error {
}

func (m *HyperVMachine) Start(name string, opts machine.StartOptions) error {
m.lock.Lock()
defer m.lock.Unlock()

vmm := hypervctl.NewVirtualMachineManager()
vm, err := vmm.GetMachine(m.Name)
if err != nil {
Expand Down Expand Up @@ -536,6 +549,9 @@ func (m *HyperVMachine) State(_ bool) (machine.Status, error) {
}

func (m *HyperVMachine) Stop(name string, opts machine.StopOptions) error {
m.lock.Lock()
defer m.lock.Unlock()

vmm := hypervctl.NewVirtualMachineManager()
vm, err := vmm.GetMachine(m.Name)
if err != nil {
Expand Down Expand Up @@ -582,6 +598,13 @@ func (m *HyperVMachine) loadFromFile() (*HyperVMachine, error) {
}
return nil, err
}

lock, err := machine.GetLock(mm.Name, vmtype)
if err != nil {
return nil, err
}
mm.lock = lock

vmm := hypervctl.NewVirtualMachineManager()
vm, err := vmm.GetMachine(m.Name)
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions pkg/machine/qemu/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ func (p *QEMUVirtualization) LoadVMByName(name string) (machine.VM, error) {
return nil, err
}

lock, err := machine.GetLock(vm.Name, vmtype)
if err != nil {
return nil, err
}
vm.lock = lock

return vm, nil
}

Expand Down
35 changes: 0 additions & 35 deletions pkg/machine/qemu/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,29 +80,6 @@ type MachineVM struct {
lock *lockfile.LockFile
}

func (v *MachineVM) setLock() error {
if v.lock != nil {
return nil
}

// FIXME: there's a painful amount of `GetConfDir` calls scattered
// across the code base. This should be done once and stored
// somewhere instead.
vmConfigDir, err := machine.GetConfDir(vmtype)
if err != nil {
return err
}

lockPath := filepath.Join(vmConfigDir, v.Name+".lock")
lock, err := lockfile.GetLockFile(lockPath)
if err != nil {
return fmt.Errorf("creating lockfile for VM: %w", err)
}

v.lock = lock
return nil
}

type Monitor struct {
// Address portion of the qmp monitor (/tmp/tmp.sock)
Address machine.VMFile
Expand Down Expand Up @@ -360,9 +337,6 @@ func (v *MachineVM) Set(_ string, opts machine.SetOptions) ([]error, error) {
// The setting(s) that failed to be applied will have its errors returned in setErrors
var setErrors []error

if err := v.setLock(); err != nil {
return nil, err
}
v.lock.Lock()
defer v.lock.Unlock()

Expand Down Expand Up @@ -596,9 +570,6 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
defaultBackoff := 500 * time.Millisecond
maxBackoffs := 6

if err := v.setLock(); err != nil {
return err
}
v.lock.Lock()
defer v.lock.Unlock()

Expand Down Expand Up @@ -929,9 +900,6 @@ func (v *MachineVM) VMPid() (int, error) {

// Stop uses the qmp monitor to call a system_powerdown
func (v *MachineVM) Stop(_ string, _ machine.StopOptions) error {
if err := v.setLock(); err != nil {
return err
}
v.lock.Lock()
defer v.lock.Unlock()

Expand Down Expand Up @@ -1154,9 +1122,6 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func()
files []string
)

if err := v.setLock(); err != nil {
return "", nil, err
}
v.lock.Lock()
defer v.lock.Unlock()

Expand Down
10 changes: 10 additions & 0 deletions pkg/machine/wsl/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ func (p *WSLVirtualization) LoadVMByName(name string) (machine.VM, error) {
}

vm, err := readAndMigrate(configPath, name)
if err != nil {
return nil, err
}

lock, err := machine.GetLock(vm.Name, vmtype)
if err != nil {
return nil, err
}
vm.lock = lock

return vm, err
}

Expand Down
15 changes: 15 additions & 0 deletions pkg/machine/wsl/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/containers/podman/v4/pkg/machine/wsl/wutil"
"github.com/containers/podman/v4/pkg/util"
"github.com/containers/storage/pkg/homedir"
"github.com/containers/storage/pkg/lockfile"
"github.com/sirupsen/logrus"
"golang.org/x/text/encoding/unicode"
"golang.org/x/text/transform"
Expand Down Expand Up @@ -300,6 +301,8 @@ type MachineVM struct {
Version int
// Whether to use user-mode networking
UserModeNetworking bool
// Used at runtime for serializing write operations
lock *lockfile.LockFile
}

type ExitCodeError struct {
Expand Down Expand Up @@ -1061,6 +1064,9 @@ func (v *MachineVM) Set(_ string, opts machine.SetOptions) ([]error, error) {
// The setting(s) that failed to be applied will have its errors returned in setErrors
var setErrors []error

v.lock.Lock()
defer v.lock.Unlock()

if opts.Rootful != nil && v.Rootful != *opts.Rootful {
err := v.setRootful(*opts.Rootful)
if err != nil {
Expand Down Expand Up @@ -1118,6 +1124,9 @@ func (v *MachineVM) Set(_ string, opts machine.SetOptions) ([]error, error) {
}

func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
v.lock.Lock()
defer v.lock.Unlock()

if v.isRunning() {
return machine.ErrVMAlreadyRunning
}
Expand Down Expand Up @@ -1406,6 +1415,9 @@ func isSystemdRunning(dist string) (bool, error) {
}

func (v *MachineVM) Stop(name string, _ machine.StopOptions) error {
v.lock.Lock()
defer v.lock.Unlock()

dist := toDist(v.Name)

wsl, err := isWSLRunning(dist)
Expand Down Expand Up @@ -1529,6 +1541,9 @@ func readWinProxyTid(v *MachineVM) (uint32, uint32, string, error) {
func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, func() error, error) {
var files []string

v.lock.Lock()
defer v.lock.Unlock()

if v.isRunning() {
if !opts.Force {
return "", nil, &machine.ErrVMRunningCannotDestroyed{Name: v.Name}
Expand Down