Merge pull request #1200 from zigg/feature/virtualbox-iso-sata

builder/virtualbox: implement `iso_interface` setting

builder/virtualbox/common/step_remove_devices.go

@@ -41,11 +41,20 @@ func (s *StepRemoveDevices) Run(state multistep.StateBag) multistep.StepAction {
 	}
 
 	if _, ok := state.GetOk("attachedIso"); ok {
+		controllerName := "IDE Controller"
+		port := "0"
+		device := "1"
+		if _, ok := state.GetOk("attachedIsoOnSata"); ok {
+			controllerName = "SATA Controller"
+			port = "1"
+			device = "0"
+		}
+
 		command := []string{
 			"storageattach", vmName,
-			"--storagectl", "IDE Controller",
-			"--port", "0",
-			"--device", "1",
+			"--storagectl", controllerName,
+			"--port", port,
+			"--device", device,
 			"--medium", "none",
 		}
 

builder/virtualbox/common/step_remove_devices_test.go

@@ -57,6 +57,33 @@ func TestStepRemoveDevices_attachedIso(t *testing.T) {
 	}
 }
 
+func TestStepRemoveDevices_attachedIsoOnSata(t *testing.T) {
+	state := testState(t)
+	step := new(StepRemoveDevices)
+
+	state.Put("attachedIso", true)
+	state.Put("attachedIsoOnSata", true)
+	state.Put("vmName", "foo")
+
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(state); action != multistep.ActionContinue {
+		t.Fatalf("bad action: %#v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("should NOT have error")
+	}
+
+	// Test that ISO was removed
+	if len(driver.VBoxManageCalls) != 1 {
+		t.Fatalf("bad: %#v", driver.VBoxManageCalls)
+	}
+	if driver.VBoxManageCalls[0][3] != "SATA Controller" {
+		t.Fatalf("bad: %#v", driver.VBoxManageCalls)
+	}
+}
+
 func TestStepRemoveDevices_floppyPath(t *testing.T) {
 	state := testState(t)
 	step := new(StepRemoveDevices)

builder/virtualbox/iso/builder.go

@@ -47,6 +47,7 @@ type config struct {
 	HTTPPortMax          uint     `mapstructure:"http_port_max"`
 	ISOChecksum          string   `mapstructure:"iso_checksum"`
 	ISOChecksumType      string   `mapstructure:"iso_checksum_type"`
+	ISOInterface         string   `mapstructure:"iso_interface"`
 	ISOUrls              []string `mapstructure:"iso_urls"`
 	VMName               string   `mapstructure:"vm_name"`
 
@@ -110,6 +111,10 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		b.config.HTTPPortMax = 9000
 	}
 
+	if b.config.ISOInterface == "" {
+		b.config.ISOInterface = "ide"
+	}
+
 	if b.config.VMName == "" {
 		b.config.VMName = fmt.Sprintf("packer-%s", b.config.PackerBuildName)
 	}
@@ -123,6 +128,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		"http_directory":         &b.config.HTTPDir,
 		"iso_checksum":           &b.config.ISOChecksum,
 		"iso_checksum_type":      &b.config.ISOChecksumType,
+		"iso_interface":          &b.config.ISOInterface,
 		"iso_url":                &b.config.RawSingleISOUrl,
 		"vm_name":                &b.config.VMName,
 	}
@@ -195,6 +201,11 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		}
 	}
 
+	if b.config.ISOInterface != "ide" && b.config.ISOInterface != "sata" {
+		errs = packer.MultiErrorAppend(
+			errs, errors.New("iso_interface can only be ide or sata"))
+	}
+
 	if b.config.RawSingleISOUrl == "" && len(b.config.ISOUrls) == 0 {
 		errs = packer.MultiErrorAppend(
 			errs, errors.New("One of iso_url or iso_urls must be specified."))

builder/virtualbox/iso/builder_test.go

@@ -401,6 +401,47 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) {
 	}
 }
 
