From 6071be15723c681c5232811d397a470f004e6775 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Mon, 8 Dec 2014 18:18:23 +0000
Subject: [PATCH 01/18] Don't create Artifact if there were errors

Prevents nil-dereference crash when errors occur
---
 builder/xenserver/builder.go | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go
index 9c9e9e5..27ae0dd 100644
--- a/builder/xenserver/builder.go
+++ b/builder/xenserver/builder.go
@@ -361,12 +361,20 @@ func (self *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (pa
 	self.runner = &multistep.BasicRunner{Steps: steps}
 	self.runner.Run(state)
 
-	artifact, _ := NewArtifact(self.config.OutputDir)
-
 	if rawErr, ok := state.GetOk("error"); ok {
 		return nil, rawErr.(error)
 	}
 
+	// If we were interrupted or cancelled, then just exit.
+	if _, ok := state.GetOk(multistep.StateCancelled); ok {
+		return nil, errors.New("Build was cancelled.")
+	}
+	if _, ok := state.GetOk(multistep.StateHalted); ok {
+		return nil, errors.New("Build was halted.")
+	}
+
+	artifact, _ := NewArtifact(self.config.OutputDir)
+
 	return artifact, nil
 }
 

From d9757f2c38aa67f1815741d3040cd7376fa26ae3 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Mon, 8 Dec 2014 18:20:37 +0000
Subject: [PATCH 02/18] Revert "Add a pause to ensure SSH has a chance to
 start. This is a tempoary workaround for the fact that the SSH tunnel code
 will cause packer to halt if the destination does not exist."

This reverts commit fd35d7c9846566f39143972e846c39daac363a05.
---
 builder/xenserver/step_start_on_himn.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/builder/xenserver/step_start_on_himn.go b/builder/xenserver/step_start_on_himn.go
index 4048a6f..18771f1 100644
--- a/builder/xenserver/step_start_on_himn.go
+++ b/builder/xenserver/step_start_on_himn.go
@@ -100,8 +100,6 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction
 		return multistep.ActionHalt
 	}
 
-	time.Sleep(10 * time.Second)
-
 	return multistep.ActionContinue
 
 }

From 1dd963278f774177561edeb0f89e5b9f47954b99 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 10:42:16 +0000
Subject: [PATCH 03/18] Add generic interruptible wait method

---
 builder/xenserver/interruptible_wait.go | 66 +++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 builder/xenserver/interruptible_wait.go

diff --git a/builder/xenserver/interruptible_wait.go b/builder/xenserver/interruptible_wait.go
new file mode 100644
index 0000000..329c9a1
--- /dev/null
+++ b/builder/xenserver/interruptible_wait.go
@@ -0,0 +1,66 @@
+package xenserver
+
+import (
+	"github.com/mitchellh/multistep"
+	"time"
+)
+
+type InterruptibleWait struct {
+	Predicate func () (result bool, err error)
+	PredicateInterval time.Duration
+	Timeout time.Duration
+}
+
+type TimeoutError struct { }
+func (err TimeoutError) Error() string {
+	return "Timed out"
+}
+
+type InterruptedError struct { }
+func (err InterruptedError) Error() string {
+	return "Interrupted"
+}
+
+type PredicateResult struct {
+	complete bool
+	err error
+}
+
+func (wait InterruptibleWait) Wait(state multistep.StateBag) error {
+	predicateResult := make(chan PredicateResult, 1)
+	stopWaiting := make(chan struct{})
+	defer close(stopWaiting)
+
+	go func() {
+		for {
+			select {
+			case <-time.After(wait.PredicateInterval):
+				if complete, err := wait.Predicate(); err != nil || complete {
+					predicateResult <- PredicateResult{complete, err}
+					return
+				}
+
+			case <-stopWaiting:
+				return
+			}
+		}
+	}()
+
+	timeout := time.After(wait.Timeout)
+	for {
+		// wait for either install to complete/error,
+		// an interrupt to come through, or a timeout to occur
+		select {
+		case result := <-predicateResult:
+			return result.err
+
+		case <-time.After(1 * time.Second):
+			if _, ok := state.GetOk(multistep.StateCancelled); ok {
+				return InterruptedError{}
+			}
+
+		case <-timeout:
+			return TimeoutError{}
+		}
+	}
+}

From cadf3a5d6a565bf2fb2c4b970199b8239b3bcfc7 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Mon, 8 Dec 2014 17:49:07 +0000
Subject: [PATCH 04/18] Add install_timeout to stop build if step_wait takes
 too long

Halts the build when the VM does not shutdown within the timeout period
Implemented with interruptible_wait, so step_wait is also interruptible
---
 builder/xenserver/builder.go   | 22 ++++++++++++++++++----
 builder/xenserver/step_wait.go | 26 ++++++++++++++++----------
 examples/centos-6.4.conf       |  1 +
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go
index 27ae0dd..d3e1e4b 100644
--- a/builder/xenserver/builder.go
+++ b/builder/xenserver/builder.go
@@ -35,10 +35,9 @@ type config struct {
 	HostPortMax uint `mapstructure:"host_port_max"`
 
 	BootCommand []string `mapstructure:"boot_command"`
-	RawBootWait string   `mapstructure:"boot_wait"`
 
-	BootWait       time.Duration ``
-	SSHWaitTimeout time.Duration ``
+	RawBootWait string        `mapstructure:"boot_wait"`
+	BootWait    time.Duration ``
 
 	ISOChecksum     string   `mapstructure:"iso_checksum"`
 	ISOChecksumType string   `mapstructure:"iso_checksum_type"`
@@ -52,7 +51,11 @@ type config struct {
 	LocalIp      string            `mapstructure:"local_ip"`
 	PlatformArgs map[string]string `mapstructure:"platform_args"`
 
-	RawSSHWaitTimeout string `mapstructure:"ssh_wait_timeout"`
+	RawInstallTimeout string        `mapstructure:"install_timeout"`
+	InstallTimeout    time.Duration ``
+
+	RawSSHWaitTimeout string        `mapstructure:"ssh_wait_timeout"`
+	SSHWaitTimeout    time.Duration ``
 
 	SSHPassword string `mapstructure:"ssh_password"`
 	SSHUser     string `mapstructure:"ssh_username"`
@@ -100,6 +103,10 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error
 		self.config.RawBootWait = "5s"
 	}
 
+	if self.config.RawInstallTimeout == "" {
+		self.config.RawInstallTimeout = "200m"
+	}
+
 	if self.config.HTTPPortMin == 0 {
 		self.config.HTTPPortMin = 8000
 	}
@@ -133,6 +140,7 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error
 		"iso_checksum_type": &self.config.ISOChecksumType,
 		"http_directory":    &self.config.HTTPDir,
 		"local_ip":          &self.config.LocalIp,
+		"install_timeout":   &self.config.RawInstallTimeout,
 		"ssh_wait_timeout":  &self.config.RawSSHWaitTimeout,
 		"ssh_username":      &self.config.SSHUser,
 		"ssh_password":      &self.config.SSHPassword,
@@ -167,6 +175,12 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error
 			errs, fmt.Errorf("Failed to parse ssh_wait_timeout: %s", err))
 	}
 
