From 2296e71e39a86615b28ae2de8df60e7d4870e722 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 26 Apr 2023 16:44:28 +0200 Subject: [PATCH 1/2] machine: qemu only remove connection after confirmation the connection remove call must be done inside the function that is returned so that we wait until the user confirmed it. Fixes #18330 Signed-off-by: Paul Holzinger --- pkg/machine/qemu/machine.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index a046a8977b..273bbdd380 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -964,13 +964,6 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() files = append(files, socketPath.Path) files = append(files, v.archRemovalFiles()...) - if err := machine.RemoveConnection(v.Name); err != nil { - logrus.Error(err) - } - if err := machine.RemoveConnection(v.Name + "-root"); err != nil { - logrus.Error(err) - } - vmConfigDir, err := machine.GetConfDir(vmtype) if err != nil { return "", nil, err @@ -1001,6 +994,12 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() logrus.Error(err) } } + if err := machine.RemoveConnection(v.Name); err != nil { + logrus.Error(err) + } + if err := machine.RemoveConnection(v.Name + "-root"); err != nil { + logrus.Error(err) + } return nil }, nil } From 64959b744f7ee801917a74866f0a93ea2176fe03 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 26 Apr 2023 16:51:52 +0200 Subject: [PATCH 2/2] pkg/machine: rework RemoveConnection() It really does not make sense to call RemoveConnection() twice and then update the config file a third time in updateDefaultMachineinConfig(). This results in unnecessary reads/writes and more code. Simplyfy this into one function that is only called once and do all updates at once. [NO NEW TESTS NEEDED] Signed-off-by: Paul Holzinger --- cmd/podman/machine/rm.go | 20 -------------------- pkg/machine/connection.go | 20 +++++++++++++++----- pkg/machine/hyperv/machine.go | 5 +---- pkg/machine/qemu/machine.go | 5 +---- pkg/machine/wsl/machine.go | 5 +---- 5 files changed, 18 insertions(+), 37 deletions(-) diff --git a/cmd/podman/machine/rm.go b/cmd/podman/machine/rm.go index 6418b91410..362c9a7d3f 100644 --- a/cmd/podman/machine/rm.go +++ b/cmd/podman/machine/rm.go @@ -9,7 +9,6 @@ import ( "os" "strings" - "github.com/containers/common/pkg/config" "github.com/containers/podman/v4/cmd/podman/registry" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/machine" @@ -91,24 +90,5 @@ func rm(_ *cobra.Command, args []string) error { return err } newMachineEvent(events.Remove, events.Event{Name: vmName}) - err = updateDefaultMachineInConfig(vmName) - if err != nil { - return fmt.Errorf("failed to update default machine: %v", err) - } return nil } - -func updateDefaultMachineInConfig(vmName string) error { - cfg, err := config.ReadCustomConfig() - if err != nil { - return err - } - if cfg.Engine.ActiveService == vmName { - cfg.Engine.ActiveService = "" - for machine := range cfg.Engine.ServiceDestinations { - cfg.Engine.ActiveService = machine - break - } - } - return cfg.Write() -} diff --git a/pkg/machine/connection.go b/pkg/machine/connection.go index 93c638cc7d..15e4fbef02 100644 --- a/pkg/machine/connection.go +++ b/pkg/machine/connection.go @@ -65,15 +65,25 @@ func ChangeDefault(name string) error { return cfg.Write() } -func RemoveConnection(name string) error { +func RemoveConnections(names ...string) error { cfg, err := config.ReadCustomConfig() if err != nil { return err } - if _, ok := cfg.Engine.ServiceDestinations[name]; ok { - delete(cfg.Engine.ServiceDestinations, name) - } else { - return fmt.Errorf("unable to find connection named %q", name) + for _, name := range names { + if _, ok := cfg.Engine.ServiceDestinations[name]; ok { + delete(cfg.Engine.ServiceDestinations, name) + } else { + return fmt.Errorf("unable to find connection named %q", name) + } + + if cfg.Engine.ActiveService == name { + cfg.Engine.ActiveService = "" + for service := range cfg.Engine.ServiceDestinations { + cfg.Engine.ActiveService = service + break + } + } } return cfg.Write() } diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 653e2653a1..687e854bf5 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -358,10 +358,7 @@ func (m *HyperVMachine) Remove(_ string, opts machine.RemoveOptions) (string, fu logrus.Error(err) } } - if err := machine.RemoveConnection(m.Name); err != nil { - logrus.Error(err) - } - if err := machine.RemoveConnection(m.Name + "-root"); err != nil { + if err := machine.RemoveConnections(m.Name, m.Name+"-root"); err != nil { logrus.Error(err) } diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 273bbdd380..93fa49a715 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -994,10 +994,7 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() logrus.Error(err) } } - if err := machine.RemoveConnection(v.Name); err != nil { - logrus.Error(err) - } - if err := machine.RemoveConnection(v.Name + "-root"); err != nil { + if err := machine.RemoveConnections(v.Name, v.Name+"-root"); err != nil { logrus.Error(err) } return nil diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index db14af074a..28afbb6551 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -1368,10 +1368,7 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun confirmationMessage += "\n" return confirmationMessage, func() error { - if err := machine.RemoveConnection(v.Name); err != nil { - logrus.Error(err) - } - if err := machine.RemoveConnection(v.Name + "-root"); err != nil { + if err := machine.RemoveConnections(v.Name, v.Name+"-root"); err != nil { logrus.Error(err) } if err := runCmdPassThrough("wsl", "--unregister", toDist(v.Name)); err != nil {