mantle/kola: Add COSA_VIRTIOFS=1 and dual 9p/virtiofs support

In coreos#3428 I tried
a wholesale switch to virtiofs.  There are some kernel/KVM crashes
we haven't yet debugged.

It wasn't hard to factor things out so that it becomes a dynamic
choice, keeping the previous 9p support.

This way for e.g. local development one can now
`env COSA_VIRTIOFS=1 cosa run --qemu-image rhcos.qcow2 --bind-ro ...`

Author: cgwalters

mantle/cmd/kola/qemuexec.go

@@ -91,8 +91,8 @@ func init() {
 	cmdQemuExec.Flags().BoolVarP(&devshellConsole, "devshell-console", "c", false, "Connect directly to serial console in devshell mode")
 	cmdQemuExec.Flags().StringVarP(&ignition, "ignition", "i", "", "Path to Ignition config")
 	cmdQemuExec.Flags().StringVarP(&butane, "butane", "B", "", "Path to Butane config")
-	cmdQemuExec.Flags().StringArrayVar(&bindro, "bind-ro", nil, "Mount readonly via 9pfs a host directory (use --bind-ro=/path/to/host,/var/mnt/guest")
-	cmdQemuExec.Flags().StringArrayVar(&bindrw, "bind-rw", nil, "Same as above, but writable")
+	cmdQemuExec.Flags().StringArrayVar(&bindro, "bind-ro", nil, "Mount $hostpath,$guestpath readonly; for example --bind-ro=/path/on/host,/var/mnt/guest)")
+	cmdQemuExec.Flags().StringArrayVar(&bindrw, "bind-rw", nil, "Mount $hostpath,$guestpath writable; for example --bind-rw=/path/on/host,/var/mnt/guest)")
 	cmdQemuExec.Flags().BoolVarP(&forceConfigInjection, "inject-ignition", "", false, "Force injecting Ignition config using guestfs")
 	cmdQemuExec.Flags().BoolVar(&propagateInitramfsFailure, "propagate-initramfs-failure", false, "Error out if the system fails in the initramfs")
 	cmdQemuExec.Flags().StringVarP(&consoleFile, "console-to-file", "", "", "Filepath in which to save serial console logs")
@@ -296,18 +296,18 @@ func runQemuExec(cmd *cobra.Command, args []string) error {
 		if err != nil {
 			return err
 		}
-		builder.Mount9p(src, dest, true)
+		builder.MountHost(src, dest, true)
 		ensureConfig()
-		config.Mount9p(dest, true)
+		config.MountHost(dest, builder.UseVirtiofs, true)
 	}
 	for _, b := range bindrw {
 		src, dest, err := parseBindOpt(b)
 		if err != nil {
 			return err
 		}
-		builder.Mount9p(src, dest, false)
+		builder.MountHost(src, dest, false)
 		ensureConfig()
-		config.Mount9p(dest, false)
+		config.MountHost(dest, builder.UseVirtiofs, false)
 	}
 	builder.ForceConfigInjection = forceConfigInjection
 	if len(firstbootkargs) > 0 {

mantle/platform/conf/conf.go

@@ -1362,11 +1362,22 @@ resize_terminal() {
 PROMPT_COMMAND+=(resize_terminal)`, 0644)
 }
 
-// Mount9p adds an Ignition config to mount an folder with 9p
-func (c *Conf) Mount9p(dest string, readonly bool) {
-	readonlyStr := ""
-	if readonly {
-		readonlyStr = ",ro"
+// MountHost adds an Ignition config to mount an folder
+func (c *Conf) MountHost(dest string, virtiofs, readonly bool) {
+	mountType := "virtiofs"
+	if !virtiofs {
+		mountType = "9p"
+	}
+	options := ""
+	if virtiofs {
+		if readonly {
+			options = "ro"
+		}
+	} else {
+		options = "trans=virtio,version=9p2000.L,msize=10485760"
+		if readonly {
+			options += ",ro"
+		}
 	}
 	content := fmt.Sprintf(`[Unit]
 DefaultDependencies=no
@@ -1375,11 +1386,11 @@ Before=basic.target
 [Mount]
 What=%s
 Where=%s
-Type=9p
-Options=trans=virtio,version=9p2000.L%s,msize=10485760
+Type=%s
+Options=%s
 [Install]
 WantedBy=multi-user.target
-`, dest, dest, readonlyStr)
+`, dest, dest, mountType, options)
 	c.AddSystemdUnit(fmt.Sprintf("%s.mount", systemdunit.UnitNameEscape(dest[1:])), content, Enable)
 }
 

mantle/platform/qemu.go

@@ -17,7 +17,7 @@
 // Why not libvirt?
 // Two main reasons.  First, we really do want to use qemu, and not
 // something else.  We rely on qemu features/APIs and there's a general
-// assumption that the qemu process is local (e.g. we expose 9p filesystem
+// assumption that the qemu process is local (e.g. we expose 9p/virtiofs filesystem
 // sharing).  Second, libvirt runs as a daemon, but we want the
 // VMs "lifecycle bound" to their creating process (e.g. kola),
 // so that e.g. Ctrl-C (SIGINT) kills both reliably.
@@ -145,11 +145,12 @@ type bootIso struct {
 
 // QemuInstance holds an instantiated VM through its lifecycle.
 type QemuInstance struct {
-	qemu               exec.Cmd
-	architecture       string
-	tempdir            string
-	swtpm              exec.Cmd
-	nbdServers         []exec.Cmd
+	qemu         exec.Cmd
+	architecture string
+	tempdir      string
+	swtpm        exec.Cmd
+	// Helpers are child processes such as nbd or virtiofsd that should be lifecycle bound to qemu
+	helpers            []exec.Cmd
 	hostForwardedPorts []HostForwardPort
 
 	journalPipe *os.File
@@ -287,12 +288,12 @@ func (inst *QemuInstance) Destroy() {
 		inst.swtpm.Kill() //nolint // Ignore errors
 		inst.swtpm = nil
 	}
-	for _, nbdServ := range inst.nbdServers {
-		if nbdServ != nil {
-			nbdServ.Kill() //nolint // Ignore errors
+	for _, p := range inst.helpers {
+		if p != nil {
+			p.Kill() //nolint // Ignore errors
 		}
 	}
-	inst.nbdServers = nil
+	inst.helpers = nil
 
 	if inst.tempdir != "" {
 		if err := os.RemoveAll(inst.tempdir); err != nil {
@@ -421,6 +422,13 @@ func (inst *QemuInstance) RemovePrimaryBlockDevice() (err2 error) {
 	return nil
 }
 
+// A directory mounted from the host into the guest, via 9p or virtiofs
+type HostMount struct {
+	src      string
+	dest     string
+	readonly bool
+}
+
 // QemuBuilder is a configurator that can then create a qemu instance
 type QemuBuilder struct {
 	// ConfigFile is a path to Ignition configuration
@@ -469,6 +477,8 @@ type QemuBuilder struct {
 	ignitionSet      bool
 	ignitionRendered bool
 
+	// UseVirtiofs should be true if mounts use virtiofs (default: 9p)
+	UseVirtiofs               bool
 	UsermodeNetworking        bool
 	RestrictNetworking        bool
 	requestedHostForwardPorts []HostForwardPort
@@ -477,9 +487,12 @@ type QemuBuilder struct {
 	finalized bool
 	diskID    uint
 	disks     []*Disk
-	fs9pID    uint
 	// virtioSerialID is incremented for each device
 	virtioSerialID uint
+	// hostMounts is an array of directories mounted (via 9p or virtiofs) from the host
+	hostMounts []HostMount
+	// hostMountSerialID is incremented for each device
+	hostMountSerialID uint
 	// fds is file descriptors we own to pass to qemu
 	fds []*os.File
 
@@ -499,12 +512,14 @@ func NewQemuBuilder() *QemuBuilder {
 	default:
 		defaultFirmware = ""
 	}
+	_, useVirtiofs := os.LookupEnv("COSA_VIRTIOFS")
 	ret := QemuBuilder{
 		Firmware:     defaultFirmware,
 		Swtpm:        true,
 		Pdeathsig:    true,
 		Argv:         []string{},
 		architecture: coreosarch.CurrentRpmArch(),
+		UseVirtiofs:  useVirtiofs,
 	}
 	return &ret
 }
@@ -702,16 +717,10 @@ func (builder *QemuBuilder) encryptIgnitionConfig() error {
 	return nil
 }
 
-// Mount9p sets up a mount point from the host to guest.  To be replaced
-// with https://virtio-fs.gitlab.io/ once it lands everywhere.
-func (builder *QemuBuilder) Mount9p(source, destHint string, readonly bool) {
-	builder.fs9pID++
-	readonlyStr := ""
-	if readonly {
-		readonlyStr = ",readonly=on"
-	}
-	builder.Append("--fsdev", fmt.Sprintf("local,id=fs%d,path=%s,security_model=mapped%s", builder.fs9pID, source, readonlyStr))
-	builder.Append("-device", virtio(builder.architecture, "9p", fmt.Sprintf("fsdev=fs%d,mount_tag=%s", builder.fs9pID, destHint)))
+// MountVirtiofs sets up a mount point from the host to guest.
+// Note that virtiofs does not currently support read-only mounts (which is really surprising!)
+func (builder *QemuBuilder) MountHost(source, dest string, readonly bool) {
+	builder.hostMounts = append(builder.hostMounts, HostMount{src: source, dest: dest, readonly: readonly})
 }
 
 // supportsFwCfg if the target system supports injecting
@@ -1691,7 +1700,7 @@ func (builder *QemuBuilder) Exec() (*QemuInstance, error) {
 			if err := cmd.Start(); err != nil {
 				return nil, errors.Wrapf(err, "spawing nbd server")
 			}
-			inst.nbdServers = append(inst.nbdServers, cmd)
+			inst.helpers = append(inst.helpers, cmd)
 		}
 	}
 
@@ -1771,6 +1780,85 @@ func (builder *QemuBuilder) Exec() (*QemuInstance, error) {
 		return nil, err
 	}
 
+	// Process 9p/virtiofs mounts
+	if len(builder.hostMounts) > 0 && builder.UseVirtiofs {
+		if err := builder.ensureTempdir(); err != nil {
+			return nil, err
+		}
+
+		plog.Debug("creating viritofs helpers")
+
+		// Allocate a shared memory object and set up NUMA, which is needed for virtiofs
+		builder.Append("-object", fmt.Sprintf("memory-backend-file,id=mem,size=%dM,mem-path=/dev/shm,share=on", builder.Memory), "-numa", "node,memdev=mem")
+
+		// We assume we're already running in a container
+		isolation := "none"
+
+		// Spawn off a virtiofsd helper per mounted path
+		virtiofsHelpers := make(map[string]exec.Cmd)
+		for i, virtiofs := range builder.hostMounts {
+			// By far the most common failure to spawn virtiofsd will be a typo'd source directory,
+			// so let's synchronously check that ourselves here.
+			if _, err := os.Stat(virtiofs.src); err != nil {
+				return nil, fmt.Errorf("Failed to access virtiofs source directory %s", virtiofs.src)
+			}
+			virtiofsChar := fmt.Sprintf("virtiofschar%d", i)
+			virtiofsdSocket := filepath.Join(builder.tempdir, fmt.Sprintf("virtiofsd-%d.sock", i))
+			builder.Append("-chardev", fmt.Sprintf("socket,id=%s,path=%s", virtiofsChar, virtiofsdSocket))
+			builder.Append("-device", fmt.Sprintf("vhost-user-fs-pci,queue-size=1024,chardev=%s,tag=%s", virtiofsChar, virtiofs.dest))
+			plog.Debugf("creating viritofs helper for %s", virtiofs.src)
+			// TODO: Honor virtiofs.readonly somehow here
+			p := exec.Command("/usr/libexec/virtiofsd", "--sandbox", isolation, "--socket-path", virtiofsdSocket, "--shared-dir", virtiofs.src)
+			// Quiet the daemon by default
+			p.Env = append(p.Env, "RUST_LOG=ERROR")
+			p.Stderr = os.Stderr
+			p.SysProcAttr = &syscall.SysProcAttr{
+				Pdeathsig: syscall.SIGTERM,
+			}
+			if err := p.Start(); err != nil {
+				return nil, fmt.Errorf("Failed to start virtiofsd")
+			}
+			virtiofsHelpers[virtiofsdSocket] = p
+		}
+		// Loop waiting for the sockets to appear
+		delay := time.Millisecond * 250
+		repetitions := time.Minute / delay
+		for i := 0; i < int(repetitions) && len(virtiofsHelpers) > 0; i++ {
+			// Delay between subsequent loops
+			if i > 0 {
+				time.Sleep(delay)
+			}
+			found := []string{}
+			for sockpath := range virtiofsHelpers {
+				if _, err := os.Stat(sockpath); err == nil {
+					found = append(found, sockpath)
+				}
+			}
+			for _, sockpath := range found {
+				delete(virtiofsHelpers, sockpath)
+			}
+		}
+		if len(virtiofsHelpers) > 0 {
+			sockets := ""
+			for sockpath := range virtiofsHelpers {
+				if len(sockets) > 0 {
+					sockets += " "
+				}
+				sockets += sockpath
+			}
+			return nil, fmt.Errorf("Failed to wait for virtiofsd sockets: %s", sockets)
+		}
+	} else if len(builder.hostMounts) > 0 {
+		for i, hostmnt := range builder.hostMounts {
+			readonlyStr := ""
+			if hostmnt.readonly {
+				readonlyStr = ",readonly=on"
+			}
+			builder.Append("--fsdev", fmt.Sprintf("local,id=fs%d,path=%s,security_model=mapped%s", i, hostmnt.src, readonlyStr))
+			builder.Append("-device", virtio(builder.architecture, "9p", fmt.Sprintf("fsdev=fs%d,mount_tag=%s", i, hostmnt.dest)))
+		}
+	}
+
 	fdnum := 3 // first additional file starts at position 3
 	for i := range builder.fds {
 		fdset := i + 1 // Start at 1