From 2588b9699392ebb77ba865dd97d36306f894fc20 Mon Sep 17 00:00:00 2001 From: Mario Loriedo Date: Fri, 28 Mar 2025 16:51:03 +0100 Subject: [PATCH 1/2] Fix logging podman machine server9 output Command `podman machine init` for Hyper-V machines invokes the command `podman machine server9` and redirects it's output to a file. But the file descriptor was closed before beeing used and the output file was always empty. Signed-off-by: Mario Loriedo --- pkg/machine/hyperv/stubber.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/machine/hyperv/stubber.go b/pkg/machine/hyperv/stubber.go index 13497e55b8..a2116a6984 100644 --- a/pkg/machine/hyperv/stubber.go +++ b/pkg/machine/hyperv/stubber.go @@ -420,10 +420,11 @@ func (h HyperVStubber) PostStartNetworking(mc *vmconfigs.MachineConfig, noInfo b fsCmd := exec.Command(executable, p9ServerArgs...) if logrus.IsLevelEnabled(logrus.DebugLevel) { - err = logCommandToFile(fsCmd, "podman-machine-server9.log") + log, err := logCommandToFile(fsCmd, "podman-machine-server9.log") if err != nil { return err } + defer log.Close() } err = fsCmd.Start() @@ -501,23 +502,22 @@ func removeIgnitionFromRegistry(vm *hypervctl.VirtualMachine) error { return nil } -func logCommandToFile(c *exec.Cmd, filename string) error { +func logCommandToFile(c *exec.Cmd, filename string) (*os.File, error) { dir, err := env.GetDataDir(define.HyperVVirt) if err != nil { - return fmt.Errorf("obtain machine dir: %w", err) + return nil, fmt.Errorf("obtain machine dir: %w", err) } path := filepath.Join(dir, filename) logrus.Infof("Going to log to %s", path) log, err := os.Create(path) if err != nil { - return fmt.Errorf("create log file: %w", err) + return nil, fmt.Errorf("create log file: %w", err) } - defer log.Close() c.Stdout = log c.Stderr = log - return nil + return log, nil } const hyperVVsockNMConnection = ` From 8e6ecb97c95b6c33b63f439eee61f7e30585b8d1 Mon Sep 17 00:00:00 2001 From: Mario Loriedo Date: Fri, 28 Mar 2025 16:22:28 +0100 Subject: [PATCH 2/2] Fix running machines with volumes containing spaces Machines configured to mount local paths containing spaces failed to start on Hyper-V and silently failed to mount the folder on macOS/Linux. On Windows/hyperv, where local paths are mounted running a 9p client inside the VM, the local host path needs to be surrounding with quotation marks before using in a `podman machine ssh ...` command. A similar behavior happened on Linux/QEMU where the path was used in a SSH command to mount the folder using virtiofs. Quoting the path when buidling the command arguments fixed the problem. On macOS/libkit,applehv the path was written as is in a systemd unit name to instruct how to mount it. Escaping space chars so that they are are parsed successfully fixed this: ```diff -- enable path with spaces.mount ++ enable path\x20with\x20spaces.mount ``` Fixes https://github.com/containers/podman/issues/25500 Signed-off-by: Mario Loriedo --- pkg/machine/e2e/init_test.go | 17 +++++++++++++---- pkg/machine/hyperv/volumes.go | 5 +++-- pkg/machine/qemu/stubber.go | 4 ++-- pkg/systemd/parser/split.go | 2 +- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index 12b9f4a2c0..314d5266ff 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -283,9 +283,6 @@ var _ = Describe("podman machine init", func() { }) It("machine init with volume", func() { - if testProvider.VMType() == define.HyperVVirt { - Skip("volumes are not supported on hyperv yet") - } skipIfWSL("WSL volumes are much different. This test will not pass as is") tmpDir, err := os.MkdirTemp("", "") @@ -296,9 +293,16 @@ var _ = Describe("podman machine init", func() { mount := tmpDir + ":/very-long-test-mount-dir-path-more-than-thirty-six-bytes" defer func() { _ = utils.GuardedRemoveAll(tmpDir) }() + tmpDirWithSpaces, err := os.MkdirTemp("", "with spaces") + Expect(err).ToNot(HaveOccurred()) + _, err = os.CreateTemp(tmpDirWithSpaces, "example") + Expect(err).ToNot(HaveOccurred()) + mountWithSpaces := tmpDirWithSpaces + ":/host folder" + defer func() { _ = utils.GuardedRemoveAll(tmpDirWithSpaces) }() + name := randomString() i := new(initMachine) - session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withVolume(mount).withNow()).run() + session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withVolume(mount).withVolume(mountWithSpaces).withNow()).run() Expect(err).ToNot(HaveOccurred()) Expect(session).To(Exit(0)) @@ -307,6 +311,11 @@ var _ = Describe("podman machine init", func() { Expect(err).ToNot(HaveOccurred()) Expect(sshSession).To(Exit(0)) Expect(sshSession.outputToString()).To(ContainSubstring("example")) + + sshSession, err = mb.setName(name).setCmd(ssh.withSSHCommand([]string{"ls \"/host folder\""})).run() + Expect(err).ToNot(HaveOccurred()) + Expect(sshSession).To(Exit(0)) + Expect(sshSession.outputToString()).To(ContainSubstring("example")) }) It("machine init with ignition path", func() { diff --git a/pkg/machine/hyperv/volumes.go b/pkg/machine/hyperv/volumes.go index ea94147dbd..08fefc4964 100644 --- a/pkg/machine/hyperv/volumes.go +++ b/pkg/machine/hyperv/volumes.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "path" + "strconv" "strings" "github.com/containers/podman/v5/pkg/machine" @@ -48,7 +49,7 @@ func startShares(mc *vmconfigs.MachineConfig) error { if requiresChattr { args = append(args, "sudo", "chattr", "-i", "/", "; ") } - args = append(args, "sudo", "mkdir", "-p", cleanTarget, "; ") + args = append(args, "sudo", "mkdir", "-p", strconv.Quote(cleanTarget), "; ") if requiresChattr { args = append(args, "sudo", "chattr", "+i", "/", "; ") } @@ -61,7 +62,7 @@ func startShares(mc *vmconfigs.MachineConfig) error { if mount.VSockNumber == nil { return errors.New("cannot start 9p shares with undefined vsock number") } - args = append(args, "machine", "client9p", fmt.Sprintf("%d", *mount.VSockNumber), mount.Target) + args = append(args, "machine", "client9p", fmt.Sprintf("%d", *mount.VSockNumber), strconv.Quote(mount.Target)) if err := machine.CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args); err != nil { return err diff --git a/pkg/machine/qemu/stubber.go b/pkg/machine/qemu/stubber.go index a1fee675b7..c8746efc3a 100644 --- a/pkg/machine/qemu/stubber.go +++ b/pkg/machine/qemu/stubber.go @@ -349,7 +349,7 @@ func (q *QEMUStubber) MountVolumesToVM(mc *vmconfigs.MachineConfig, quiet bool) if !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") { args = append(args, "sudo", "chattr", "-i", "/", ";") } - args = append(args, "sudo", "mkdir", "-p", mount.Target) + args = append(args, "sudo", "mkdir", "-p", strconv.Quote(mount.Target)) if !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") { args = append(args, ";", "sudo", "chattr", "+i", "/", ";") } @@ -362,7 +362,7 @@ func (q *QEMUStubber) MountVolumesToVM(mc *vmconfigs.MachineConfig, quiet bool) // in other words we don't want to make people unnecessarily reprovision their machines // to upgrade from 9p to virtiofs. mountOptions := []string{"-t", "virtiofs"} - mountOptions = append(mountOptions, []string{mount.Tag, mount.Target}...) + mountOptions = append(mountOptions, []string{mount.Tag, strconv.Quote(mount.Target)}...) mountFlags := fmt.Sprintf("context=\"%s\"", machine.NFSSELinuxContext) if mount.ReadOnly { mountFlags += ",ro" diff --git a/pkg/systemd/parser/split.go b/pkg/systemd/parser/split.go index b7acc695c1..cd2dfed221 100644 --- a/pkg/systemd/parser/split.go +++ b/pkg/systemd/parser/split.go @@ -473,7 +473,7 @@ func escapeString(escaped *strings.Builder, word string, isPath bool) { case '\\': escaped.WriteString("\\\\") case ' ': - escaped.WriteString(" ") + escaped.WriteString("\\x20") case '"': escaped.WriteString("\\\"") case '\'':