From a06df4fc1aca86ddc4007f4957b53509e8d39e19 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Tue, 29 Mar 2022 15:04:35 -0500 Subject: Machine refactor part 2 This PR further implements a more structured approach to handling the files needed by machine. More files are now made as MachineFile which can then have a symlink (using a shorter path) to them. Also added Set and Get methods for many of the files. The next part of the refactor will implement the use of symlinks on MacOS. Signed-off-by: Brent Baude [NO NEW TESTS NEEDED] Signed-off-by: Brent Baude --- pkg/machine/qemu/config.go | 19 ++- pkg/machine/qemu/machine.go | 269 +++++++++++++++++++++---------- pkg/machine/qemu/options_darwin_arm64.go | 6 +- 3 files changed, 201 insertions(+), 93 deletions(-) (limited to 'pkg') diff --git a/pkg/machine/qemu/config.go b/pkg/machine/qemu/config.go index 05a1d74d3..3d1032fba 100644 --- a/pkg/machine/qemu/config.go +++ b/pkg/machine/qemu/config.go @@ -5,6 +5,7 @@ package qemu import ( "errors" + "io/ioutil" "os" "time" @@ -59,6 +60,8 @@ type MachineVMV1 struct { } type MachineVM struct { + // ConfigPath is the path to the configuration file + ConfigPath MachineFile // The command line representation of the qemu command CmdLine []string // HostUser contains info about host user @@ -83,11 +86,11 @@ type MachineVM struct { // ImageConfig describes the bootable image for the VM type ImageConfig struct { - IgnitionFilePath string + IgnitionFilePath MachineFile // ImageStream is the update stream for the image ImageStream string // ImagePath is the fq path to - ImagePath string + ImagePath MachineFile } // HostUser describes the host user @@ -171,11 +174,19 @@ func (m *MachineFile) GetPath() string { // the actual path func (m *MachineFile) Delete() error { if m.Symlink != nil { - if err := os.Remove(*m.Symlink); err != nil { + if err := os.Remove(*m.Symlink); err != nil && !errors.Is(err, os.ErrNotExist) { logrus.Errorf("unable to remove symlink %q", *m.Symlink) } } - return os.Remove(m.Path) + if err := os.Remove(m.Path); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + return nil +} + +// Read the contents of a given file and return in []bytes +func (m *MachineFile) Read() ([]byte, error) { + return ioutil.ReadFile(m.GetPath()) } // NewMachineFile is a constructor for MachineFile diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 07155bbcf..65980129d 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -71,10 +71,17 @@ func (p *Provider) NewMachine(opts machine.InitOptions) (machine.VM, error) { if len(opts.Name) > 0 { vm.Name = opts.Name } - ignitionFile := filepath.Join(vmConfigDir, vm.Name+".ign") - vm.IgnitionFilePath = ignitionFile + ignitionFile, err := NewMachineFile(filepath.Join(vmConfigDir, vm.Name+".ign"), nil) + if err != nil { + return nil, err + } + vm.IgnitionFilePath = *ignitionFile - vm.ImagePath = opts.ImagePath + imagePath, err := NewMachineFile(opts.ImagePath, nil) + if err != nil { + return nil, err + } + vm.ImagePath = *imagePath vm.RemoteUsername = opts.Username // Add a random port for ssh @@ -104,7 +111,7 @@ func (p *Provider) NewMachine(opts machine.InitOptions) (machine.VM, error) { // Add cpus cmd = append(cmd, []string{"-smp", strconv.Itoa(int(vm.CPUs))}...) // Add ignition file - cmd = append(cmd, []string{"-fw_cfg", "name=opt/com.coreos/config,file=" + vm.IgnitionFilePath}...) + cmd = append(cmd, []string{"-fw_cfg", "name=opt/com.coreos/config,file=" + vm.IgnitionFilePath.GetPath()}...) // Add qmp socket monitor, err := NewQMPMonitor("unix", vm.Name, defaultQMPTimeout) if err != nil { @@ -117,17 +124,19 @@ func (p *Provider) NewMachine(opts machine.InitOptions) (machine.VM, error) { // Right now the mac address is hardcoded so that the host networking gives it a specific IP address. This is // why we can only run one vm at a time right now cmd = append(cmd, []string{"-netdev", "socket,id=vlan,fd=3", "-device", "virtio-net-pci,netdev=vlan,mac=5a:94:ef:e4:0c:ee"}...) - socketPath, err := getRuntimeDir() - if err != nil { + if err := vm.setReadySocket(); err != nil { return nil, err } - virtualSocketPath := filepath.Join(socketPath, "podman", vm.Name+"_ready.sock") + // Add serial port for readiness cmd = append(cmd, []string{ "-device", "virtio-serial", - "-chardev", "socket,path=" + virtualSocketPath + ",server=on,wait=off,id=" + vm.Name + "_ready", + "-chardev", "socket,path=" + vm.getReadySocket() + ",server=on,wait=off,id=" + vm.Name + "_ready", "-device", "virtserialport,chardev=" + vm.Name + "_ready" + ",name=org.fedoraproject.port.0"}...) vm.CmdLine = cmd + if err := vm.setQEMUAndPIDSocket(); err != nil { + return nil, err + } return vm, nil } @@ -165,12 +174,26 @@ func migrateVM(configPath string, config []byte, vm *MachineVM) error { vm.ResourceConfig = ResourceConfig{} vm.SSHConfig = SSHConfig{} + ignitionFilePath, err := NewMachineFile(old.IgnitionFilePath, nil) + if err != nil { + return err + } + imagePath, err := NewMachineFile(old.ImagePath, nil) + if err != nil { + return err + } + + // setReadySocket will stick the entry into the new struct + if err := vm.setReadySocket(); err != nil { + return err + } + vm.CPUs = old.CPUs vm.CmdLine = old.CmdLine vm.DiskSize = old.DiskSize vm.IdentityPath = old.IdentityPath - vm.IgnitionFilePath = old.IgnitionFilePath - vm.ImagePath = old.ImagePath + vm.IgnitionFilePath = *ignitionFilePath + vm.ImagePath = *imagePath vm.ImageStream = old.ImageStream vm.Memory = old.Memory vm.Mounts = old.Mounts @@ -200,31 +223,15 @@ func migrateVM(configPath string, config []byte, vm *MachineVM) error { return os.Remove(configPath + ".orig") } -// LoadByName reads a json file that describes a known qemu vm +// LoadVMByName reads a json file that describes a known qemu vm // and returns a vm instance func (p *Provider) LoadVMByName(name string) (machine.VM, error) { vm := &MachineVM{Name: name} vm.HostUser = HostUser{UID: -1} // posix reserves -1, so use it to signify undefined - vmConfigDir, err := machine.GetConfDir(vmtype) - if err != nil { + if err := vm.update(); err != nil { return nil, err } - path := filepath.Join(vmConfigDir, name+".json") - b, err := ioutil.ReadFile(path) - if os.IsNotExist(err) { - return nil, errors.Wrap(machine.ErrNoSuchVM, name) - } - if err != nil { - return nil, err - } - err = json.Unmarshal(b, vm) - if err != nil { - migrateErr := migrateVM(path, b, vm) - if migrateErr != nil { - return nil, migrateErr - } - err = migrateErr - } + // It is here for providing the ability to propagate // proxy settings (e.g. HTTP_PROXY and others) on a start // and avoid a need of re-creating/re-initiating a VM @@ -239,7 +246,7 @@ func (p *Provider) LoadVMByName(name string) (machine.VM, error) { } logrus.Debug(vm.CmdLine) - return vm, err + return vm, nil } // Init writes the json configuration file to the filesystem for @@ -261,7 +268,11 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { if err != nil { return false, err } - v.ImagePath = dd.Get().LocalUncompressedFile + uncompressedFile, err := NewMachineFile(dd.Get().LocalUncompressedFile, nil) + if err != nil { + return false, err + } + v.ImagePath = *uncompressedFile if err := machine.DownloadImage(dd); err != nil { return false, err } @@ -273,14 +284,17 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { if err != nil { return false, err } - v.ImagePath = g.Get().LocalUncompressedFile + imagePath, err := NewMachineFile(g.Get().LocalUncompressedFile, nil) + if err != nil { + return false, err + } + v.ImagePath = *imagePath if err := machine.DownloadImage(g); err != nil { return false, err } } // Add arch specific options including image location v.CmdLine = append(v.CmdLine, v.addArchOptions()...) - var volumeType string switch opts.VolumeDriver { case "virtfs": @@ -330,7 +344,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { v.UID = os.Getuid() // Add location of bootable image - v.CmdLine = append(v.CmdLine, "-drive", "if=virtio,file="+v.ImagePath) + v.CmdLine = append(v.CmdLine, "-drive", "if=virtio,file="+v.getImageFile()) // This kind of stinks but no other way around this r/n if len(opts.IgnitionPath) < 1 { uri := machine.SSHRemoteConnection.MakeSSHURL("localhost", fmt.Sprintf("/run/user/%d/podman/podman.sock", v.UID), strconv.Itoa(v.Port), v.RemoteUsername) @@ -373,7 +387,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { return false, err } - originalDiskSize, err := getDiskSize(v.ImagePath) + originalDiskSize, err := getDiskSize(v.getImageFile()) if err != nil { return false, err } @@ -390,7 +404,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { if err != nil { return false, err } - resize := exec.Command(resizePath, []string{"resize", v.ImagePath, strconv.Itoa(int(opts.DiskSize)) + "G"}...) + resize := exec.Command(resizePath, []string{"resize", v.getImageFile(), strconv.Itoa(int(opts.DiskSize)) + "G"}...) resize.Stdout = os.Stdout resize.Stderr = os.Stderr if err := resize.Run(); err != nil { @@ -404,7 +418,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { if err != nil { return false, err } - return false, ioutil.WriteFile(v.IgnitionFilePath, inputIgnition, 0644) + return false, ioutil.WriteFile(v.getIgnitionFile(), inputIgnition, 0644) } // Write the ignition file ign := machine.DynamicIgnition{ @@ -412,14 +426,14 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) { Key: key, VMName: v.Name, TimeZone: opts.TimeZone, - WritePath: v.IgnitionFilePath, + WritePath: v.getIgnitionFile(), UID: v.UID, } err = machine.NewIgnitionFile(ign) return err == nil, err } -func (v *MachineVM) Set(name string, opts machine.SetOptions) error { +func (v *MachineVM) Set(_ string, opts machine.SetOptions) error { if v.Rootful == opts.Rootful { return nil } @@ -473,17 +487,16 @@ func (v *MachineVM) Start(name string, _ machine.StartOptions) error { return err } } - qemuSocketPath, _, err := v.getSocketandPid() - if err != nil { + if err := v.setQEMUAndPIDSocket(); err != nil { return err } // If the qemusocketpath exists and the vm is off/down, we should rm // it before the dial as to avoid a segv - if err := os.Remove(qemuSocketPath); err != nil && !errors.Is(err, os.ErrNotExist) { - logrus.Warn(err) + if err := v.QMPMonitor.Address.Delete(); err != nil { + return err } for i := 0; i < 6; i++ { - qemuSocketConn, err = net.Dial("unix", qemuSocketPath) + qemuSocketConn, err = net.Dial("unix", v.QMPMonitor.Address.GetPath()) if err == nil { break } @@ -650,7 +663,7 @@ func (v *MachineVM) checkStatus(monitor *qmp.SocketMonitor) (machine.QemuMachine } // Stop uses the qmp monitor to call a system_powerdown -func (v *MachineVM) Stop(name string, _ machine.StopOptions) error { +func (v *MachineVM) Stop(_ string, _ machine.StopOptions) error { var disconnected bool // check if the qmp socket is there. if not, qemu instance is gone if _, err := os.Stat(v.QMPMonitor.Address.GetPath()); os.IsNotExist(err) { @@ -687,14 +700,10 @@ func (v *MachineVM) Stop(name string, _ machine.StopOptions) error { return err } - qemuSocketFile, pidFile, err := v.getSocketandPid() - if err != nil { - return err - } - if _, err := os.Stat(pidFile); os.IsNotExist(err) { + if _, err := os.Stat(v.getPidFile()); os.IsNotExist(err) { return nil } - pidString, err := ioutil.ReadFile(pidFile) + pidString, err := v.PidFilePath.Read() if err != nil { return err } @@ -712,11 +721,11 @@ func (v *MachineVM) Stop(name string, _ machine.StopOptions) error { return err } // Remove the pidfile - if err := os.Remove(pidFile); err != nil && !errors.Is(err, os.ErrNotExist) { - logrus.Warn(err) + if err := v.PidFilePath.Delete(); err != nil { + return err } // Remove socket - if err := os.Remove(qemuSocketFile); err != nil { + if err := v.QMPMonitor.Address.Delete(); err != nil { return err } @@ -774,7 +783,7 @@ func NewQMPMonitor(network, name string, timeout time.Duration) (Monitor, error) } // Remove deletes all the files associated with a machine including ssh keys, the image itself -func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, func() error, error) { +func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() error, error) { var ( files []string ) @@ -793,10 +802,10 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun files = append(files, v.IdentityPath, v.IdentityPath+".pub") } if !opts.SaveIgnition { - files = append(files, v.IgnitionFilePath) + files = append(files, v.getIgnitionFile()) } if !opts.SaveImage { - files = append(files, v.ImagePath) + files = append(files, v.getImageFile()) } socketPath, err := v.getForwardSocketPath() if err != nil { @@ -822,19 +831,15 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun confirmationMessage += msg + "\n" } - // Get path to socket and pidFile before we do any cleanups - qemuSocketFile, pidFile, errSocketFile := v.getSocketandPid() //silently try to delete socket and pid file //remove socket and pid file if any: warn at low priority if things fail - if errSocketFile == nil { - // Remove the pidfile - if err := os.Remove(pidFile); err != nil && !errors.Is(err, os.ErrNotExist) { - logrus.Debugf("Error while removing pidfile: %v", err) - } - // Remove socket - if err := os.Remove(qemuSocketFile); err != nil && !errors.Is(err, os.ErrNotExist) { - logrus.Debugf("Error while removing podman-machine-socket: %v", err) - } + // Remove the pidfile + if err := v.PidFilePath.Delete(); err != nil { + logrus.Debugf("Error while removing pidfile: %v", err) + } + // Remove socket + if err := v.QMPMonitor.Address.Delete(); err != nil { + logrus.Debugf("Error while removing podman-machine-socket: %v", err) } confirmationMessage += "\n" @@ -890,7 +895,7 @@ func (v *MachineVM) isListening() bool { // SSH opens an interactive SSH session to the vm specified. // Added ssh function to VM interface: pkg/machine/config/go : line 58 -func (v *MachineVM) SSH(name string, opts machine.SSHOptions) error { +func (v *MachineVM) SSH(_ string, opts machine.SSHOptions) error { running, err := v.isRunning() if err != nil { return err @@ -966,10 +971,10 @@ func getDiskSize(path string) (uint64, error) { // List lists all vm's that use qemu virtualization func (p *Provider) List(_ machine.ListOptions) ([]*machine.ListResponse, error) { - return GetVMInfos() + return getVMInfos() } -func GetVMInfos() ([]*machine.ListResponse, error) { +func getVMInfos() ([]*machine.ListResponse, error) { vmConfigDir, err := machine.GetConfDir(vmtype) if err != nil { return nil, err @@ -1011,7 +1016,7 @@ func GetVMInfos() ([]*machine.ListResponse, error) { } listEntry.CreatedAt = fi.ModTime() - fi, err = os.Stat(vm.ImagePath) + fi, err = os.Stat(vm.getImageFile()) if err != nil { return err } @@ -1034,7 +1039,7 @@ func GetVMInfos() ([]*machine.ListResponse, error) { } func (p *Provider) IsValidVMName(name string) (bool, error) { - infos, err := GetVMInfos() + infos, err := getVMInfos() if err != nil { return false, err } @@ -1049,7 +1054,7 @@ func (p *Provider) IsValidVMName(name string) (bool, error) { // CheckExclusiveActiveVM checks if there is a VM already running // that does not allow other VMs to be running func (p *Provider) CheckExclusiveActiveVM() (bool, string, error) { - vms, err := GetVMInfos() + vms, err := getVMInfos() if err != nil { return false, "", errors.Wrap(err, "error checking VM active") } @@ -1073,16 +1078,12 @@ func (v *MachineVM) startHostNetworking() (string, apiForwardingState, error) { return "", noForwarding, err } - qemuSocket, pidFile, err := v.getSocketandPid() - if err != nil { - return "", noForwarding, err - } attr := new(os.ProcAttr) // Pass on stdin, stdout, stderr files := []*os.File{os.Stdin, os.Stdout, os.Stderr} attr.Files = files cmd := []string{binary} - cmd = append(cmd, []string{"-listen-qemu", fmt.Sprintf("unix://%s", qemuSocket), "-pid-file", pidFile}...) + cmd = append(cmd, []string{"-listen-qemu", fmt.Sprintf("unix://%s", v.getQMPMonitorSocket()), "-pid-file", v.getPidFile()}...) // Add the ssh port cmd = append(cmd, []string{"-ssh-port", fmt.Sprintf("%d", v.Port)}...) @@ -1171,6 +1172,52 @@ func (v *MachineVM) getForwardSocketPath() (string, error) { return filepath.Join(path, "podman.sock"), nil } +func (v *MachineVM) setConfigPath() error { + vmConfigDir, err := machine.GetConfDir(vmtype) + if err != nil { + return err + } + + configPath, err := NewMachineFile(filepath.Join(vmConfigDir, v.Name)+".json", nil) + if err != nil { + return err + } + v.ConfigPath = *configPath + return nil +} + +func (v *MachineVM) setReadySocket() error { + rtPath, err := getRuntimeDir() + if err != nil { + return err + } + virtualSocketPath, err := NewMachineFile(filepath.Join(rtPath, "podman", v.Name+"_ready.sock"), nil) + if err != nil { + return err + } + v.ReadySocket = *virtualSocketPath + return nil +} + +func (v *MachineVM) setQEMUAndPIDSocket() error { + rtPath, err := getRuntimeDir() + if err != nil { + return err + } + if !rootless.IsRootless() { + rtPath = "/run" + } + socketDir := filepath.Join(rtPath, "podman") + pidFilePath, err := NewMachineFile(filepath.Join(socketDir, fmt.Sprintf("%s.pid", v.Name)), nil) + if err != nil { + return err + } + v.PidFilePath = *pidFilePath + return nil +} + +// Deprecated: getSocketandPid is being replace by setQEMUAndPIDSocket and +// machinefiles. func (v *MachineVM) getSocketandPid() (string, string, error) { rtPath, err := getRuntimeDir() if err != nil { @@ -1287,23 +1334,73 @@ func (v *MachineVM) waitAPIAndPrintInfo(forwardState apiForwardingState, forward } } -func (v *MachineVM) writeConfig() error { - // GetConfDir creates the directory so no need to check for - // its existence - vmConfigDir, err := machine.GetConfDir(vmtype) +// update returns the content of the VM's +// configuration file in json +func (v *MachineVM) update() error { + if err := v.setConfigPath(); err != nil { + return err + } + b, err := v.ConfigPath.Read() if err != nil { + if os.IsNotExist(err) { + return errors.Wrap(machine.ErrNoSuchVM, v.Name) + } return err } + if err != nil { + return err + } + err = json.Unmarshal(b, v) + if err != nil { + err = migrateVM(v.ConfigPath.GetPath(), b, v) + if err != nil { + return err + } + } + return err +} - jsonFile := filepath.Join(vmConfigDir, v.Name) + ".json" +func (v *MachineVM) writeConfig() error { + // Set the path of the configfile before writing to make + // life easier down the line + if err := v.setConfigPath(); err != nil { + return err + } // Write the JSON file b, err := json.MarshalIndent(v, "", " ") if err != nil { return err } - if err := ioutil.WriteFile(jsonFile, b, 0644); err != nil { + if err := ioutil.WriteFile(v.ConfigPath.GetPath(), b, 0644); err != nil { return err } - return nil } + +// getPidFile gets the file where the machine pid is stored +func (v *MachineVM) getPidFile() string { + return v.PidFilePath.GetPath() +} + +// getQMPMonitorSocket gets the socket used by qemu to interact +// with the instance +func (v *MachineVM) getQMPMonitorSocket() string { + return v.QMPMonitor.Address.GetPath() +} + +// getReadySocket returns the socket used to communicate +// with the machinevm and report when it is booted +func (v *MachineVM) getReadySocket() string { + return v.ReadySocket.GetPath() +} + +// getImageFile returns the path to the image used +// to boot the VM +func (v *MachineVM) getImageFile() string { + return v.ImagePath.GetPath() +} + +// getIgnitionFile returns the path to the ignition file +func (v *MachineVM) getIgnitionFile() string { + return v.IgnitionFilePath.GetPath() +} diff --git a/pkg/machine/qemu/options_darwin_arm64.go b/pkg/machine/qemu/options_darwin_arm64.go index 5b6cdc86d..4c954af00 100644 --- a/pkg/machine/qemu/options_darwin_arm64.go +++ b/pkg/machine/qemu/options_darwin_arm64.go @@ -11,7 +11,7 @@ var ( ) func (v *MachineVM) addArchOptions() []string { - ovmfDir := getOvmfDir(v.ImagePath, v.Name) + ovmfDir := getOvmfDir(v.ImagePath.GetPath(), v.Name) opts := []string{ "-accel", "hvf", "-accel", "tcg", @@ -23,13 +23,13 @@ func (v *MachineVM) addArchOptions() []string { } func (v *MachineVM) prepare() error { - ovmfDir := getOvmfDir(v.ImagePath, v.Name) + ovmfDir := getOvmfDir(v.ImagePath.GetPath(), v.Name) cmd := []string{"/bin/dd", "if=/dev/zero", "conv=sync", "bs=1m", "count=64", "of=" + ovmfDir} return exec.Command(cmd[0], cmd[1:]...).Run() } func (v *MachineVM) archRemovalFiles() []string { - ovmDir := getOvmfDir(v.ImagePath, v.Name) + ovmDir := getOvmfDir(v.ImagePath.GetPath(), v.Name) return []string{ovmDir} } -- cgit v1.2.3-54-g00ecf