Merge pull request #6 from chengsun/master

Fixing up a lot of the ignored error handling.

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

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 ``
+	RawBootWait string        `mapstructure:"boot_wait"`
-	SSHWaitTimeout time.Duration ``
+	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,
@@ -206,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 == "" {
@@ -361,12 +375,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
 }
 

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,8 +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"])
-		log.Fatal(res["ErrorDescription"])
+		return fmt.Errorf("API Error: %s", res["ErrorDescription"])
-		return errors.New("API Error occurred")
 	} else {
 		result.Value = res["Value"]
 	}
@@ -328,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)
@@ -346,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)
@@ -733,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) {

builder/xenserver/interruptible_wait.go

@@ -0,0 +1,83 @@
+package xenserver
+
+import (
+	"github.com/mitchellh/multistep"
+	"time"
+)
+
+type InterruptibleWait struct {
+	Timeout time.Duration
+
+	// optional:
+	Predicate         func() (result bool, err error)
+	PredicateInterval 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
+}
+
+/* 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)
+
+	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
+				}
+			}
+		}()
+	}
+
+	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:
+			if wait.Predicate != nil {
+				return TimeoutError{}
+			} else {
+				return nil
+			}
+		}
+	}
+}

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
 }
 
@@ -95,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() {
@@ -110,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

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
 }

builder/xenserver/step_create_instance.go

@@ -4,11 +4,11 @@ import (
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
-	"log"
 )
 
 type stepCreateInstance struct {
-	InstanceId string
+	instance *VM
+	vdi      *VDI
 }
 
 func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepAction {
@@ -25,20 +25,40 @@ 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
 	}
 
 	template := vms[0]
 
 	// Clone that VM template
-	instance, _ := template.Clone(config.InstanceName)
+	instance, err := template.Clone(config.InstanceName)
-	instance.SetIsATemplate(false)
+	if err != nil {
-	instance.SetStaticMemoryRange(config.InstanceMemory, config.InstanceMemory)
+		ui.Error(fmt.Sprintf("Error cloning VM: %s", err.Error()))
+		return multistep.ActionHalt
+	}
+	self.instance = instance
+
+	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
@@ -49,7 +69,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,25 +78,34 @@ 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
 		}
 
 		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
+	}
+	self.vdi = vdi
 
-	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
 
@@ -91,7 +120,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 +128,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 +139,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 +149,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 +181,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
 	}
 
@@ -164,29 +193,43 @@ 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)
 
-	// Stash the VM reference
+	err = instance.ConnectVdi(iso, true)
-	self.InstanceId, _ = instance.GetUuid()
+	if err != nil {
-	state.Put("instance_uuid", self.InstanceId)
+		ui.Error(fmt.Sprintf("Unable to connect ISO VDI: %s", err.Error()))
+		return multistep.ActionHalt
+	}
+
+	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", 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)
+	if self.instance != nil {
-	//    config := state.Get("config").(config)
+		ui.Say("Destroying VM")
-	//    ui := state.Get("ui").(packer.Ui)
+		_ = self.instance.HardShutdown()
-
+		err := self.instance.Destroy()
-	// If instance hasn't been created, we have nothing to do.
+		if err != nil {
-	if self.InstanceId == "" {
+			ui.Error(err.Error())
-		return
+		}
 	}
 
-	// @todo: destroy the created instance.
+	if self.vdi != nil {
-
+		ui.Say("Destroying VDI")
-	return
+		err := self.vdi.Destroy()
+		if err != nil {
+			ui.Error(err.Error())
+		}
+	}
 }

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
 	}
 

builder/xenserver/step_get_vnc_port.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
-	"log"
 	"strconv"
 )
 
@@ -20,13 +19,17 @@ 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)
 
 	if err != nil {
-		log.Fatal(err.Error())
+		ui.Error(fmt.Sprintf("Unable to convert '%s' to an int", remote_vncport))
-		log.Fatal(fmt.Sprintf("Unable to convert '%s' to an int", remote_vncport))
+		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
 

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.

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"
@@ -31,8 +32,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")
+		ui.Error("Unable to find a host internal management network")
-		log.Fatal(err.Error())
+		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
 
@@ -41,8 +42,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")
+		ui.Error("Error creating VIF")
-		log.Fatal(err.Error())
+		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
 
@@ -52,56 +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++ {
+	err = InterruptibleWait{
-		ips, _ := himn.GetAssignedIPs()
+		Predicate: func() (found bool, err error) {
-		log.Printf("IPs: %s", ips)
+			ips, err := himn.GetAssignedIPs()
-		log.Printf("Ref: %s", instance.Ref)
+			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
+			//Check for instance.Ref in map
-		if vm_ip, ok := ips[himn_vif.Ref]; ok {
+			if vm_ip, ok := ips[himn_vif.Ref]; ok {
-			ui.Say("Found the VM's IP " + vm_ip)
+				ui.Say("Found the VM's IP: " + vm_ip)
-			himn_iface_ip = vm_ip
+				himn_iface_ip = vm_ip
-			break
+				return true, nil
-		}
+			}
 
-		ui.Say("Wait for IP address...")
+			ui.Say("Wait for IP address...")
-		time.Sleep(10 * time.Second)
+			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 {
-		log.Fatal("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
+	err = InterruptibleWait{
-	for i := 0; i < 30; i++ {
+		Predicate: func() (success bool, err error) {
-		ui.Message(fmt.Sprintf("Attempting to ping interface: %s", ping_cmd))
+			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 = execute_ssh_cmd(ping_cmd, config.HostIp, "22", config.Username, config.Password)
 
-		if err == nil {
+			switch err.(type) {
-			ui.Message("Ping success! Continuing...")
+			case nil:
-			break
+				// ping succeeded
-		}
+				return true, nil
-
+			case *gossh.ExitError:
-		time.Sleep(10 * time.Second)
+				// 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 {
-		log.Fatal("Unable to ping interface. Something is wrong. Has the VM not booted?")
+		ui.Error(fmt.Sprintf("Unable to ping interface. (Has the VM not booted?): %s", err.Error()))
-		log.Fatal(err.Error())
 		return multistep.ActionHalt
 	}
 
-	time.Sleep(10 * time.Second)
+	ui.Message("Ping success! Continuing...")
-
 	return multistep.ActionContinue
 
 }

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

builder/xenserver/step_wait.go

@@ -1,6 +1,7 @@
 package xenserver
 
 import (
+	"fmt"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/packer"
 	"time"
@@ -9,31 +10,53 @@ 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)
+	instance, err := client.GetVMByUuid(instance_id)
-
+	if err != nil {
-	for {
+		ui.Error(fmt.Sprintf("Could not get VM from UUID %s", instance_id))
-		time.Sleep(30 * time.Second)
+		ui.Error(err.Error())
-		ui.Say("Waiting for VM install...")
+		return multistep.ActionHalt
-
-		power_state, _ := instance.GetPowerState()
-		if power_state == "Halted" {
-			ui.Say("Install has completed. Moving on.")
-			break
-		}
 	}
 
+	//Expect install to be configured to shutdown on completion
+	err = InterruptibleWait{
+		Predicate: func() (bool, error) {
+			ui.Say("Waiting for install to complete.")
+			power_state, err := instance.GetPowerState()
+			return power_state == "Halted", err
+		},
+		PredicateInterval: 30 * time.Second,
+		Timeout:           config.InstallTimeout,
+	}.Wait(state)
+
+	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()
+	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
@@ -49,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()

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": "600s",
         "ssh_username": "root",
         "ssh_password": "vmpassword",
         "boot_command":