+func TestBuilderPrepare_ISOInterface(t *testing.T) {
+	var b Builder
+	config := testConfig()
+
+	// Test a default boot_wait
+	delete(config, "iso_interface")
+	warns, err := b.Prepare(config)
+	if len(warns) > 0 {
+		t.Fatalf("bad: %#v", warns)
+	}
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if b.config.ISOInterface != "ide" {
+		t.Fatalf("bad: %s", b.config.ISOInterface)
+	}
+
+	// Test with a bad
+	config["iso_interface"] = "fake"
+	b = Builder{}
+	warns, err = b.Prepare(config)
+	if len(warns) > 0 {
+		t.Fatalf("bad: %#v", warns)
+	}
+	if err == nil {
+		t.Fatal("should have error")
+	}
+
+	// Test with a good
+	config["iso_interface"] = "sata"
+	b = Builder{}
+	warns, err = b.Prepare(config)
+	if len(warns) > 0 {
+		t.Fatalf("bad: %#v", warns)
+	}
+	if err != nil {
+		t.Fatalf("should not have error: %s", err)
+	}
+}
+
 func TestBuilderPrepare_ISOUrl(t *testing.T) {
 	var b Builder
 	config := testConfig()

builder/virtualbox/iso/step_attach_iso.go

@@ -17,17 +17,27 @@ type stepAttachISO struct {
 }
 
 func (s *stepAttachISO) Run(state multistep.StateBag) multistep.StepAction {
+	config := state.Get("config").(*config)
 	driver := state.Get("driver").(vboxcommon.Driver)
 	isoPath := state.Get("iso_path").(string)
 	ui := state.Get("ui").(packer.Ui)
 	vmName := state.Get("vmName").(string)
 
+	controllerName := "IDE Controller"
+	port := "0"
+	device := "1"
+	if config.ISOInterface == "sata" {
+		controllerName = "SATA Controller"
+		port = "1"
+		device = "0"
+	}
+
 	// Attach the disk to the controller
 	command := []string{
 		"storageattach", vmName,
-		"--storagectl", "IDE Controller",
-		"--port", "0",
-		"--device", "1",
+		"--storagectl", controllerName,
+		"--port", port,
+		"--device", device,
 		"--type", "dvddrive",
 		"--medium", isoPath,
 	}
@@ -43,6 +53,9 @@ func (s *stepAttachISO) Run(state multistep.StateBag) multistep.StepAction {
 
 	// Set some state so we know to remove
 	state.Put("attachedIso", true)
+	if controllerName == "SATA Controller" {
+		state.Put("attachedIsoOnSata", true)
+	}
 
 	return multistep.ActionContinue
 }
@@ -52,14 +65,24 @@ func (s *stepAttachISO) Cleanup(state multistep.StateBag) {
 		return
 	}
 
+	config := state.Get("config").(*config)
 	driver := state.Get("driver").(vboxcommon.Driver)
 	vmName := state.Get("vmName").(string)
 
+	controllerName := "IDE Controller"
+	port := "0"
+	device := "1"
+	if config.ISOInterface == "sata" {
+		controllerName = "SATA Controller"
+		port = "1"
+		device = "0"
+	}
+
 	command := []string{
 		"storageattach", vmName,
-		"--storagectl", "IDE Controller",
-		"--port", "0",
-		"--device", "1",
+		"--storagectl", controllerName,
+		"--port", port,
+		"--device", device,
 		"--medium", "none",
 	}
 

builder/virtualbox/iso/step_create_disk.go

@@ -43,8 +43,7 @@ func (s *stepCreateDisk) Run(state multistep.StateBag) multistep.StepAction {
 	// Add the IDE controller so we can later attach the disk.
 	// When the hard disk controller is not IDE, this device is still used
 	// by VirtualBox to deliver the guest extensions.
-	controllerName := "IDE Controller"
-	err = driver.VBoxManage("storagectl", vmName, "--name", controllerName, "--add", "ide")
+	err = driver.VBoxManage("storagectl", vmName, "--name", "IDE Controller", "--add", "ide")
 	if err != nil {
 		err := fmt.Errorf("Error creating disk controller: %s", err)
 		state.Put("error", err)
@@ -55,9 +54,8 @@ func (s *stepCreateDisk) Run(state multistep.StateBag) multistep.StepAction {
 	// Add a SATA controller if we were asked to use SATA. We still attach
 	// the IDE controller above because some other things (disks) require
 	// that.
-	if config.HardDriveInterface == "sata" {
-		controllerName = "SATA Controller"
-		if err := driver.CreateSATAController(vmName, controllerName); err != nil {
+	if config.HardDriveInterface == "sata" || config.ISOInterface == "sata" {
+		if err := driver.CreateSATAController(vmName, "SATA Controller"); err != nil {
 			err := fmt.Errorf("Error creating disk controller: %s", err)
 			state.Put("error", err)
 			ui.Error(err.Error())
@@ -66,6 +64,11 @@ func (s *stepCreateDisk) Run(state multistep.StateBag) multistep.StepAction {
 	}
 
 	// Attach the disk to the controller
+	controllerName := "IDE Controller"
+	if config.HardDriveInterface == "sata" {
+		controllerName = "SATA Controller"
+	}
+
 	command = []string{
 		"storageattach", vmName,
 		"--storagectl", controllerName,

website/source/docs/builders/virtualbox-iso.html.markdown

@@ -156,6 +156,10 @@ each category, the available options are alphabetized and described.
   server to be on one port, make this minimum and maximum port the same.
   By default the values are 8000 and 9000, respectively.
 
+* `iso_interface` (string) - The type of controller that the ISO is attached
+  to, defaults to "ide".  When set to "sata", the drive is attached to an
+  AHCI SATA controller.
+
 * `iso_urls` (array of strings) - Multiple URLs for the ISO to download.
   Packer will try these in order. If anything goes wrong attempting to download
   or while downloading a single URL, it will move on to the next. All URLs