Support .vhd file

[remi-espie]

This PR adds support for .vhd files.

Before this PR, when using a .vhd as the iso_url (as stated possible in the documentation), VirtualBox returns the error Error attaching ISO: VBoxManage error: VBoxManage.exe: error: The medium '<path/to/vhd>' can't be used as the requested device type (DVD, detected HDD).

This solves the issue by checking the extension of the file and, if it is .vhd, by changing the disk type from dvddrive to hdd in the VirtualBox' VM creation command.
If using a hdd type disk, it will not be unmounted before export.
It also removes the creation of a .vdi disk if using a .vhd.

However, by using directly the .vhd file, it is modified and won't remain the same after running packer.
An improvement would be to clone the .vhd file to the temp directory, as (for example) the Hyper-V plugin do.

This closes #123

[lbajolet-hashicorp]

Hi @remi-espie,

Thanks for the contribution, and apologies for taking so long to review!

The idea makes sense, however before we can consider merging it I would suggest not making the changes in-place, and instead as you point out, clone the disk to another disk, and boot on it at that time. This becomes especially relevant as reusing input as the target makes options like output_directory or output_filename incompatible with that capability, so if we decide to NOT proceed with copying, we should add checks during the Prepare step to make sure that no option gets ignored.

I believe this copy could be done even as a separate builder potentially? I feel like calling the builder ISO and clone a disk may prove slightly confusing, but I can imagine that being a possibility though, especially considering the iso_url docs do mention VHDs.

Thanks again, please let me know your thoughts and feel free to ping me when you're ready for another round of review, I'll take another look then!

[lbajolet-hashicorp]

This looks like the condition could be reversed to limit the amount of changes here?

Suggested change

	if path.Ext(state.Get("iso_path").(string)) != ".vhd" {
	if path.Ext(state.Get("iso_path").(string)) == ".vhd" {
	return multistep.ActionContinue
	}

[lbajolet-hashicorp]

Also, are we certain iso_path is always set? I worry accessing this and casting it to a string may crash the plugin if that's not the case.

[remi-espie]

Hello @lbajolet-hashicorp ,
Per documentation and per code we are sure that the iso_url is always set (in case of iso builder).
Btw, this is resolved in the following commits.

[hashicorp-cla-app]

All committers have signed the CLA.

[remi-espie]

With the latest commit, the source .vhd file is copied to a temporary directory randomly named with your tmp.Dir function, and the job is done on this copy.

The copy takes place during the attach_isos step of the common builder. I thought it made sense to put it here as it is where the file extension is also checked, and it made the modifications minimal.

As always, thanks for your review !