From 565a5452a1e2b1ed1d3f04091be34f5bf96c2e8b Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Wed, 19 Sep 2018 18:33:47 +0200 Subject: vm/vmm: don't start separate process for console vmctl console fails from time to time with: vmctl: console not found Probably there is some race (most of these things assume that there is a human typing commands with delays). Also, vmctl start can connect to console itself with -c flag. So use that because it both solves the console race and also makes code much more similar to other VM implementations (qemu, gvisor). This also eliminates 3 additional goroutines per VM. --- vm/vmm/vmm.go | 158 ++++++++++++++++++++++------------------------------------ 1 file changed, 60 insertions(+), 98 deletions(-) (limited to 'vm/vmm') diff --git a/vm/vmm/vmm.go b/vm/vmm/vmm.go index 128f15604..05387c5f1 100644 --- a/vm/vmm/vmm.go +++ b/vm/vmm/vmm.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" "regexp" "strings" @@ -37,19 +38,17 @@ type Pool struct { type instance struct { cfg *Config - index int image string debug bool os string - workdir string sshkey string sshuser string sshhost string sshport int merger *vmimpl.OutputMerger vmName string - stop chan bool - diagnose chan string + vmm *exec.Cmd + consolew io.WriteCloser } var ipRegex = regexp.MustCompile(`bound to (([0-9]+\.){3}3)`) @@ -100,21 +99,22 @@ func (pool *Pool) Create(workdir string, index int) (vmimpl.Instance, error) { return nil, err } - name := fmt.Sprintf("%v-%v", pool.env.Name, index) + var tee io.Writer + if pool.env.Debug { + tee = os.Stdout + } inst := &instance{ - cfg: pool.cfg, - index: index, - image: image, - debug: pool.env.Debug, - os: pool.env.OS, - workdir: workdir, - sshkey: pool.env.SSHKey, - sshuser: pool.env.SSHUser, - sshport: 22, - vmName: name, - stop: make(chan bool), - diagnose: make(chan string), + cfg: pool.cfg, + image: image, + debug: pool.env.Debug, + os: pool.env.OS, + sshkey: pool.env.SSHKey, + sshuser: pool.env.SSHUser, + sshport: 22, + vmName: fmt.Sprintf("%v-%v", pool.env.Name, index), + merger: vmimpl.NewOutputMerger(tee), } + inst.vmctl("stop", inst.vmName, "-f", "-w") // in case it's still running from the previous run closeInst := inst defer func() { @@ -132,30 +132,46 @@ func (pool *Pool) Create(workdir string, index int) (vmimpl.Instance, error) { } func (inst *instance) Boot() error { - mem := fmt.Sprintf("%vM", inst.cfg.Mem) + outr, outw, err := osutil.LongPipe() + if err != nil { + return err + } + inr, inw, err := osutil.LongPipe() + if err != nil { + outr.Close() + outw.Close() + return err + } startArgs := []string{ "start", inst.vmName, "-b", inst.cfg.Kernel, "-d", inst.image, - "-m", mem, + "-m", fmt.Sprintf("%vM", inst.cfg.Mem), "-L", // add a local network interface + "-c", // connect to the console } if inst.cfg.Template != "" { startArgs = append(startArgs, "-t", inst.cfg.Template) } - if _, err := inst.vmctl(startArgs...); err != nil { - return err - } - - var tee io.Writer if inst.debug { - tee = os.Stdout + log.Logf(0, "running command: vmctl %#v", startArgs) } - inst.merger = vmimpl.NewOutputMerger(tee) - - if err := inst.console(); err != nil { + cmd := osutil.Command("vmctl", startArgs...) + cmd.Stdin = inr + cmd.Stdout = outw + cmd.Stderr = outw + if err := cmd.Start(); err != nil { + outr.Close() + outw.Close() + inr.Close() + inw.Close() return err } + inst.vmm = cmd + inst.consolew = inw + outw.Close() + inr.Close() + inst.merger.Add("console", outr) var bootOutput []byte bootOutputStop := make(chan bool) @@ -202,6 +218,14 @@ func (inst *instance) Boot() error { func (inst *instance) Close() { inst.vmctl("stop", inst.vmName, "-f") + if inst.consolew != nil { + inst.consolew.Close() + } + if inst.vmm != nil { + inst.vmm.Process.Kill() + inst.vmm.Wait() + } + inst.merger.Wait() } func (inst *instance) Forward(port int) (string, error) { @@ -264,95 +288,33 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin signal(vmimpl.ErrTimeout) case err := <-inst.merger.Err: cmd.Process.Kill() - inst.stop <- true - inst.merger.Wait() if cmdErr := cmd.Wait(); cmdErr == nil { // If the command exited successfully, we got EOF error from merger. // But in this case no error has happened and the EOF is expected. err = nil } - signal(err) return } cmd.Process.Kill() - inst.stop <- true - inst.merger.Wait() cmd.Wait() }() return inst.merger.Output, errc, nil } func (inst *instance) Diagnose() bool { - commands := []string{"", "trace", "show registers"} - for _, c := range commands { - select { - case inst.diagnose <- c: - case <-time.After(5 * time.Second): - } + // TODO(dvyukov): this does not work because console asks for login: + // OpenBSD/amd64 (syzkaller.my.domain) (tty00) + // login: trace + // Password: + // Login incorrect + for _, c := range []string{"\n", "trace\n", "show registers\n"} { + inst.consolew.Write([]byte(c)) + time.Sleep(1 * time.Second) } return true } -func (inst *instance) console() error { - outr, outw, err := osutil.LongPipe() - if err != nil { - return err - } - inr, inw, err := osutil.LongPipe() - if err != nil { - outr.Close() - outw.Close() - return err - } - - cmd := osutil.Command("vmctl", "console", inst.vmName) - cmd.Stdin = inr - cmd.Stdout = outw - cmd.Stderr = outw - if err := cmd.Start(); err != nil { - outr.Close() - outw.Close() - inr.Close() - inw.Close() - return err - } - outw.Close() - inr.Close() - inst.merger.Add("console", outr) - - go func() { - stopDiagnose := make(chan bool) - go func() { - for { - select { - case s := <-inst.diagnose: - inw.Write([]byte(s + "\n")) - time.Sleep(1 * time.Second) - case <-stopDiagnose: - return - } - } - }() - - stopProcess := make(chan bool) - go func() { - select { - case <-inst.stop: - cmd.Process.Kill() - case <-stopProcess: - } - }() - - _, err = cmd.Process.Wait() - inw.Close() - stopDiagnose <- true - stopProcess <- true - }() - - return nil -} - // Run the given vmctl(8) command and wait for it to finish. func (inst *instance) vmctl(args ...string) (string, error) { if inst.debug { -- cgit mrf-deployment