diff --git a/cmd/podman/machine/rm.go b/cmd/podman/machine/rm.go index a14d4784db..4a2130cbd8 100644 --- a/cmd/podman/machine/rm.go +++ b/cmd/podman/machine/rm.go @@ -3,18 +3,11 @@ package machine import ( - "bufio" - "fmt" - "os" - "strings" - "github.com/containers/podman/v5/cmd/podman/registry" "github.com/containers/podman/v5/libpod/events" "github.com/containers/podman/v5/pkg/machine" - "github.com/containers/podman/v5/pkg/machine/define" "github.com/containers/podman/v5/pkg/machine/shim" "github.com/containers/podman/v5/pkg/machine/vmconfigs" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -71,69 +64,9 @@ func rm(_ *cobra.Command, args []string) error { return err } - state, err := provider.State(mc, false) - if err != nil { + if err := shim.Remove(mc, provider, dirs, destroyOptions); err != nil { return err } - - if state == define.Running { - if !destroyOptions.Force { - return &define.ErrVMRunningCannotDestroyed{Name: vmName} - } - if err := shim.Stop(mc, provider, dirs, true); err != nil { - return err - } - } - - rmFiles, genericRm, err := mc.Remove(destroyOptions.SaveIgnition, destroyOptions.SaveImage) - if err != nil { - return err - } - - providerFiles, providerRm, err := provider.Remove(mc) - if err != nil { - return err - } - - // Add provider specific files to the list - rmFiles = append(rmFiles, providerFiles...) - - // Important! - // Nothing can be removed at this point. The user can still opt out below - // - - if !destroyOptions.Force { - // Warn user - confirmationMessage(rmFiles) - reader := bufio.NewReader(os.Stdin) - fmt.Print("Are you sure you want to continue? [y/N] ") - answer, err := reader.ReadString('\n') - if err != nil { - return err - } - if strings.ToLower(answer)[0] != 'y' { - return nil - } - } - - // - // All actual removal of files and vms should occur after this - // - - if err := providerRm(); err != nil { - logrus.Errorf("failed to remove virtual machine from provider for %q: %v", vmName, err) - } - - if err := genericRm(); err != nil { - return fmt.Errorf("failed to remove machines files: %v", err) - } newMachineEvent(events.Remove, events.Event{Name: vmName}) return nil } - -func confirmationMessage(files []string) { - fmt.Printf("The following files will be deleted:\n\n\n") - for _, msg := range files { - fmt.Println(msg) - } -} diff --git a/cmd/podman/machine/set.go b/cmd/podman/machine/set.go index bf6a21c66d..431de52b5d 100644 --- a/cmd/podman/machine/set.go +++ b/cmd/podman/machine/set.go @@ -10,6 +10,7 @@ import ( "github.com/containers/podman/v5/cmd/podman/registry" "github.com/containers/podman/v5/pkg/machine" "github.com/containers/podman/v5/pkg/machine/define" + "github.com/containers/podman/v5/pkg/machine/shim" "github.com/containers/podman/v5/pkg/machine/vmconfigs" "github.com/spf13/cobra" ) @@ -136,10 +137,5 @@ func setMachine(cmd *cobra.Command, args []string) error { // At this point, we have the known changed information, etc // Walk through changes to the providers if they need them - if err := provider.SetProviderAttrs(mc, setOpts); err != nil { - return err - } - - // Update the configuration file last if everything earlier worked - return mc.Write() + return shim.Set(mc, provider, setOpts) } diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index c5b397b3b0..4c701eedb9 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -18,9 +18,6 @@ import ( ) func (a *AppleHVStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() error, error) { - mc.Lock() - defer mc.Unlock() - return []string{}, func() error { return nil }, nil } @@ -75,8 +72,6 @@ func (a *AppleHVStubber) State(mc *vmconfigs.MachineConfig, _ bool) (define.Stat } func (a *AppleHVStubber) StopVM(mc *vmconfigs.MachineConfig, _ bool) error { - mc.Lock() - defer mc.Unlock() return mc.AppleHypervisor.Vfkit.Stop(false, true) } diff --git a/pkg/machine/applehv/stubber.go b/pkg/machine/applehv/stubber.go index 0603485a95..d4915ba0bb 100644 --- a/pkg/machine/applehv/stubber.go +++ b/pkg/machine/applehv/stubber.go @@ -93,9 +93,6 @@ func (a AppleHVStubber) RemoveAndCleanMachines(_ *define.MachineDirs) error { } func (a AppleHVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, opts define.SetOptions) error { - mc.Lock() - defer mc.Unlock() - state, err := a.State(mc, false) if err != nil { return err diff --git a/pkg/machine/hyperv/stubber.go b/pkg/machine/hyperv/stubber.go index 43f61f20de..d3070151e7 100644 --- a/pkg/machine/hyperv/stubber.go +++ b/pkg/machine/hyperv/stubber.go @@ -138,9 +138,6 @@ func (h HyperVStubber) MountVolumesToVM(mc *vmconfigs.MachineConfig, quiet bool) } func (h HyperVStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() error, error) { - mc.Lock() - defer mc.Unlock() - _, vm, err := GetVMFromMC(mc) if err != nil { return nil, nil, err @@ -247,9 +244,6 @@ func (h HyperVStubber) State(mc *vmconfigs.MachineConfig, bypass bool) (define.S } func (h HyperVStubber) StopVM(mc *vmconfigs.MachineConfig, hardStop bool) error { - mc.Lock() - defer mc.Unlock() - vmm := hypervctl.NewVirtualMachineManager() vm, err := vmm.GetMachine(mc.Name) if err != nil { @@ -307,9 +301,6 @@ func (h HyperVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, opts define cpuChanged, memoryChanged bool ) - mc.Lock() - defer mc.Unlock() - _, vm, err := GetVMFromMC(mc) if err != nil { return err diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index ce71243c94..1b62240127 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -114,9 +114,6 @@ func (q *QEMUStubber) waitForMachineToStop(mc *vmconfigs.MachineConfig) error { // Stop uses the qmp monitor to call a system_powerdown func (q *QEMUStubber) StopVM(mc *vmconfigs.MachineConfig, _ bool) error { - mc.Lock() - defer mc.Unlock() - if err := mc.Refresh(); err != nil { return err } @@ -229,9 +226,6 @@ func (q *QEMUStubber) stopLocked(mc *vmconfigs.MachineConfig) error { // Remove deletes all the files associated with a machine including the image itself func (q *QEMUStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() error, error) { - mc.Lock() - defer mc.Unlock() - qemuRmFiles := []string{ mc.QEMUHypervisor.QEMUPidPath.GetPath(), mc.QEMUHypervisor.QMPMonitor.Address.GetPath(), diff --git a/pkg/machine/qemu/stubber.go b/pkg/machine/qemu/stubber.go index 9d88e29d0d..df056ec803 100644 --- a/pkg/machine/qemu/stubber.go +++ b/pkg/machine/qemu/stubber.go @@ -252,9 +252,6 @@ func (q *QEMUStubber) resizeDisk(newSize strongunits.GiB, diskPath *define.VMFil } func (q *QEMUStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, opts define.SetOptions) error { - mc.Lock() - defer mc.Unlock() - state, err := q.State(mc, false) if err != nil { return err diff --git a/pkg/machine/shim/host.go b/pkg/machine/shim/host.go index 06ff9a207f..133ec91242 100644 --- a/pkg/machine/shim/host.go +++ b/pkg/machine/shim/host.go @@ -1,11 +1,13 @@ package shim import ( + "bufio" "errors" "fmt" "os" "path/filepath" "runtime" + "strings" "time" "github.com/containers/podman/v5/pkg/machine" @@ -310,6 +312,14 @@ func getMCsOverProviders(vmstubbers []vmconfigs.VMProvider) (map[string]*vmconfi func Stop(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDefine.MachineDirs, hardStop bool) error { // state is checked here instead of earlier because stopping a stopped vm is not considered // an error. so putting in one place instead of sprinkling all over. + mc.Lock() + defer mc.Unlock() + + return stopLocked(mc, mp, dirs, hardStop) +} + +// stopLocked stops the machine and expects the caller to hold the machine's lock. +func stopLocked(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDefine.MachineDirs, hardStop bool) error { state, err := mp.State(mc, false) if err != nil { return err @@ -354,6 +364,9 @@ func Start(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDe defaultBackoff := 500 * time.Millisecond maxBackoffs := 6 + mc.Lock() + defer mc.Unlock() + gvproxyPidFile, err := dirs.RuntimeDir.AppendToNewVMFile("gvproxy.pid", nil) if err != nil { return err @@ -467,6 +480,91 @@ func Start(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDe return nil } +func Set(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, opts machineDefine.SetOptions) error { + mc.Lock() + defer mc.Unlock() + + if err := mp.SetProviderAttrs(mc, opts); err != nil { + return err + } + + // Update the configuration file last if everything earlier worked + return mc.Write() +} + +func Remove(mc *vmconfigs.MachineConfig, mp vmconfigs.VMProvider, dirs *machineDefine.MachineDirs, opts machine.RemoveOptions) error { + mc.Lock() + defer mc.Unlock() + + state, err := mp.State(mc, false) + if err != nil { + return err + } + + if state == machineDefine.Running { + if !opts.Force { + return &machineDefine.ErrVMRunningCannotDestroyed{Name: mc.Name} + } + } + + rmFiles, genericRm, err := mc.Remove(opts.SaveIgnition, opts.SaveImage) + if err != nil { + return err + } + + providerFiles, providerRm, err := mp.Remove(mc) + if err != nil { + return err + } + + // Add provider specific files to the list + rmFiles = append(rmFiles, providerFiles...) + + // Important! + // Nothing can be removed at this point. The user can still opt out below + // + + if !opts.Force { + // Warn user + confirmationMessage(rmFiles) + reader := bufio.NewReader(os.Stdin) + fmt.Print("Are you sure you want to continue? [y/N] ") + answer, err := reader.ReadString('\n') + if err != nil { + return err + } + if strings.ToLower(answer)[0] != 'y' { + return nil + } + } + + if state == machineDefine.Running { + if err := stopLocked(mc, mp, dirs, true); err != nil { + return err + } + } + + // + // All actual removal of files and vms should occur after this + // + + if err := providerRm(); err != nil { + logrus.Errorf("failed to remove virtual machine from provider for %q: %v", mc.Name, err) + } + + if err := genericRm(); err != nil { + return fmt.Errorf("failed to remove machines files: %v", err) + } + return nil +} + +func confirmationMessage(files []string) { + fmt.Printf("The following files will be deleted:\n\n\n") + for _, msg := range files { + fmt.Println(msg) + } +} + func Reset(dirs *machineDefine.MachineDirs, mp vmconfigs.VMProvider, mcs map[string]*vmconfigs.MachineConfig) error { var resetErrors *multierror.Error for _, mc := range mcs { diff --git a/pkg/machine/vmconfigs/machine.go b/pkg/machine/vmconfigs/machine.go index 0189fab337..b1877db0fa 100644 --- a/pkg/machine/vmconfigs/machine.go +++ b/pkg/machine/vmconfigs/machine.go @@ -108,13 +108,6 @@ func (mc *MachineConfig) Unlock() { mc.lock.Unlock() } -// Write is a locking way to the machine configuration file -func (mc *MachineConfig) Write() error { - mc.Lock() - defer mc.Unlock() - return mc.write() -} - // Refresh reloads the config file from disk func (mc *MachineConfig) Refresh() error { content, err := os.ReadFile(mc.configPath.GetPath()) @@ -125,7 +118,7 @@ func (mc *MachineConfig) Refresh() error { } // write is a non-locking way to write the machine configuration file to disk -func (mc *MachineConfig) write() error { +func (mc *MachineConfig) Write() error { if mc.configPath == nil { return fmt.Errorf("no configuration file associated with vm %q", mc.Name) } diff --git a/pkg/machine/wsl/stubber.go b/pkg/machine/wsl/stubber.go index 0e994727a8..fdb07ad18e 100644 --- a/pkg/machine/wsl/stubber.go +++ b/pkg/machine/wsl/stubber.go @@ -120,9 +120,6 @@ func (w WSLStubber) RemoveAndCleanMachines(_ *define.MachineDirs) error { } func (w WSLStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, opts define.SetOptions) error { - mc.Lock() - defer mc.Unlock() - state, err := w.State(mc, false) if err != nil { return err @@ -242,10 +239,7 @@ func (w WSLStubber) StopVM(mc *vmconfigs.MachineConfig, hardStop bool) error { var ( err error ) - mc.Lock() - defer mc.Unlock() - // recheck after lock if running, err := isRunning(mc.Name); !running { return err }