From 3bc0c4aa25c8a0d863fafe4329d4a52d07cb1d3c Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 5 Nov 2013 21:40:49 -0800
Subject: [PATCH] builder/qemu: simplify driver, make things more Go-like

---
 builder/qemu/builder.go                |  14 +-
 builder/qemu/driver.go                 | 203 +++++++++----------------
 builder/qemu/step_boot_wait.go         |  25 +++
 builder/qemu/step_run.go               | 127 ++++------------
 builder/qemu/step_shutdown.go          |  33 ++--
 builder/qemu/step_wait_for_shutdown.go |  43 ++++++
 6 files changed, 197 insertions(+), 248 deletions(-)
 create mode 100644 builder/qemu/step_boot_wait.go
 create mode 100644 builder/qemu/step_wait_for_shutdown.go

diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go
index 9a93efcd8..31d851963 100644
--- a/builder/qemu/builder.go
+++ b/builder/qemu/builder.go
@@ -372,7 +372,19 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		new(stepHTTPServer),
 		new(stepForwardSSH),
 		new(stepConfigureVNC),
-		new(stepRun),
+		&stepRun{
+			BootDrive: "d",
+			Message:   "Starting VM, booting from CD-ROM",
+		},
+		&stepBootWait{},
+		&stepTypeBootCommand{},
+		&stepWaitForShutdown{
+			Message: "Waiting for initial VM boot to shut down",
+		},
+		&stepRun{
+			BootDrive: "c",
+			Message:   "Starting VM, booting from hard disk",
+		},
 		&common.StepConnectSSH{
 			SSHAddress:     sshAddress,
 			SSHConfig:      sshConfig,
diff --git a/builder/qemu/driver.go b/builder/qemu/driver.go
index 858420e7e..7aa74f80f 100644
--- a/builder/qemu/driver.go
+++ b/builder/qemu/driver.go
@@ -3,7 +3,6 @@ package qemu
 import (
 	"bufio"
 	"bytes"
-	"errors"
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"io"
@@ -11,7 +10,8 @@ import (
 	"os/exec"
 	"regexp"
 	"strings"
-	"time"
+	"sync"
+	"syscall"
 	"unicode"
 )
 
@@ -25,21 +25,14 @@ type Driver interface {
 	//            qemuImgPath - string value for the qemu-img executable
 	Initialize(string, string)
 
-	// Checks if the VM with the given name is running.
-	IsRunning(string) (bool, error)
-
 	// Stop stops a running machine, forcefully.
-	Stop(string) error
+	Stop() error
 
 	// Qemu executes the given command via qemu-system-x86_64
-	Qemu(vmName string, qemuArgs ...string) error
+	Qemu(qemuArgs ...string) error
 
 	// wait on shutdown of the VM with option to cancel
-	WaitForShutdown(
-		vmName string,
-		block bool,
-		state multistep.StateBag,
-		cancellCallback DriverCancelCallback) error
+	WaitForShutdown(<-chan struct{}) bool
 
 	// Qemu executes the given command via qemu-img
 	QemuImg(...string) error
@@ -53,156 +46,108 @@ type Driver interface {
 	Version() (string, error)
 }
 
-type driverState struct {
-	cmd        *exec.Cmd
-	cancelChan chan struct{}
-	waitDone   chan error
-}
-
 type QemuDriver struct {
 	qemuPath    string
 	qemuImgPath string
-	state       map[string]*driverState
-}
 
-func (d *QemuDriver) getDriverState(name string) *driverState {
-	if _, ok := d.state[name]; !ok {
-		d.state[name] = &driverState{}
-	}
-	return d.state[name]
+	vmCmd   *exec.Cmd
+	vmEndCh <-chan int
+	lock    sync.Mutex
 }
 
 func (d *QemuDriver) Initialize(qemuPath string, qemuImgPath string) {
 	d.qemuPath = qemuPath
 	d.qemuImgPath = qemuImgPath
-	d.state = make(map[string]*driverState)
 }
 
-func (d *QemuDriver) IsRunning(name string) (bool, error) {
-	ds := d.getDriverState(name)
-	return ds.cancelChan != nil, nil
-}
+func (d *QemuDriver) Stop() error {
+	d.lock.Lock()
+	defer d.lock.Unlock()
 
-func (d *QemuDriver) Stop(name string) error {
-	ds := d.getDriverState(name)
-
-	// signal to the command 'wait' to kill the process
-	if ds.cancelChan != nil {
-		close(ds.cancelChan)
-		ds.cancelChan = nil
+	if d.vmCmd != nil {
+		if err := d.vmCmd.Process.Kill(); err != nil {
+			return err
+		}
 	}
+
 	return nil
 }
 
-func (d *QemuDriver) Qemu(vmName string, qemuArgs ...string) error {
+func (d *QemuDriver) Qemu(qemuArgs ...string) error {
+	d.lock.Lock()
+	defer d.lock.Unlock()
+
+	if d.vmCmd != nil {
+		panic("Existing VM state found")
+	}
+
 	stdout_r, stdout_w := io.Pipe()
 	stderr_r, stderr_w := io.Pipe()
 
 	log.Printf("Executing %s: %#v", d.qemuPath, qemuArgs)
-	ds := d.getDriverState(vmName)
-	ds.cmd = exec.Command(d.qemuPath, qemuArgs...)
-	ds.cmd.Stdout = stdout_w
-	ds.cmd.Stderr = stderr_w
+	cmd := exec.Command(d.qemuPath, qemuArgs...)
+	cmd.Stdout = stdout_w
+	cmd.Stderr = stderr_w
+
+	err := cmd.Start()
+	if err != nil {
+		err = fmt.Errorf("Error starting VM: %s", err)
+		return err
+	}
 
 	go logReader("Qemu stdout", stdout_r)
 	go logReader("Qemu stderr", stderr_r)
 
-	err := ds.cmd.Start()
+	log.Printf("Started Qemu. Pid: %d", cmd.Process.Pid)
 
-	if err != nil {
-		err = fmt.Errorf("Error starting VM: %s", err)
-	} else {
-		log.Printf("---- Started Qemu ------- PID = %d", ds.cmd.Process.Pid)
+	// Wait for Qemu to complete in the background, and mark when its done
+	endCh := make(chan int, 1)
+	go func() {
+		defer stderr_w.Close()
+		defer stdout_w.Close()
 
-		ds.cancelChan = make(chan struct{})
-
-		// make the channel to watch the process
-		ds.waitDone = make(chan error)
-
-		// start the virtual machine in the background
-		go func() {
-			defer stderr_w.Close()
-			defer stdout_w.Close()
-			ds.waitDone <- ds.cmd.Wait()
-		}()
-	}
-
-	return err
-}
-
-func (d *QemuDriver) WaitForShutdown(vmName string,
-	block bool,
-	state multistep.StateBag,
-	cancelCallback DriverCancelCallback) error {
-	var err error
-
-	ds := d.getDriverState(vmName)
-
-	if block {
-		// wait in the background for completion or caller cancel
-		for {
-			select {
-			case <-ds.cancelChan:
-				log.Println("Qemu process request to cancel -- killing Qemu process.")
-				if err = ds.cmd.Process.Kill(); err != nil {
-					log.Printf("Failed to kill qemu: %v", err)
-				}
-
-				// clear out the error channel since it's just a cancel
-				// and therefore the reason for failure is clear
-				log.Println("Empytying waitDone channel.")
-				<-ds.waitDone
-
-				// this gig is over -- assure calls to IsRunning see the nil
-				log.Println("'Nil'ing out cancelChan.")
-				ds.cancelChan = nil
-				return errors.New("WaitForShutdown cancelled")
-			case err = <-ds.waitDone:
-				log.Printf("Qemu Process done with output = %v", err)
-				// assure calls to IsRunning see the nil
-				log.Println("'Nil'ing out cancelChan.")
-				ds.cancelChan = nil
-				return nil
-			case <-time.After(1 * time.Second):
-				cancel := cancelCallback(state)
-				if cancel {
-					log.Println("Qemu process request to cancel -- killing Qemu process.")
-
-					// The step sequence was cancelled, so cancel waiting for SSH
-					// and just start the halting process.
-					close(ds.cancelChan)
-
-					log.Println("Cancel request made, quitting waiting for Qemu.")
-					return errors.New("WaitForShutdown cancelled by interrupt.")
+		var exitCode int = 0
+		if err := cmd.Wait(); err != nil {
+			if exiterr, ok := err.(*exec.ExitError); ok {
+				// The program has exited with an exit code != 0
+				if status, ok := exiterr.Sys().(syscall.WaitStatus); ok {
+					exitCode = status.ExitStatus()
+				} else {
+					exitCode = 254
 				}
 			}
 		}
-	} else {
-		go func() {
-			select {
-			case <-ds.cancelChan:
-				log.Println("Qemu process request to cancel -- killing Qemu process.")
-				if err = ds.cmd.Process.Kill(); err != nil {
-					log.Printf("Failed to kill qemu: %v", err)
-				}
 
-				// clear out the error channel since it's just a cancel
-				// and therefore the reason for failure is clear
-				log.Println("Empytying waitDone channel.")
-				<-ds.waitDone
-				log.Println("'Nil'ing out cancelChan.")
-				ds.cancelChan = nil
+		endCh <- exitCode
 
-			case err = <-ds.waitDone:
-				log.Printf("Qemu Process done with output = %v", err)
-				log.Println("'Nil'ing out cancelChan.")
-				ds.cancelChan = nil
-			}
-		}()
+		d.lock.Lock()
+		defer d.lock.Unlock()
+		d.vmCmd = nil
+		d.vmEndCh = nil
+	}()
+
+	// Setup our state so we know we are running
+	d.vmCmd = cmd
+	d.vmEndCh = endCh
+
+	return nil
+}
+
+func (d *QemuDriver) WaitForShutdown(cancelCh <-chan struct{}) bool {
+	d.lock.Lock()
+	endCh := d.vmEndCh
+	d.lock.Unlock()
+
+	if endCh == nil {
+		return true
 	}
 
-	ds.cancelChan = nil
-	return err
+	select {
+	case <-endCh:
+		return true
+	case <-cancelCh:
+		return false
+	}
 }
 
 func (d *QemuDriver) QemuImg(args ...string) error {
diff --git a/builder/qemu/step_boot_wait.go b/builder/qemu/step_boot_wait.go
new file mode 100644
index 000000000..46a48dcbc
--- /dev/null
+++ b/builder/qemu/step_boot_wait.go
@@ -0,0 +1,25 @@
+package qemu
+
+import (
+	"fmt"
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/packer"
+	"time"
+)
+
+// stepBootWait waits the configured time period.
+type stepBootWait struct{}
+
+func (s *stepBootWait) Run(state multistep.StateBag) multistep.StepAction {
+	config := state.Get("config").(*config)
+	ui := state.Get("ui").(packer.Ui)
+
+	if int64(config.bootWait) > 0 {
+		ui.Say(fmt.Sprintf("Waiting %s for boot...", config.bootWait))
+		time.Sleep(config.bootWait)
+	}
+
+	return multistep.ActionContinue
+}
+
+func (s *stepBootWait) Cleanup(state multistep.StateBag) {}
diff --git a/builder/qemu/step_run.go b/builder/qemu/step_run.go
index f79590f03..480d008fc 100644
--- a/builder/qemu/step_run.go
+++ b/builder/qemu/step_run.go
@@ -6,49 +6,51 @@ import (
 	"github.com/mitchellh/packer/packer"
 	"path/filepath"
 	"strings"
-	"time"
 )
 
+// stepRun runs the virtual machine
 type stepRun struct {
-	vmName string
+	BootDrive string
+	Message   string
 }
 
-func runBootCommand(state multistep.StateBag,
-	actionChannel chan multistep.StepAction) {
-	config := state.Get("config").(*config)
+func (s *stepRun) Run(state multistep.StateBag) multistep.StepAction {
+	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
-	bootCmd := stepTypeBootCommand{}
 
-	if int64(config.bootWait) > 0 {
-		ui.Say(fmt.Sprintf("Waiting %s for boot...", config.bootWait))
-		time.Sleep(config.bootWait)
+	ui.Say(s.Message)
+
+	command := getCommandArgs(s.BootDrive, state)
+	if err := driver.Qemu(command...); err != nil {
+		err := fmt.Errorf("Error launching VM: %s", err)
+		ui.Error(err.Error())
+		return multistep.ActionHalt
 	}
 
-	actionChannel <- bootCmd.Run(state)
+	return multistep.ActionContinue
 }
 
-func cancelCallback(state multistep.StateBag) bool {
-	cancel := false
-	if _, ok := state.GetOk(multistep.StateCancelled); ok {
-		cancel = true
-	}
-	return cancel
-}
-
-func (s *stepRun) getCommandArgs(
-	bootDrive string,
-	state multistep.StateBag) []string {
-
+func (s *stepRun) Cleanup(state multistep.StateBag) {
+	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
+
+	if err := driver.Stop(); err != nil {
+		ui.Error(fmt.Sprintf("Error shutting down VM: %s", err))
+	}
+}
+
+func getCommandArgs(bootDrive string, state multistep.StateBag) []string {
 	config := state.Get("config").(*config)
+	isoPath := state.Get("iso_path").(string)
+	vncPort := state.Get("vnc_port").(uint)
+	sshHostPort := state.Get("sshHostPort").(uint)
+	ui := state.Get("ui").(packer.Ui)
+
+	guiArgument := "sdl"
+	vnc := fmt.Sprintf("0.0.0.0:%d", vncPort-5900)
 	vmName := config.VMName
 	imgPath := filepath.Join(config.OutputDir,
 		fmt.Sprintf("%s.%s", vmName, strings.ToLower(config.Format)))
-	isoPath := state.Get("iso_path").(string)
-	vncPort := state.Get("vnc_port").(uint)
-	guiArgument := "sdl"
-	sshHostPort := state.Get("sshHostPort").(uint)
-	vnc := fmt.Sprintf("0.0.0.0:%d", vncPort-5900)
 
 	if config.Headless == true {
 		ui.Message("WARNING: The VM will be started in headless mode, as configured.\n" +
@@ -112,74 +114,3 @@ func (s *stepRun) getCommandArgs(
 
 	return outArgs
 }
-
-func (s *stepRun) runVM(
-	sendBootCommands bool,
-	bootDrive string,
-	state multistep.StateBag) multistep.StepAction {
-
-	config := state.Get("config").(*config)
-	driver := state.Get("driver").(Driver)
-	ui := state.Get("ui").(packer.Ui)
-	vmName := config.VMName
-
-	ui.Say("Starting the virtual machine for OS Install...")
-	command := s.getCommandArgs(bootDrive, state)
-	if err := driver.Qemu(vmName, command...); err != nil {
-		err := fmt.Errorf("Error launching VM: %s", err)
-		ui.Error(err.Error())
-		return multistep.ActionHalt
-	}
-
-	s.vmName = vmName
-
-	// run the boot command after its own timeout
-	if sendBootCommands {
-		waitDone := make(chan multistep.StepAction, 1)
-		go runBootCommand(state, waitDone)
-		select {
-		case action := <-waitDone:
-			if action != multistep.ActionContinue {
-				// stop the VM in its tracks
-				driver.Stop(vmName)
-				return multistep.ActionHalt
-			}
-		}
-	}
-
-	ui.Say("Waiting for VM to shutdown...")
-	if err := driver.WaitForShutdown(vmName, sendBootCommands, state, cancelCallback); err != nil {
-		err := fmt.Errorf("Error waiting for initial VM install to shutdown: %s", err)
-		ui.Error(err.Error())
-		return multistep.ActionHalt
-	}
-
-	return multistep.ActionContinue
-}
-
-func (s *stepRun) Run(state multistep.StateBag) multistep.StepAction {
-	// First, the OS install boot
-	action := s.runVM(true, "d", state)
-
-	if action == multistep.ActionContinue {
-		// Then the provisioning install
-		action = s.runVM(false, "c", state)
-	}
-
-	return action
-}
-
-func (s *stepRun) Cleanup(state multistep.StateBag) {
-	if s.vmName == "" {
-		return
-	}
-
-	driver := state.Get("driver").(Driver)
-	ui := state.Get("ui").(packer.Ui)
-
-	if running, _ := driver.IsRunning(s.vmName); running {
-		if err := driver.Stop(s.vmName); err != nil {
-			ui.Error(fmt.Sprintf("Error shutting down VM: %s", err))
-		}
-	}
-}
diff --git a/builder/qemu/step_shutdown.go b/builder/qemu/step_shutdown.go
index fd59fec84..a225496b2 100644
--- a/builder/qemu/step_shutdown.go
+++ b/builder/qemu/step_shutdown.go
@@ -17,7 +17,6 @@ import (
 //   config *config
 //   driver Driver
 //   ui     packer.Ui
-//   vmName string
 //
 // Produces:
 //   <nothing>
@@ -28,7 +27,6 @@ func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 	config := state.Get("config").(*config)
 	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
-	vmName := config.VMName
 
 	if config.ShutdownCommand != "" {
 		ui.Say("Gracefully halting virtual machine...")
@@ -41,28 +39,23 @@ func (s *stepShutdown) Run(state multistep.StateBag) multistep.StepAction {
 			return multistep.ActionHalt
 		}
 
-		// Wait for the machine to actually shut down
-		log.Printf("Waiting max %s for shutdown to complete", config.shutdownTimeout)
-		shutdownTimer := time.After(config.shutdownTimeout)
-		for {
-			running, _ := driver.IsRunning(vmName)
-			if !running {
-				break
-			}
+		// Start the goroutine that will time out our graceful attempt
+		cancelCh := make(chan struct{}, 1)
+		go func() {
+			defer close(cancelCh)
+			<-time.After(config.shutdownTimeout)
+		}()
 
-			select {
-			case <-shutdownTimer:
-				err := errors.New("Timeout while waiting for machine to shut down.")
-				state.Put("error", err)
-				ui.Error(err.Error())
-				return multistep.ActionHalt
-			default:
-				time.Sleep(1 * time.Second)
-			}
+		log.Printf("Waiting max %s for shutdown to complete", config.shutdownTimeout)
+		if ok := driver.WaitForShutdown(cancelCh); !ok {
+			err := errors.New("Timeout while waiting for machine to shut down.")
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
 		}
 	} else {
 		ui.Say("Halting the virtual machine...")
-		if err := driver.Stop(vmName); err != nil {
+		if err := driver.Stop(); err != nil {
 			err := fmt.Errorf("Error stopping VM: %s", err)
 			state.Put("error", err)
 			ui.Error(err.Error())
diff --git a/builder/qemu/step_wait_for_shutdown.go b/builder/qemu/step_wait_for_shutdown.go
new file mode 100644
index 000000000..5418b967f
--- /dev/null
+++ b/builder/qemu/step_wait_for_shutdown.go
@@ -0,0 +1,43 @@
+package qemu
+
+import (
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/packer"
+	"time"
+)
+
+// stepWaitForShutdown waits for the shutdown of the currently running
+// qemu VM.
+type stepWaitForShutdown struct {
+	Message string
+}
+
+func (s *stepWaitForShutdown) Run(state multistep.StateBag) multistep.StepAction {
+	driver := state.Get("driver").(Driver)
+	ui := state.Get("ui").(packer.Ui)
+
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	cancelCh := make(chan struct{})
+	go func() {
+		for {
+			if _, ok := state.GetOk(multistep.StateCancelled); ok {
+				close(cancelCh)
+				return
+			}
+
+			select {
+			case <-stopCh:
+				return
+			case <-time.After(100 * time.Millisecond):
+			}
+		}
+	}()
+
+	ui.Say(s.Message)
+	driver.WaitForShutdown(cancelCh)
+	return multistep.ActionContinue
+}
+
+func (s *stepWaitForShutdown) Cleanup(state multistep.StateBag) {}