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

Improve cross platform support of QEMU machine #21252

Closed
wants to merge 1 commit into from
Closed
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
9 changes: 9 additions & 0 deletions pkg/machine/e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,12 @@ Note: To run specific test files, add the test files to the end of the winmake c
1. `export CONTAINERS_MACHINE_PROVIDER="applehv"`
1. `export MACHINE_IMAGE="https://fedorapeople.org/groups/podman/testing/applehv/arm64/fedora-coreos-38.20230925.dev.0-applehv.aarch64.raw.gz"`
1. `make localmachine` (Add `FOCUS_FILE=basic_test.go` to only run basic test)

### QEMU

1. Install Podman and QEMU for MacOS bundle using latest release from https://github.com/containers/podman/releases
1. `make podman-remote`
1. `export CONTAINERS_MACHINE_PROVIDER="qemu"`
1. Add bundled QEMU to path `export PATH=/opt/podman/qemu/bin:$PATH`
1. Set search path to gvproxy from bundle `export CONTAINERS_HELPER_BINARY_DIR=/opt/podman/bin`
1. `make localmachine` (Add `FOCUS_FILE=basic_test.go` to only run basic test)
4 changes: 2 additions & 2 deletions pkg/machine/machine_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func StopWinProxy(name string, vmtype define.VMType) error {
if err != nil {
return nil
}
sendQuit(tid)
SendQuit(tid)
_ = waitTimeout(proc, 20*time.Second)
_ = os.Remove(tidFile)

Expand Down Expand Up @@ -200,7 +200,7 @@ func waitTimeout(proc *os.Process, timeout time.Duration) bool {
return ret
}

func sendQuit(tid uint32) {
func SendQuit(tid uint32) {
user32 := syscall.NewLazyDLL("user32.dll")
postMessage := user32.NewProc("PostThreadMessageW")
postMessage.Call(uintptr(tid), WM_QUIT, 0, 0)
Expand Down
1 change: 0 additions & 1 deletion pkg/machine/qemu/command/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func (q *QemuCmd) SetNetwork() {
*q = append(*q, "-netdev", "socket,id=vlan,fd=3", "-device", "virtio-net-pci,netdev=vlan,mac=5a:94:ef:e4:0c:ee")
}

// SetNetwork adds a network device to the machine
func (q *QemuCmd) SetUSBHostPassthrough(usbs []USBConfig) {
if len(usbs) == 0 {
return
Expand Down
81 changes: 49 additions & 32 deletions pkg/machine/qemu/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ const (
MountType9p = "9p"
dockerSock = "/var/run/docker.sock"
dockerConnectTimeout = 5 * time.Second
retryCountForStop = 5
windowsFallbackUID = 501
baseBackoff = 500 * time.Millisecond
maxStartupBackoffs = 6
)

type MachineVM struct {
Expand Down Expand Up @@ -146,6 +150,9 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
}

v.UID = os.Getuid()
if v.UID == -1 {
v.UID = windowsFallbackUID // Used on Windows to match FCOS image
}

// Add location of bootable image
v.CmdLine.SetBootableImage(v.getImageFile())
Expand Down Expand Up @@ -422,7 +429,7 @@ func (v *MachineVM) qemuPid() (int, error) {
logrus.Warnf("Reading QEMU pidfile: %v", err)
return -1, nil
}
return findProcess(pid)
return pingProcess(pid)
}

// Start executes the qemu command line and forks it
Expand All @@ -433,9 +440,6 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
qemuSocketConn net.Conn
)

defaultBackoff := 500 * time.Millisecond
maxBackoffs := 6

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

Expand Down Expand Up @@ -489,7 +493,7 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
logrus.Errorf("machine %q is incompatible with this release of podman and needs to be recreated, starting for recovery only", v.Name)
}

forwardSock, forwardState, err := v.startHostNetworking()
forwardSock, forwardState, _, err := v.startHostNetworking(&v.QMPMonitor.Address)
if err != nil {
return fmt.Errorf("unable to start host networking: %q", err)
}
Expand All @@ -513,7 +517,7 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
return err
}

qemuSocketConn, err = sockets.DialSocketWithBackoffs(maxBackoffs, defaultBackoff, v.QMPMonitor.Address.Path)
qemuSocketConn, err = sockets.DialSocketWithBackoffs(maxStartupBackoffs, baseBackoff, v.QMPMonitor.Address.Path)
if err != nil {
return fmt.Errorf("failed to connect to qemu monitor socket: %w", err)
}
Expand All @@ -532,9 +536,6 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
defer dnr.Close()
defer dnw.Close()

attr := new(os.ProcAttr)
files := []*os.File{dnr, dnw, dnw, fd}
attr.Files = files
cmdLine := v.CmdLine

cmdLine.SetPropagatedHostEnvs()
Expand All @@ -551,12 +552,15 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {

// actually run the command that starts the virtual machine
cmd := &exec.Cmd{
Args: cmdLine,
Path: cmdLine[0],
Stdin: dnr,
Stdout: dnw,
Stderr: stderrBuf,
ExtraFiles: []*os.File{fd},
Args: cmdLine,
Path: cmdLine[0],
Stdin: dnr,
Stdout: dnw,
Stderr: stderrBuf,
}
// Forward FD if one was allocated
if fd != nil {
cmd.ExtraFiles = []*os.File{fd}
}

if err := runStartVMCommand(cmd); err != nil {
Expand All @@ -569,7 +573,7 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
fmt.Println("Waiting for VM ...")
}

conn, err = sockets.DialSocketWithBackoffsAndProcCheck(maxBackoffs, defaultBackoff, v.ReadySocket.GetPath(), checkProcessStatus, "qemu", cmd.Process.Pid, stderrBuf)
conn, err = sockets.DialSocketWithBackoffsAndProcCheck(maxStartupBackoffs, baseBackoff, v.ReadySocket.GetPath(), checkProcessStatus, "qemu", cmd.Process.Pid, stderrBuf)
if err != nil {
return err
}
Expand Down Expand Up @@ -603,7 +607,7 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
return nil
}

connected, sshError, err := v.conductVMReadinessCheck(name, maxBackoffs, defaultBackoff)
connected, sshError, err := v.conductVMReadinessCheck(name, maxStartupBackoffs, baseBackoff)
if err != nil {
return err
}
Expand Down Expand Up @@ -677,7 +681,7 @@ func (v *MachineVM) checkStatus(monitor *qmp.SocketMonitor) (define.Status, erro
func (v *MachineVM) waitForMachineToStop() error {
fmt.Println("Waiting for VM to stop running...")
waitInternal := 250 * time.Millisecond
for i := 0; i < 5; i++ {
for i := 0; i < retryCountForStop; i++ {
state, err := v.State(false)
if err != nil {
return err
Expand Down Expand Up @@ -710,15 +714,15 @@ func (v *MachineVM) ProxyPID() (int, error) {
return proxyPid, nil
}

// cleanupVMProxyProcess kills the proxy process and removes the VM's pidfile
func (v *MachineVM) cleanupVMProxyProcess(proxyProc *os.Process) error {
// cleanupVMProxyProcess kills the proxy process
func (v *MachineVM) cleanupVMProxyProcess(proxyPid int) error {
// Kill the process
if err := proxyProc.Kill(); err != nil {
if err := killProcess(proxyPid, false); err != nil {
return err
}
// Remove the pidfile
if err := v.PidFilePath.Delete(); err != nil {
return err
logrus.Debugf("Error while removing proxy pidfile: %v", err)
}
return nil
}
Expand Down Expand Up @@ -762,7 +766,7 @@ func (v *MachineVM) Stop(_ string, _ machine.StopOptions) error {
return stopErr
}

if err := sigKill(qemuPid); err != nil {
if err := killProcess(qemuPid, true); err != nil {
if stopErr == nil {
return err
}
Expand Down Expand Up @@ -837,7 +841,7 @@ func (v *MachineVM) stopLocked() error {
return err
}

if err := v.cleanupVMProxyProcess(proxyProc); err != nil {
if err := v.cleanupVMProxyProcess(proxyPid); err != nil {
return err
}

Expand Down Expand Up @@ -869,8 +873,18 @@ func (v *MachineVM) stopLocked() error {
}

fmt.Println("Waiting for VM to exit...")
for isProcessAlive(vmPid) {
time.Sleep(500 * time.Millisecond)
retries := 60
for {
alive, _ := isProcessAlive(vmPid)
if retries <= 0 {
logrus.Warning("Giving up on waiting for VM to exit. VM process might still terminate")
break
}
if !alive {
break
}
time.Sleep(baseBackoff)
retries--
}

return nil
Expand Down Expand Up @@ -1101,18 +1115,18 @@ func getDiskSize(path string) (uint64, error) {

// startHostNetworking runs a binary on the host system that allows users
// to set up port forwarding to the podman virtual machine
func (v *MachineVM) startHostNetworking() (string, machine.APIForwardingState, error) {
func (v *MachineVM) startHostNetworking(vlanSocket *define.VMFile) (string, machine.APIForwardingState, *os.Process, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we passing in the qmp socket into the function if we already have access to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have implementation, which supports UNIX socket VLAN from newer QEMU QAPI, which is compatible also with Windows OS supporting AF_UNIX. In this case it is impossible to use hack with reusing QMP address and need to provide dedicated VLAN socket address as we need both in the same command line arixmkii@9e3e20f#diff-4a3677ed5171641eba40e911f19bbf5970550a9e1ce75a30959e6c8032b2e39cR524

cfg, err := config.Default()
if err != nil {
return "", machine.NoForwarding, err
return "", machine.NoForwarding, nil, err
}
binary, err := cfg.FindHelperBinary(machine.ForwarderBinaryName, false)
if err != nil {
return "", machine.NoForwarding, err
return "", machine.NoForwarding, nil, err
}

cmd := gvproxy.NewGvproxyCommand()
cmd.AddQemuSocket(fmt.Sprintf("unix://%s", v.QMPMonitor.Address.GetPath()))
cmd.AddQemuSocket(fmt.Sprintf("unix://%s", filepath.ToSlash(vlanSocket.GetPath())))
cmd.PidFile = v.PidFilePath.GetPath()
cmd.SSHPort = v.Port

Expand All @@ -1127,11 +1141,13 @@ func (v *MachineVM) startHostNetworking() (string, machine.APIForwardingState, e
logrus.Debug(cmd)
}

cargs := cmd.ToCmdline()
logrus.Debugf("gvproxy cmd: %v", append([]string{binary}, cargs...))
c := cmd.Cmd(binary)
if err := c.Start(); err != nil {
return "", 0, fmt.Errorf("unable to execute: %q: %w", cmd.ToCmdline(), err)
return "", 0, nil, fmt.Errorf("unable to execute: %q: %w", cmd.ToCmdline(), err)
}
return forwardSock, state, nil
return forwardSock, state, c.Process, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the decision behind returning the os.Process? it currently isn't being used by the caller

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have implementation based of UNIX socket VLAN based which checks the pid of this process to understand if we have the process alive arixmkii@9e3e20f#diff-4a3677ed5171641eba40e911f19bbf5970550a9e1ce75a30959e6c8032b2e39cR542

}

func (v *MachineVM) setupAPIForwarding(cmd gvproxy.GvproxyCommand) (gvproxy.GvproxyCommand, string, machine.APIForwardingState) {
Expand Down Expand Up @@ -1325,6 +1341,7 @@ func (v *MachineVM) Inspect() (*machine.InspectInfo, error) {
return nil, err
}
connInfo.PodmanSocket = podmanSocket
connInfo.PodmanPipe = podmanPipe(v.Name)
return &machine.InspectInfo{
ConfigPath: v.ConfigPath,
ConnectionInfo: *connInfo,
Expand Down
45 changes: 27 additions & 18 deletions pkg/machine/qemu/machine_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,41 @@ import (
"strings"
"syscall"

"github.com/containers/podman/v4/pkg/machine/define"
"golang.org/x/sys/unix"
)

func isProcessAlive(pid int) bool {
func isProcessAlive(pid int) (bool, error) {
err := unix.Kill(pid, syscall.Signal(0))
if err == nil || err == unix.EPERM {
return true
return true, nil
}
return false
return false, err
}

func pingProcess(pid int) (int, error) {
alive, err := isProcessAlive(pid)
if !alive {
if err == unix.ESRCH {
return -1, nil
}
return -1, fmt.Errorf("pinging QEMU process: %w", err)
}
return pid, nil
}

func killProcess(pid int, force bool) error {
if force {
return unix.Kill(pid, unix.SIGKILL)
}
return unix.Kill(pid, unix.SIGTERM)
}

func checkProcessStatus(processHint string, pid int, stderrBuf *bytes.Buffer) error {
var status syscall.WaitStatus
pid, err := syscall.Wait4(pid, &status, syscall.WNOHANG, nil)
if err != nil {
return fmt.Errorf("failed to read qem%su process status: %w", processHint, err)
return fmt.Errorf("failed to read %s process status: %w", processHint, err)
}
if pid > 0 {
// child exited
Expand All @@ -32,6 +51,10 @@ func checkProcessStatus(processHint string, pid int, stderrBuf *bytes.Buffer) er
return nil
}

func podmanPipe(name string) *define.VMFile {
return nil
}

func pathsFromVolume(volume string) []string {
return strings.SplitN(volume, ":", 3)
}
Expand All @@ -42,17 +65,3 @@ func extractTargetPath(paths []string) string {
}
return paths[0]
}

func sigKill(pid int) error {
return unix.Kill(pid, unix.SIGKILL)
}

func findProcess(pid int) (int, error) {
if err := unix.Kill(pid, 0); err != nil {
if err == unix.ESRCH {
return -1, nil
}
return -1, fmt.Errorf("pinging QEMU process: %w", err)
}
return pid, nil
}
34 changes: 27 additions & 7 deletions pkg/machine/qemu/machine_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,27 @@ import (
"strings"

"github.com/containers/podman/v4/pkg/machine"
"github.com/containers/podman/v4/pkg/machine/define"
)

func isProcessAlive(pid int) bool {
func isProcessAlive(pid int) (bool, error) {
if checkProcessStatus("process", pid, nil) == nil {
return true
return true, nil
}
return false
return false, nil
}

func pingProcess(pid int) (int, error) {
alive, _ := isProcessAlive(pid)
if !alive {
return -1, nil
}
return pid, nil
}

func killProcess(pid int, force bool) error {
machine.SendQuit(uint32(pid))
return nil
}

func checkProcessStatus(processHint string, pid int, stderrBuf *bytes.Buffer) error {
Expand Down Expand Up @@ -51,10 +65,16 @@ func extractTargetPath(paths []string) string {
return dedup.ReplaceAllLiteralString("/"+target, "/")
}

func sigKill(pid int) error {
return nil
func podmanPipe(name string) *define.VMFile {
return &define.VMFile{Path: `\\.\pipe\` + toPipeName(name)}
}

func findProcess(pid int) (int, error) {
return -1, nil
func toPipeName(name string) string {
if !strings.HasPrefix(name, "qemu-podman") {
if !strings.HasPrefix(name, "podman") {
name = "podman-" + name
}
name = "qemu-" + name
}
return name
}