Merge pull request #25722 from l0rd/fix-hyperv-volumes-with-space

Fix machines failing to start when a volume's path contains spaces

Author: openshift-merge-bot[bot]

pkg/machine/e2e/init_test.go

@@ -299,9 +299,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("", "")
@@ -312,9 +309,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))
 
@@ -323,6 +327,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() {

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 = `

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

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"

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 '\'':