+	self.config.InstallTimeout, err = time.ParseDuration(self.config.RawInstallTimeout)
+	if err != nil {
+		errs = packer.MultiErrorAppend(
+			errs, fmt.Errorf("Failed to parse install_timeout: %s", err))
+	}
+
 	for i, command := range self.config.BootCommand {
 		if err := self.config.tpl.Validate(command); err != nil {
 			errs = packer.MultiErrorAppend(errs,
diff --git a/builder/xenserver/step_wait.go b/builder/xenserver/step_wait.go
index f68dcb9..ddb716f 100644
--- a/builder/xenserver/step_wait.go
+++ b/builder/xenserver/step_wait.go
@@ -9,27 +9,33 @@ import (
 type stepWait struct{}
 
 func (self *stepWait) Run(state multistep.StateBag) multistep.StepAction {
+	config := state.Get("config").(config)
 	ui := state.Get("ui").(packer.Ui)
 	client := state.Get("client").(XenAPIClient)
 
 	ui.Say("Step: Wait for install to complete.")
 
-	//Expect install to be configured to shutdown on completion
-
 	instance_id := state.Get("instance_uuid").(string)
 	instance, _ := client.GetVMByUuid(instance_id)
 
-	for {
-		time.Sleep(30 * time.Second)
-		ui.Say("Waiting for VM install...")
+	//Expect install to be configured to shutdown on completion
+	err := InterruptibleWait{
+		Predicate: func () (bool, error) {
+			power_state, err := instance.GetPowerState()
+			return power_state == "Halted", err
+		},
+		PredicateInterval: 30 * time.Second,
+		Timeout: config.InstallTimeout,
+	}.Wait(state)
 
-		power_state, _ := instance.GetPowerState()
-		if power_state == "Halted" {
-			ui.Say("Install has completed. Moving on.")
-			break
-		}
+	if err != nil {
+		ui.Error(err.Error())
+		ui.Error("Giving up waiting for installation to complete.")
+		return multistep.ActionHalt
 	}
 
+	ui.Say("Install has completed. Moving on.")
+
 	// Eject ISO from drive
 	vbds, _ := instance.GetVBDs()
 	for _, vbd := range vbds {
diff --git a/examples/centos-6.4.conf b/examples/centos-6.4.conf
index 7271ce1..64232b1 100644
--- a/examples/centos-6.4.conf
+++ b/examples/centos-6.4.conf
@@ -10,6 +10,7 @@
         "iso_name": "CentOS-6.4-x86_64-minimal.iso",
         "http_directory": "http",
         "local_ip": "10.80.3.223",
+        "install_timeout": "600",
         "ssh_username": "root",
         "ssh_password": "vmpassword",
         "boot_command": 

From efbc365b6d1f646d8e9a7e0f51afe1a845e64083 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 11:17:35 +0000
Subject: [PATCH 05/18] fmt step_wait, interruptible_wait

---
 builder/xenserver/interruptible_wait.go | 12 +++++++-----
 builder/xenserver/step_wait.go          |  4 ++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/builder/xenserver/interruptible_wait.go b/builder/xenserver/interruptible_wait.go
index 329c9a1..ab90608 100644
--- a/builder/xenserver/interruptible_wait.go
+++ b/builder/xenserver/interruptible_wait.go
@@ -6,24 +6,26 @@ import (
 )
 
 type InterruptibleWait struct {
-	Predicate func () (result bool, err error)
+	Predicate         func() (result bool, err error)
 	PredicateInterval time.Duration
-	Timeout time.Duration
+	Timeout           time.Duration
 }
 
-type TimeoutError struct { }
+type TimeoutError struct{}
+
 func (err TimeoutError) Error() string {
 	return "Timed out"
 }
 
-type InterruptedError struct { }
+type InterruptedError struct{}
+
 func (err InterruptedError) Error() string {
 	return "Interrupted"
 }
 
 type PredicateResult struct {
 	complete bool
-	err error
+	err      error
 }
 
 func (wait InterruptibleWait) Wait(state multistep.StateBag) error {
diff --git a/builder/xenserver/step_wait.go b/builder/xenserver/step_wait.go
index ddb716f..8321785 100644
--- a/builder/xenserver/step_wait.go
+++ b/builder/xenserver/step_wait.go
@@ -20,12 +20,12 @@ func (self *stepWait) Run(state multistep.StateBag) multistep.StepAction {
 
 	//Expect install to be configured to shutdown on completion
 	err := InterruptibleWait{
-		Predicate: func () (bool, error) {
+		Predicate: func() (bool, error) {
 			power_state, err := instance.GetPowerState()
 			return power_state == "Halted", err
 		},
 		PredicateInterval: 30 * time.Second,
-		Timeout: config.InstallTimeout,
+		Timeout:           config.InstallTimeout,
 	}.Wait(state)
 
 	if err != nil {

From c88168f4ba134d6dae7942304e45e9a94880c5c2 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 11:27:31 +0000
Subject: [PATCH 06/18] Replace all Fatal calls with Error calls

---
 builder/xenserver/client.go                   |  8 -----
 builder/xenserver/step_create_instance.go     | 29 +++++++++----------
 .../xenserver/step_forward_port_over_ssh.go   |  2 +-
 builder/xenserver/step_get_vnc_port.go        |  5 ++--
 builder/xenserver/step_start_on_himn.go       | 14 ++++-----
 5 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/builder/xenserver/client.go b/builder/xenserver/client.go
index 17351cd..4957ba7 100644
--- a/builder/xenserver/client.go
+++ b/builder/xenserver/client.go
@@ -4,15 +4,8 @@ import (
 	"errors"
 	"fmt"
 	"github.com/nilshell/xmlrpc"
-	"log"
 )
 
-func check(err error) {
-	if err != nil {
-		log.Fatal(err)
-	}
-}
-
 type XenAPIClient struct {
 	Session  interface{}
 	Host     string
@@ -118,7 +111,6 @@ func (client *XenAPIClient) APICall(result *APIResult, method string, params ...
 	if result.Status != "Success" {
 		fmt.Println("Encountered an API error: ", result.Status)
 		fmt.Println(res["ErrorDescription"])
-		log.Fatal(res["ErrorDescription"])
 		return errors.New("API Error occurred")
 	} else {
 		result.Value = res["Value"]
diff --git a/builder/xenserver/step_create_instance.go b/builder/xenserver/step_create_instance.go
index 353a40a..bbf05d2 100644
--- a/builder/xenserver/step_create_instance.go
+++ b/builder/xenserver/step_create_instance.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
-	"log"
 )
 
 type stepCreateInstance struct {
@@ -25,10 +24,10 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 
 	switch {
 	case len(vms) == 0:
-		log.Fatal(fmt.Sprintf("Couldn't find a template with the name-label '%s'. Aborting.", config.CloneTemplate))
+		ui.Error(fmt.Sprintf("Couldn't find a template with the name-label '%s'. Aborting.", config.CloneTemplate))
 		return multistep.ActionHalt
 	case len(vms) > 1:
-		log.Fatal(fmt.Sprintf("Found more than one template with the name '%s'. The name must be unique. Aborting.", config.CloneTemplate))
+		ui.Error(fmt.Sprintf("Found more than one template with the name '%s'. The name must be unique. Aborting.", config.CloneTemplate))
 		return multistep.ActionHalt
 	}
 
@@ -49,7 +48,7 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		sr = default_sr
 
 		if err != nil {
-			log.Fatal(fmt.Sprintf("Error getting default SR: %s", err.Error()))
+			ui.Error(fmt.Sprintf("Error getting default SR: %s", err.Error()))
 			return multistep.ActionHalt
 		}
 
@@ -58,16 +57,16 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		srs, err := client.GetSRByNameLabel(config.SrName)
 
 		if err != nil {
-			log.Fatal(fmt.Sprintf("Error getting default SR: %s", err.Error()))
+			ui.Error(fmt.Sprintf("Error getting default SR: %s", err.Error()))
 			return multistep.ActionHalt
 		}
 
 		switch {
 		case len(srs) == 0:
-			log.Fatal(fmt.Sprintf("Couldn't find a SR with the specified name-label '%s'. Aborting.", config.SrName))
+			ui.Error(fmt.Sprintf("Couldn't find a SR with the specified name-label '%s'. Aborting.", config.SrName))
 			return multistep.ActionHalt
 		case len(srs) > 1:
-			log.Fatal(fmt.Sprintf("Found more than one SR with the name '%s'. The name must be unique. Aborting.", config.SrName))
+			ui.Error(fmt.Sprintf("Found more than one SR with the name '%s'. The name must be unique. Aborting.", config.SrName))
 			return multistep.ActionHalt
 		}
 
@@ -91,7 +90,7 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		pifs, err := client.GetPIFs()
 
 		if err != nil {
-			log.Fatal(fmt.Sprintf("Error getting PIFs %s", err.Error()))
+			ui.Error(fmt.Sprintf("Error getting PIFs %s", err.Error()))
 			return multistep.ActionHalt
 		}
 
@@ -99,7 +98,7 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 			pif_rec, err := pif.GetRecord()
 
 			if err != nil {
-				log.Fatal(fmt.Sprintf("Error getting PIF record: %s", err.Error()))
+				ui.Error(fmt.Sprintf("Error getting PIF record: %s", err.Error()))
 				return multistep.ActionHalt
 			}
 
@@ -110,7 +109,7 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		}
 
 		if network.Ref == "" {
-			log.Fatal("Error: couldn't find management network. Aborting.")
+			ui.Error("Error: couldn't find management network. Aborting.")
 			return multistep.ActionHalt
 		}
 
@@ -120,16 +119,16 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		networks, err := client.GetNetworkByNameLabel(config.NetworkName)
 
 		if err != nil {
-			log.Fatal(fmt.Sprintf("Error occured getting Network by name-label: %s", err.Error()))
+			ui.Error(fmt.Sprintf("Error occured getting Network by name-label: %s", err.Error()))
 			return multistep.ActionHalt
 		}
 
 		switch {
 		case len(networks) == 0:
-			log.Fatal(fmt.Sprintf("Couldn't find a network with the specified name-label '%s'. Aborting.", config.NetworkName))
+			ui.Error(fmt.Sprintf("Couldn't find a network with the specified name-label '%s'. Aborting.", config.NetworkName))
 			return multistep.ActionHalt
 		case len(networks) > 1:
-			log.Fatal(fmt.Sprintf("Found more than one SR with the name '%s'. The name must be unique. Aborting.", config.NetworkName))
+			ui.Error(fmt.Sprintf("Found more than one SR with the name '%s'. The name must be unique. Aborting.", config.NetworkName))
 			return multistep.ActionHalt
 		}
 
@@ -152,10 +151,10 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 
 	switch {
 	case len(isos) == 0:
-		log.Fatal(fmt.Sprintf("Couldn't find an ISO named '%s'. Aborting", config.IsoName))
+		ui.Error(fmt.Sprintf("Couldn't find an ISO named '%s'. Aborting", config.IsoName))
 		return multistep.ActionHalt
 	case len(isos) > 1:
-		log.Fatal(fmt.Sprintf("Found more than one VDI with name '%s'. Name must be unique. Aborting.", config.IsoName))
+		ui.Error(fmt.Sprintf("Found more than one VDI with name '%s'. Name must be unique. Aborting.", config.IsoName))
 		return multistep.ActionHalt
 	}
 
diff --git a/builder/xenserver/step_forward_port_over_ssh.go b/builder/xenserver/step_forward_port_over_ssh.go
index 58f9d83..a41c670 100644
--- a/builder/xenserver/step_forward_port_over_ssh.go
+++ b/builder/xenserver/step_forward_port_over_ssh.go
@@ -47,7 +47,7 @@ func (self *stepForwardPortOverSSH) Run(state multistep.StateBag) multistep.Step
 	}
 
 	if !foundPort {
-		log.Fatal("Error: unable to find free host port. Try providing a larger range")
+		ui.Error("Error: unable to find free host port. Try providing a larger range")
 		return multistep.ActionHalt
 	}
 
diff --git a/builder/xenserver/step_get_vnc_port.go b/builder/xenserver/step_get_vnc_port.go
index 46c1ac8..838da6d 100644
--- a/builder/xenserver/step_get_vnc_port.go
+++ b/builder/xenserver/step_get_vnc_port.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
-	"log"
 	"strconv"
 )
 
@@ -25,8 +24,8 @@ func (self *stepGetVNCPort) Run(state multistep.StateBag) multistep.StepAction {
 	remote_port, err := strconv.ParseUint(remote_vncport, 10, 16)
 
 	if err != nil {
-		log.Fatal(err.Error())
-		log.Fatal(fmt.Sprintf("Unable to convert '%s' to an int", remote_vncport))
+		ui.Error(fmt.Sprintf("Unable to convert '%s' to an int", remote_vncport))
+		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
 
diff --git a/builder/xenserver/step_start_on_himn.go b/builder/xenserver/step_start_on_himn.go
index 18771f1..e8223f3 100644
--- a/builder/xenserver/step_start_on_himn.go
+++ b/builder/xenserver/step_start_on_himn.go
@@ -31,8 +31,8 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction
 	// Find the HIMN Ref
 	networks, err := client.GetNetworkByNameLabel("Host internal management network")
 	if err != nil || len(networks) == 0 {
-		log.Fatal("Unable to find a host internal management network")
-		log.Fatal(err.Error())
+		ui.Error("Unable to find a host internal management network")
+		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
 
@@ -41,8 +41,8 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction
 	// Create a VIF for the HIMN
 	himn_vif, err := instance.ConnectNetwork(himn, "0")
 	if err != nil {
-		log.Fatal("Error creating VIF")
-		log.Fatal(err.Error())
+		ui.Error("Error creating VIF")
+		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
 
@@ -73,7 +73,7 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction
 		state.Put("himn_ssh_address", himn_iface_ip)
 		ui.Say("Stored VM's IP " + himn_iface_ip)
 	} else {
-		log.Fatal("Unable to find an IP on the Host-internal management interface")
+		ui.Error("Unable to find an IP on the Host-internal management interface")
 		return multistep.ActionHalt
 	}
 
@@ -95,8 +95,8 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction
 	}
 
 	if err != nil {
-		log.Fatal("Unable to ping interface. Something is wrong. Has the VM not booted?")
-		log.Fatal(err.Error())
+		ui.Error("Unable to ping interface. Something is wrong. Has the VM not booted?")
+		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
 

From 70e57a2d6638dbb6f7c38c389dc4453afda70b7b Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 11:47:07 +0000
Subject: [PATCH 07/18] Add error handling to step_wait

---
 builder/xenserver/step_wait.go | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/builder/xenserver/step_wait.go b/builder/xenserver/step_wait.go
index 8321785..4cd8fe3 100644
--- a/builder/xenserver/step_wait.go
+++ b/builder/xenserver/step_wait.go
@@ -1,6 +1,7 @@
 package xenserver
 
 import (
+	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 	"time"
@@ -16,11 +17,17 @@ func (self *stepWait) Run(state multistep.StateBag) multistep.StepAction {
 	ui.Say("Step: Wait for install to complete.")
 
 	instance_id := state.Get("instance_uuid").(string)
-	instance, _ := client.GetVMByUuid(instance_id)
+	instance, err := client.GetVMByUuid(instance_id)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Could not get VM from UUID %s", instance_id))
+		ui.Error(err.Error())
+		return multistep.ActionHalt
+	}
 
 	//Expect install to be configured to shutdown on completion
-	err := InterruptibleWait{
+	err = InterruptibleWait{
 		Predicate: func() (bool, error) {
+			ui.Say("Waiting for install to complete.")
 			power_state, err := instance.GetPowerState()
 			return power_state == "Halted", err
 		},
@@ -37,9 +44,19 @@ func (self *stepWait) Run(state multistep.StateBag) multistep.StepAction {
 	ui.Say("Install has completed. Moving on.")
 
 	// Eject ISO from drive
-	vbds, _ := instance.GetVBDs()
+	vbds, err := instance.GetVBDs()
+	if err != nil {
+		ui.Error(fmt.Sprintf("Could not get VBDs"))
+		ui.Error(err.Error())
+		return multistep.ActionHalt
+	}
 	for _, vbd := range vbds {
-		rec, _ := vbd.GetRecord()
+		rec, err := vbd.GetRecord()
+		if err != nil {
+			ui.Error(fmt.Sprintf("Could not get record for VBD"))
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
 
 		// Hack - should encapsulate this in the client really
 		// This is needed because we can't guarentee the type
@@ -55,7 +72,12 @@ func (self *stepWait) Run(state multistep.StateBag) multistep.StepAction {
 	}
 
 	// Destroy all connected VIFs
-	vifs, _ := instance.GetVIFs()
+	vifs, err := instance.GetVIFs()
+	if err != nil {
+		ui.Error(fmt.Sprintf("Could not get VIFs"))
+		ui.Error(err.Error())
+		return multistep.ActionHalt
+	}
 	for _, vif := range vifs {
 		ui.Message("Destroying VIF " + vif.Ref)
 		vif.Destroy()

From 8a41cc693a2aafd5ca30af4c2e267c48e352ecd7 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 14:02:48 +0000
Subject: [PATCH 08/18] Add more error handling

---
 builder/xenserver/client.go                   |  2 +-
 builder/xenserver/step_create_instance.go     | 53 ++++++++++++++++---
 builder/xenserver/step_get_vnc_port.go        |  6 ++-
 builder/xenserver/step_shutdown_and_export.go | 24 +++++++--
 builder/xenserver/step_start_vm_paused.go     | 20 +++++--
 5 files changed, 88 insertions(+), 17 deletions(-)

diff --git a/builder/xenserver/client.go b/builder/xenserver/client.go
index 4957ba7..a1bb3dd 100644
--- a/builder/xenserver/client.go
+++ b/builder/xenserver/client.go
@@ -111,7 +111,7 @@ func (client *XenAPIClient) APICall(result *APIResult, method string, params ...
 	if result.Status != "Success" {
 		fmt.Println("Encountered an API error: ", result.Status)
 		fmt.Println(res["ErrorDescription"])
-		return errors.New("API Error occurred")
+		return fmt.Errorf("API Error: %s", res["ErrorDescription"])
 	} else {
 		result.Value = res["Value"]
 	}
diff --git a/builder/xenserver/step_create_instance.go b/builder/xenserver/step_create_instance.go
index bbf05d2..5a6c9be 100644
--- a/builder/xenserver/step_create_instance.go
+++ b/builder/xenserver/step_create_instance.go
@@ -34,10 +34,29 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 	template := vms[0]
 
 	// Clone that VM template
-	instance, _ := template.Clone(config.InstanceName)
-	instance.SetIsATemplate(false)
-	instance.SetStaticMemoryRange(config.InstanceMemory, config.InstanceMemory)
+	instance, err := template.Clone(config.InstanceName)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Error cloning VM: %s", err.Error()))
+		return multistep.ActionHalt
+	}
+
+	err = instance.SetIsATemplate(false)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Error setting is_a_template=false: %s", err.Error()))
+		return multistep.ActionHalt
+	}
+
+	err = instance.SetStaticMemoryRange(config.InstanceMemory, config.InstanceMemory)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Error setting VM memory=%s: %s", config.InstanceMemory, err.Error()))
+		return multistep.ActionHalt
+	}
+
 	instance.SetPlatform(config.PlatformArgs)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Error setting VM platform: %s", err.Error()))
+		return multistep.ActionHalt
+	}
 
 	// Create VDI for the instance
 	var sr *SR
@@ -73,9 +92,17 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		sr = srs[0]
 	}
 
-	vdi, _ := sr.CreateVdi("Packer-disk", config.RootDiskSize)
+	vdi, err := sr.CreateVdi("Packer-disk", config.RootDiskSize)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to create packer disk VDI: %s", err.Error()))
+		return multistep.ActionHalt
+	}
 
-	instance.ConnectVdi(vdi, false)
+	err = instance.ConnectVdi(vdi, false)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to connect packer disk VDI: %s", err.Error()))
+		return multistep.ActionHalt
+	}
 
 	// Connect Network
 
@@ -90,7 +117,7 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		pifs, err := client.GetPIFs()
 
 		if err != nil {
-			ui.Error(fmt.Sprintf("Error getting PIFs %s", err.Error()))
+			ui.Error(fmt.Sprintf("Error getting PIFs: %s", err.Error()))
 			return multistep.ActionHalt
 		}
 
@@ -163,10 +190,20 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 	//iso, _ := client.GetVdiByUuid(config.IsoUuid)
 	//ui.Say("Using VDI: " + iso_vdi_uuid)
 	//iso, _ := client.GetVdiByUuid(iso_vdi_uuid)
-	instance.ConnectVdi(iso, true)
+
+	err = instance.ConnectVdi(iso, true)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to connect ISO VDI: %s", err.Error()))
+		return multistep.ActionHalt
+	}
 
 	// Stash the VM reference
-	self.InstanceId, _ = instance.GetUuid()
+	self.InstanceId, err = instance.GetUuid()
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to get VM UUID: %s", err.Error()))
+		return multistep.ActionHalt
+	}
+
 	state.Put("instance_uuid", self.InstanceId)
 	state.Put("instance", instance)
 	ui.Say(fmt.Sprintf("Created instance '%s'", self.InstanceId))
diff --git a/builder/xenserver/step_get_vnc_port.go b/builder/xenserver/step_get_vnc_port.go
index 838da6d..ff3ef22 100644
--- a/builder/xenserver/step_get_vnc_port.go
+++ b/builder/xenserver/step_get_vnc_port.go
@@ -19,7 +19,11 @@ func (self *stepGetVNCPort) Run(state multistep.StateBag) multistep.StepAction {
 	domid := state.Get("domid").(string)
 	cmd := fmt.Sprintf("xenstore-read /local/domain/%s/console/vnc-port", domid)
 
-	remote_vncport, _ := execute_ssh_cmd(cmd, config.HostIp, "22", config.Username, config.Password)
+	remote_vncport, err := execute_ssh_cmd(cmd, config.HostIp, "22", config.Username, config.Password)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to get VNC port (is the VM running?): %s", err.Error()))
+		return multistep.ActionHalt
+	}
 
 	remote_port, err := strconv.ParseUint(remote_vncport, 10, 16)
 
diff --git a/builder/xenserver/step_shutdown_and_export.go b/builder/xenserver/step_shutdown_and_export.go
index 81dca51..ed25fd4 100644
--- a/builder/xenserver/step_shutdown_and_export.go
+++ b/builder/xenserver/step_shutdown_and_export.go
@@ -49,13 +49,21 @@ func (stepShutdownAndExport) Run(state multistep.StateBag) multistep.StepAction
 	client := state.Get("client").(XenAPIClient)
 	instance_uuid := state.Get("instance_uuid").(string)
 
-	instance, _ := client.GetVMByUuid(instance_uuid)
+	instance, err := client.GetVMByUuid(instance_uuid)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Could not get VM with UUID '%s': %s", instance_uuid, err.Error()))
+		return multistep.ActionHalt
+	}
 
 	ui.Say("Step: Shutdown and export VPX")
 
 	// Shutdown the VM
 	ui.Say("Shutting down the VM...")
-	instance.CleanShutdown()
+	err = instance.CleanShutdown()
+	if err != nil {
+		ui.Error(fmt.Sprintf("Could not shut down VM: %s", err.Error()))
+		return multistep.ActionHalt
+	}
 
 	//Export the VM
 
@@ -69,9 +77,17 @@ func (stepShutdownAndExport) Run(state multistep.StateBag) multistep.StepAction
 	ui.Say("Getting metadata " + export_url)
 	downloadFile(export_url, export_filename)
 
-	disks, _ := instance.GetDisks()
+	disks, err := instance.GetDisks()
+	if err != nil {
+		ui.Error(fmt.Sprintf("Could not get VM disks: %s", err.Error()))
+		return multistep.ActionHalt
+	}
 	for _, disk := range disks {
-		disk_uuid, _ := disk.GetUuid()
+		disk_uuid, err := disk.GetUuid()
+		if err != nil {
+			ui.Error(fmt.Sprintf("Could not get disk with UUID '%s': %s", disk_uuid, err.Error()))
+			return multistep.ActionHalt
+		}
 
 		// Basic auth in URL request is required as session token is not
 		// accepted for some reason.
diff --git a/builder/xenserver/step_start_vm_paused.go b/builder/xenserver/step_start_vm_paused.go
index fd10f4d..8a0e246 100644
--- a/builder/xenserver/step_start_vm_paused.go
+++ b/builder/xenserver/step_start_vm_paused.go
@@ -1,6 +1,7 @@
 package xenserver
 
 import (
+	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 )
@@ -14,11 +15,24 @@ func (self *stepStartVmPaused) Run(state multistep.StateBag) multistep.StepActio
 
 	ui.Say("Step: Start VM Paused")
 
-	instance, _ := client.GetVMByUuid(state.Get("instance_uuid").(string))
+	uuid := state.Get("instance_uuid").(string)
+	instance, err := client.GetVMByUuid(uuid)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to get VM from UUID '%s': %s", uuid, err.Error()))
+		return multistep.ActionHalt
+	}
 
-	instance.Start(true, false)
+	err = instance.Start(true, false)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to start VM with UUID '%s': %s", uuid, err.Error()))
+		return multistep.ActionHalt
+	}
 
-	domid, _ := instance.GetDomainId()
+	domid, err := instance.GetDomainId()
+	if err != nil {
+		ui.Error(fmt.Sprintf("Unable to get domid of VM with UUID '%s': %s", uuid, err.Error()))
+		return multistep.ActionHalt
+	}
 	state.Put("domid", domid)
 
 	return multistep.ActionContinue

From 94dcf160df01323a7d70ef534020e313397f6087 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 14:17:47 +0000
Subject: [PATCH 09/18] Don't close session twice

---
 builder/xenserver/ssh.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builder/xenserver/ssh.go b/builder/xenserver/ssh.go
index 4d68684..8455236 100644
--- a/builder/xenserver/ssh.go
+++ b/builder/xenserver/ssh.go
@@ -80,7 +80,6 @@ func execute_ssh_cmd(cmd, host, port, username, password string) (stdout string,
 		return "", err
 	}
 
-	session.Close()
 	return strings.Trim(b.String(), "\n"), nil
 }
 

From 651274e5081b27f3c4b82804721d0661bf863c61 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 14:50:44 +0000
Subject: [PATCH 10/18] interruptible_wait: evaluate Predicate immediately

Previous behaviour was to wait PredicateInterval before the first
time that the Predicate is evaluated
---
 builder/xenserver/interruptible_wait.go | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builder/xenserver/interruptible_wait.go b/builder/xenserver/interruptible_wait.go
index ab90608..9757377 100644
--- a/builder/xenserver/interruptible_wait.go
+++ b/builder/xenserver/interruptible_wait.go
@@ -35,13 +35,14 @@ func (wait InterruptibleWait) Wait(state multistep.StateBag) error {
 
 	go func() {
 		for {
+			if complete, err := wait.Predicate(); err != nil || complete {
+				predicateResult <- PredicateResult{complete, err}
+				return
+			}
+
 			select {
 			case <-time.After(wait.PredicateInterval):
-				if complete, err := wait.Predicate(); err != nil || complete {
-					predicateResult <- PredicateResult{complete, err}
-					return
-				}
-
+				// do nothing; loop again
 			case <-stopWaiting:
 				return
 			}

From ba7c5ddcd2803d77653e1cc4b0f046efe8231058 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 14:55:35 +0000
Subject: [PATCH 11/18] interruptible_wait: allow nil Predicate

Allow for the case where InterruptibleWait is used purely for the
timeout.
See the documentation string for details.
---
 builder/xenserver/interruptible_wait.go | 44 ++++++++++++++++---------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/builder/xenserver/interruptible_wait.go b/builder/xenserver/interruptible_wait.go
index 9757377..8ad7f6f 100644
--- a/builder/xenserver/interruptible_wait.go
+++ b/builder/xenserver/interruptible_wait.go
@@ -6,9 +6,11 @@ import (
 )
 
 type InterruptibleWait struct {
+	Timeout time.Duration
+
+	// optional:
 	Predicate         func() (result bool, err error)
 	PredicateInterval time.Duration
-	Timeout           time.Duration
 }
 
 type TimeoutError struct{}
@@ -28,26 +30,34 @@ type PredicateResult struct {
 	err      error
 }
 
+/* Wait waits for up to Timeout duration, checking an optional Predicate every PredicateInterval duration.
+   The first run of Predicate is immediately after Wait is called.
+   If the command is interrupted by the user, then an InterruptedError is returned.
+   If Predicate is not nil, a timeout leads to TimeoutError being returned, and a successful Predicate run leads to nil being returned.
+   If Predicate is nil, a timeout is not an error, and nil is returned.
+*/
 func (wait InterruptibleWait) Wait(state multistep.StateBag) error {
 	predicateResult := make(chan PredicateResult, 1)
 	stopWaiting := make(chan struct{})
 	defer close(stopWaiting)
 
-	go func() {
-		for {
-			if complete, err := wait.Predicate(); err != nil || complete {
-				predicateResult <- PredicateResult{complete, err}
-				return
-			}
+	if wait.Predicate != nil {
+		go func() {
+			for {
+				if complete, err := wait.Predicate(); err != nil || complete {
+					predicateResult <- PredicateResult{complete, err}
+					return
+				}
 
-			select {
-			case <-time.After(wait.PredicateInterval):
-				// do nothing; loop again
-			case <-stopWaiting:
-				return
+				select {
+				case <-time.After(wait.PredicateInterval):
+					// do nothing; loop again
+				case <-stopWaiting:
+					return
+				}
 			}
-		}
-	}()
+		}()
+	}
 
 	timeout := time.After(wait.Timeout)
 	for {
@@ -63,7 +73,11 @@ func (wait InterruptibleWait) Wait(state multistep.StateBag) error {
 			}
 
 		case <-timeout:
-			return TimeoutError{}
+			if wait.Predicate != nil {
+				return TimeoutError{}
+			} else {
+				return nil
+			}
 		}
 	}
 }

From c75b4555ccd22859bc4eb1643c01293c745b190d Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 14:56:21 +0000
Subject: [PATCH 12/18] Make boot_wait interruptible

---
 builder/xenserver/step_boot_wait.go | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/builder/xenserver/step_boot_wait.go b/builder/xenserver/step_boot_wait.go
index 58f8126..4978619 100644
--- a/builder/xenserver/step_boot_wait.go
+++ b/builder/xenserver/step_boot_wait.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
-	"time"
 )
 
 type stepBootWait struct{}
@@ -20,7 +19,11 @@ func (self *stepBootWait) Run(state multistep.StateBag) multistep.StepAction {
 
 	if int64(config.BootWait) > 0 {
 		ui.Say(fmt.Sprintf("Waiting %s for boot...", config.BootWait))
-		time.Sleep(config.BootWait)
+		err := InterruptibleWait{Timeout: config.BootWait}.Wait(state)
+		if err != nil {
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
 	}
 	return multistep.ActionContinue
 }

From acd16bb984143225a6998997cdad9c019dd2caf7 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 14:56:42 +0000
Subject: [PATCH 13/18] Make HIMN IP finding and ping interruptible

---
 builder/xenserver/step_start_on_himn.go | 73 +++++++++++++++----------
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/builder/xenserver/step_start_on_himn.go b/builder/xenserver/step_start_on_himn.go
index e8223f3..6151f9e 100644
--- a/builder/xenserver/step_start_on_himn.go
+++ b/builder/xenserver/step_start_on_himn.go
@@ -1,6 +1,7 @@
 package xenserver
 
 import (
+	gossh "code.google.com/p/go.crypto/ssh"
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
@@ -52,54 +53,70 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction
 	var himn_iface_ip string = ""
 
 	// Obtain the allocated IP
-	for i := 0; i < 10; i++ {
-		ips, _ := himn.GetAssignedIPs()
-		log.Printf("IPs: %s", ips)
-		log.Printf("Ref: %s", instance.Ref)
+	err = InterruptibleWait{
+		Predicate: func() (found bool, err error) {
+			ips, err := himn.GetAssignedIPs()
+			if err != nil {
+				return false, fmt.Errorf("Can't get assigned IPs: %s", err.Error())
+			}
+			log.Printf("IPs: %s", ips)
+			log.Printf("Ref: %s", instance.Ref)
 
-		//Check for instance.Ref in map
-		if vm_ip, ok := ips[himn_vif.Ref]; ok {
-			ui.Say("Found the VM's IP " + vm_ip)
-			himn_iface_ip = vm_ip
-			break
-		}
+			//Check for instance.Ref in map
+			if vm_ip, ok := ips[himn_vif.Ref]; ok {
+				ui.Say("Found the VM's IP: " + vm_ip)
+				himn_iface_ip = vm_ip
+				return true, nil
+			}
 
-		ui.Say("Wait for IP address...")
-		time.Sleep(10 * time.Second)
+			ui.Say("Wait for IP address...")
+			return false, nil
+		},
+		PredicateInterval: 10 * time.Second,
+		Timeout:           100 * time.Second,
+	}.Wait(state)
 
+	if err != nil || himn_iface_ip == "" {
+		ui.Error(fmt.Sprintf("Unable to find an IP on the Host-internal management interface: %s", err.Error()))
+		return multistep.ActionHalt
 	}
 
 	if himn_iface_ip != "" {
 		state.Put("himn_ssh_address", himn_iface_ip)
 		ui.Say("Stored VM's IP " + himn_iface_ip)
-	} else {
-		ui.Error("Unable to find an IP on the Host-internal management interface")
-		return multistep.ActionHalt
 	}
 
 	// Wait for the VM to boot, and check we can ping this interface
 
 	ping_cmd := fmt.Sprintf("ping -c 1 %s", himn_iface_ip)
 
-	err = nil
-	for i := 0; i < 30; i++ {
-		ui.Message(fmt.Sprintf("Attempting to ping interface: %s", ping_cmd))
-		_, err := execute_ssh_cmd(ping_cmd, config.HostIp, "22", config.Username, config.Password)
+	err = InterruptibleWait{
+		Predicate: func() (success bool, err error) {
+			ui.Message(fmt.Sprintf("Attempting to ping interface: %s", ping_cmd))
+			_, err = execute_ssh_cmd(ping_cmd, config.HostIp, "22", config.Username, config.Password)
 
-		if err == nil {
-			ui.Message("Ping success! Continuing...")
-			break
-		}
-
-		time.Sleep(10 * time.Second)
-	}
+			switch err.(type) {
+			case nil:
+				// ping succeeded
+				return true, nil
+			case *gossh.ExitError:
+				// ping failed, try again
+				return false, nil
+			default:
+				// unknown error
+				return false, err
+			}
+		},
+		PredicateInterval: 10 * time.Second,
+		Timeout:           300 * time.Second,
+	}.Wait(state)
 
 	if err != nil {
-		ui.Error("Unable to ping interface. Something is wrong. Has the VM not booted?")
-		ui.Error(err.Error())
+		ui.Error(fmt.Sprintf("Unable to ping interface. (Has the VM not booted?): %s", err.Error()))
 		return multistep.ActionHalt
 	}
 
+	ui.Message("Ping success! Continuing...")
 	return multistep.ActionContinue
 
 }

From 1450fd0568d8d0eda8d98fb21dcc036d114445ee Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 16:40:32 +0000
Subject: [PATCH 14/18] Clean up VM instance and root VDI

---
 builder/xenserver/client.go               | 27 +++++++++++++++++
 builder/xenserver/step_create_instance.go | 37 ++++++++++++++---------
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/builder/xenserver/client.go b/builder/xenserver/client.go
index a1bb3dd..96dea6f 100644
--- a/builder/xenserver/client.go
+++ b/builder/xenserver/client.go
@@ -320,6 +320,15 @@ func (self *VM) Clone(label string) (new_instance *VM, err error) {
 	return
 }
 
+func (self *VM) Destroy() (err error) {
+	result := APIResult{}
+	err = self.Client.APICall(&result, "VM.destroy", self.Ref)
+	if err != nil {
+		return err
+	}
+	return
+}
+
 func (self *VM) Start(paused, force bool) (err error) {
 	result := APIResult{}
 	err = self.Client.APICall(&result, "VM.start", self.Ref, paused, force)
@@ -338,6 +347,15 @@ func (self *VM) CleanShutdown() (err error) {
 	return
 }
 
+func (self *VM) HardShutdown() (err error) {
+	result := APIResult{}
+	err = self.Client.APICall(&result, "VM.hard_shutdown", self.Ref)
+	if err != nil {
+		return err
+	}
+	return
+}
+
 func (self *VM) Unpause() (err error) {
 	result := APIResult{}
 	err = self.Client.APICall(&result, "VM.unpause", self.Ref)
@@ -725,6 +743,15 @@ func (self *VDI) GetUuid() (vdi_uuid string, err error) {
 	return vdi_uuid, nil
 }
 
+func (self *VDI) Destroy() (err error) {
+	result := APIResult{}
+	err = self.Client.APICall(&result, "VDI.destroy", self.Ref)
+	if err != nil {
+		return err
+	}
+	return
+}
+
 // Client Initiator
 
 func NewXenAPIClient(host, username, password string) (client XenAPIClient) {
diff --git a/builder/xenserver/step_create_instance.go b/builder/xenserver/step_create_instance.go
index 5a6c9be..6ebf8cd 100644
--- a/builder/xenserver/step_create_instance.go
+++ b/builder/xenserver/step_create_instance.go
@@ -7,7 +7,8 @@ import (
 )
 
 type stepCreateInstance struct {
-	InstanceId string
+	instance *VM
+	vdi      *VDI
 }
 
 func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepAction {
@@ -39,6 +40,7 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		ui.Error(fmt.Sprintf("Error cloning VM: %s", err.Error()))
 		return multistep.ActionHalt
 	}
+	self.instance = instance
 
 	err = instance.SetIsATemplate(false)
 	if err != nil {
@@ -97,6 +99,7 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		ui.Error(fmt.Sprintf("Unable to create packer disk VDI: %s", err.Error()))
 		return multistep.ActionHalt
 	}
+	self.vdi = vdi
 
 	err = instance.ConnectVdi(vdi, false)
 	if err != nil {
@@ -197,32 +200,36 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi
 		return multistep.ActionHalt
 	}
 
-	// Stash the VM reference
-	self.InstanceId, err = instance.GetUuid()
+	instanceId, err := instance.GetUuid()
 	if err != nil {
 		ui.Error(fmt.Sprintf("Unable to get VM UUID: %s", err.Error()))
 		return multistep.ActionHalt
 	}
 
-	state.Put("instance_uuid", self.InstanceId)
+	state.Put("instance_uuid", instanceId)
 	state.Put("instance", instance)
-	ui.Say(fmt.Sprintf("Created instance '%s'", self.InstanceId))
+	ui.Say(fmt.Sprintf("Created instance '%s'", instanceId))
 
 	return multistep.ActionContinue
 }
 
 func (self *stepCreateInstance) Cleanup(state multistep.StateBag) {
+	ui := state.Get("ui").(packer.Ui)
 
-	//    client := state.Get("client").(*XenAPIClient)
-	//    config := state.Get("config").(config)
-	//    ui := state.Get("ui").(packer.Ui)
-
-	// If instance hasn't been created, we have nothing to do.
-	if self.InstanceId == "" {
-		return
+	if self.instance != nil {
+		ui.Say("Destroying VM")
+		_ = self.instance.HardShutdown()
+		err := self.instance.Destroy()
+		if err != nil {
+			ui.Error(err.Error())
+		}
 	}
 
-	// @todo: destroy the created instance.
-
-	return
+	if self.vdi != nil {
+		ui.Say("Destroying VDI")
+		err := self.vdi.Destroy()
+		if err != nil {
+			ui.Error(err.Error())
+		}
+	}
 }

From 97df6fd283f7f3626e6ad80830dbeb0815428d08 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 17:44:21 +0000
Subject: [PATCH 15/18] Close SSH connections once we're done with them

Fixes "Waiting for SSH" hang at end of install
---
 builder/xenserver/ssh.go | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/builder/xenserver/ssh.go b/builder/xenserver/ssh.go
index 8455236..935221d 100644
--- a/builder/xenserver/ssh.go
+++ b/builder/xenserver/ssh.go
@@ -94,14 +94,19 @@ func forward(local_conn net.Conn, config *gossh.ClientConfig, server, remote_des
 	ssh_conn, err := ssh_client_conn.Dial("tcp", remote_loc)
 	if err != nil {
 		log.Printf("ssh.Dial error: %s", err)
+		ssh_client_conn.Close()
 		return err
 	}
 
+	txDone := make(chan struct{})
+	rxDone := make(chan struct{})
+
 	go func() {
 		_, err = io.Copy(ssh_conn, local_conn)
 		if err != nil {
 			log.Printf("io.copy failed: %v", err)
 		}
+		close(txDone)
 	}()
 
 	go func() {
@@ -109,6 +114,14 @@ func forward(local_conn net.Conn, config *gossh.ClientConfig, server, remote_des
 		if err != nil {
 			log.Printf("io.copy failed: %v", err)
 		}
+		close(rxDone)
+	}()
+
+	go func() {
+		<-txDone
+		<-rxDone
+		ssh_client_conn.Close()
+		ssh_conn.Close()
 	}()
 
 	return nil

From 490ff260c2217608ce0a4daf3e74553dba99b08e Mon Sep 17 00:00:00 2001
From: Cheng Sun <_@chengsun.uk>
Date: Wed, 10 Dec 2014 10:14:18 +0000
Subject: [PATCH 16/18] Update README to link to centos example directly

---
 README.md | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/README.md b/README.md
index 498227f..7554112 100644
--- a/README.md
+++ b/README.md
@@ -68,31 +68,8 @@ in your $PACKERPATH directory and you are ready to get going with packer.
 
 Once you've setup the above, you are good to go with an example. 
 
-To get you started, there is an example config file which you can use `examples/centos-6.4.conf`:
+To get you started, there is an example config file which you can use: [`examples/centos-6.4.conf`](https://github.com/rdobson/packer-builder-xenserver/blob/master/examples/centos-6.4.conf)
 
-```shell
-{
-    "builders": [{
-        "type": "xenserver",
-        "username": "root",
-        "password": "hostpassword",
-        "host_ip": "10.81.2.105",
-        "instance_name": "packer-centos-6-4",
-        "instance_memory": "2048000000",
-        "root_disk_size": "40000000000",
-        "iso_name": "CentOS-6.4-x86_64-minimal.iso",
-        "http_directory": "http",
-        "local_ip": "10.80.3.223",
-        "ssh_username": "root",
-        "ssh_password": "vmpassword",
-        "boot_command": 
-            [
-                "<tab><wait>",
-                " ks=http://{{ .HTTPIP }}:{{ .HTTPPort }}/centos6-ks.cfg<enter>"
-            ]
-    }]
-}
-```
 Currently it is not (easily) possible to take care of the ISO download and upload,
 so you will need to attach an ISO SR to the XenServer host (NFS/CIFS) with the
 ISO you want to use for installation. You will then need to specify the name

From ca80e009dbe651f1939624bf2669da1e5cbd08ce Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Tue, 9 Dec 2014 18:38:59 +0000
Subject: [PATCH 17/18] Fix typo

---
 builder/xenserver/builder.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go
index d3e1e4b..523e5ec 100644
--- a/builder/xenserver/builder.go
+++ b/builder/xenserver/builder.go
@@ -220,7 +220,7 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error
 
 	if self.config.InstanceName == "" {
 		errs = packer.MultiErrorAppend(
-			errs, errors.New("An insatnce name must be specified."))
+			errs, errors.New("An instance name must be specified."))
 	}
 
 	if self.config.InstanceMemory == "" {

From 4cd0fcf288ee2333b91b9c977ab785ea3971c6d2 Mon Sep 17 00:00:00 2001
From: Cheng Sun <cheng.sun@citrix.com>
Date: Wed, 10 Dec 2014 10:25:08 +0000
Subject: [PATCH 18/18] example config: explicitly state units in
 install_timeout

---
 examples/centos-6.4.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/centos-6.4.conf b/examples/centos-6.4.conf
index 64232b1..9e4ee99 100644
--- a/examples/centos-6.4.conf
+++ b/examples/centos-6.4.conf
@@ -10,7 +10,7 @@
         "iso_name": "CentOS-6.4-x86_64-minimal.iso",
         "http_directory": "http",
         "local_ip": "10.80.3.223",
-        "install_timeout": "600",
+        "install_timeout": "600s",
         "ssh_username": "root",
         "ssh_password": "vmpassword",
         "boot_command":