Merge pull request #21517 from jakecorrenti/fix-qemu-todos

machine: Address some QEMU TODOs

cmd/podman/machine/inspect.go

@@ -67,10 +67,6 @@ func inspect(cmd *cobra.Command, args []string) error {
 		if err != nil {
 			return err
 		}
-		ignFile, err := mc.IgnitionFile()
-		if err != nil {
-			return err
-		}
 
 		podmanSocket, podmanPipe, err := mc.ConnectionInfo(provider.VMType())
 		if err != nil {
@@ -83,13 +79,7 @@ func inspect(cmd *cobra.Command, args []string) error {
 				PodmanSocket: podmanSocket,
 				PodmanPipe:   podmanPipe,
 			},
-			Created: mc.Created,
-			// TODO This is no longer applicable; we dont care about the provenance
-			// of the image
-			Image: machine.ImageConfig{
-				IgnitionFile: *ignFile,
-				ImagePath:    *mc.ImagePath,
-			},
+			Created:            mc.Created,
 			LastUp:             mc.LastUp,
 			Name:               mc.Name,
 			Resources:          mc.Resources,

cmd/podman/machine/rm.go

@@ -120,14 +120,12 @@ func rm(_ *cobra.Command, args []string) error {
 	// All actual removal of files and vms should occur after this
 	//
 
-	// TODO Should this be a hard error?
 	if err := providerRm(); err != nil {
-		logrus.Errorf("failed to remove virtual machine from provider for %q", vmName)
+		logrus.Errorf("failed to remove virtual machine from provider for %q: %v", vmName, err)
 	}
 
-	// TODO Should this be a hard error?
 	if err := genericRm(); err != nil {
-		logrus.Error("failed to remove machines files")
+		return fmt.Errorf("failed to remove machines files: %v", err)
 	}
 	newMachineEvent(events.Remove, events.Event{Name: vmName})
 	return nil

docs/source/markdown/podman-machine-inspect.1.md

@@ -28,7 +28,6 @@ Print results with a Go template.
 | .ConfigDir ...      | Machine configuration directory location                                   |
 | .ConnectionInfo ... | Machine connection information                                        |
 | .Created ...        | Machine creation time (string, ISO3601)                               |
-| .Image ...          | Machine image config                                                  |
 | .LastUp ...         | Time when machine was last booted                                     |
 | .Name               | Name of the machine                                                   |
 | .Resources ...      | Resources used by the machine                                         |

pkg/machine/config.go

@@ -110,7 +110,6 @@ type InspectInfo struct {
 	ConfigDir          define.VMFile
 	ConnectionInfo     ConnectionConfig
 	Created            time.Time
-	Image              ImageConfig
 	LastUp             time.Time
 	Name               string
 	Resources          vmconfigs.ResourceConfig

pkg/machine/e2e/init_test.go

@@ -287,16 +287,6 @@ var _ = Describe("podman machine init", func() {
 		Expect(err).ToNot(HaveOccurred())
 		cfgpth := filepath.Join(inspectSession.outputToString(), fmt.Sprintf("%s.json", name))
 
-		inspect = inspect.withFormat("{{.Image.IgnitionFile.Path}}")
-		inspectSession, err = mb.setCmd(inspect).run()
-		Expect(err).ToNot(HaveOccurred())
-		ign := inspectSession.outputToString()
-
-		inspect = inspect.withFormat("{{.Image.ImagePath.Path}}")
-		inspectSession, err = mb.setCmd(inspect).run()
-		Expect(err).ToNot(HaveOccurred())
-		img := inspectSession.outputToString()
-
 		rm := rmMachine{}
 		removeSession, err := mb.setCmd(rm.withForce()).run()
 		Expect(err).ToNot(HaveOccurred())
@@ -317,13 +307,17 @@ var _ = Describe("podman machine init", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(session).To(Exit(125))
 
-			// ensure files created by init are cleaned up on init failure
-			_, err = os.Stat(img)
+			imageSuffix := mb.imagePath[strings.LastIndex(mb.imagePath, "/")+1:]
+			imgPath := filepath.Join(testDir, ".local", "share", "containers", "podman", "machine", "qemu", mb.name+"_"+imageSuffix)
+			_, err = os.Stat(imgPath)
 			Expect(err).To(HaveOccurred())
+
+			cfgDir := filepath.Join(testDir, ".config", "containers", "podman", "machine", testProvider.VMType().String())
 			_, err = os.Stat(cfgpth)
 			Expect(err).To(HaveOccurred())
 
-			_, err = os.Stat(ign)
+			ignPath := filepath.Join(cfgDir, mb.name+".ign")
+			_, err = os.Stat(ignPath)
 			Expect(err).To(HaveOccurred())
 		}
 	})

pkg/machine/e2e/rm_test.go

@@ -94,16 +94,6 @@ var _ = Describe("podman machine rm", func() {
 		key := inspectSession.outputToString()
 		pubkey := key + ".pub"
 
-		inspect = inspect.withFormat("{{.Image.IgnitionFile.Path}}")
-		inspectSession, err = mb.setCmd(inspect).run()
-		Expect(err).ToNot(HaveOccurred())
-		ign := inspectSession.outputToString()
-
-		inspect = inspect.withFormat("{{.Image.ImagePath.Path}}")
-		inspectSession, err = mb.setCmd(inspect).run()
-		Expect(err).ToNot(HaveOccurred())
-		img := inspectSession.outputToString()
-
 		rm := rmMachine{}
 		removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage()).run()
 		Expect(err).ToNot(HaveOccurred())
@@ -122,10 +112,11 @@ var _ = Describe("podman machine rm", func() {
 
 		// WSL does not use ignition
 		if testProvider.VMType() != define.WSLVirt {
-			_, err = os.Stat(ign)
+			ignPath := filepath.Join(testDir, ".config", "containers", "podman", "machine", testProvider.VMType().String(), mb.name+".ign")
+			_, err = os.Stat(ignPath)
 			Expect(err).ToNot(HaveOccurred())
 		}
-		_, err = os.Stat(img)
+		_, err = os.Stat(mb.imagePath)
 		Expect(err).ToNot(HaveOccurred())
 	})
 

pkg/machine/qemu/machine.go

@@ -16,6 +16,7 @@ import (
 	"time"
 
 	"github.com/containers/common/pkg/config"
+	"github.com/containers/podman/v5/pkg/errorhandling"
 	"github.com/containers/podman/v5/pkg/machine/define"
 	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
 	"github.com/digitalocean/go-qemu/qmp"
@@ -237,7 +238,15 @@ func (q *QEMUStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() erro
 	}
 
 	return qemuRmFiles, func() error {
-		return nil
+		var errs []error
+		if err := mc.QEMUHypervisor.QEMUPidPath.Delete(); err != nil {
+			errs = append(errs, err)
+		}
+
+		if err := mc.QEMUHypervisor.QMPMonitor.Address.Delete(); err != nil {
+			errs = append(errs, err)
+		}
+		return errorhandling.JoinErrors(errs)
 	}, nil
 }
 

pkg/machine/vmconfigs/machine.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	define2 "github.com/containers/podman/v5/libpod/define"
+	"github.com/containers/podman/v5/pkg/errorhandling"
 	"github.com/containers/podman/v5/pkg/machine/connection"
 	"github.com/containers/podman/v5/pkg/machine/define"
 	"github.com/containers/podman/v5/pkg/machine/lock"
@@ -184,28 +185,32 @@ func (mc *MachineConfig) Remove(saveIgnition, saveImage bool) ([]string, func()
 	}
 
 	mcRemove := func() error {
+		var errs []error
+		if err := connection.RemoveConnections(mc.Name, mc.Name+"-root"); err != nil {
+			errs = append(errs, err)
+		}
+
 		if !saveIgnition {
 			if err := ignitionFile.Delete(); err != nil {
-				logrus.Error(err)
+				errs = append(errs, err)
 			}
 		}
 		if !saveImage {
 			if err := mc.ImagePath.Delete(); err != nil {
-				logrus.Error(err)
+				errs = append(errs, err)
 			}
 		}
-		if err := mc.configPath.Delete(); err != nil {
-			logrus.Error(err)
-		}
 		if err := readySocket.Delete(); err != nil {
-			logrus.Error()
+			errs = append(errs, err)
 		}
 		if err := logPath.Delete(); err != nil {
-			logrus.Error(err)
+			errs = append(errs, err)
+		}
+
+		if err := mc.configPath.Delete(); err != nil {
+			errs = append(errs, err)
 		}
-		// TODO This should be bumped up into delete and called out in the text given then
-		// are not technically files per'se
-		return connection.RemoveConnections(mc.Name, mc.Name+"-root")
+		return errorhandling.JoinErrors(errs)
 	}
 
 	return rmFiles, mcRemove, nil