docs: Improvement regarding accel on windows

[ciarancourtney]

Some docs improvments related to accel arg, particularly on windows hosts

[hashicorp-cla]

All committers have signed the CLA.

[vheon]

why is setting the qemuargs directly is better then setting:

machine_type = "q35"
accelerator = "hvf:kvm:whpx:tcg"

??

[ciarancourtney]

why is setting the qemuargs directly is better then setting:

machine_type = "q35"
accelerator = "hvf:kvm:whpx:tcg"

??

Ultimately that would be better but its not supported in the current version, you can only set one accelerator. I had thought about submitting a PR to fallback to all possible accelerators and let qemu take the lead but thought it would introduce too much magic.

[vheon]

@ciarancourtney I didn't tried and only now I see this code here

packer-plugin-qemu/builder/qemu/config.go

Lines 810 to 813 in 8ad4058

	if _, ok := accels[c.Accelerator]; !ok {
	errs = packersdk.MultiErrorAppend(
	errs, errors.New("invalid accelerator, only 'kvm', 'tcg', 'xen', 'hax', 'hvf', 'whpx', or 'none' are allowed"))
	}

maybe if we do a simple split on the passed string and check that each section should be one of the supported one could work. Something like:

for _, accelerator := range strings.Split(c.Accelerator, ":") {
		if _, ok := accels[accelerator]; !ok {
			errs = packersdk.MultiErrorAppend(
			errs, errors.New("invalid accelerator, only 'kvm', 'tcg', 'xen', 'hax', 'hvf', 'whpx', or 'none' are allowed"))
		}
	}

probably none should be handled differently...

[nywilken]

@lbajolet-hashicorp when you get a second could you review and validate the documentation changes.

@vheon I don't enough to comment directly on your thoughts to improve the accelerator config settings. Maybe Lucas has thoughts. I recommend opening a separate issue for it so that it can be discussed or a PR to see how things would work.

[nywilken]

Why is this removed?

[lbajolet-hashicorp]

According to the commit that changes this, WHPX is supported by Stefan Weil's Windows qemu distribution since 2019, so in that sense I tend to agree that this can be removed from our docs.
I cannot test this however since I don't have a Windows machine on hand

[nywilken]

This change also requires the execution of go generate ./... to update the generated docs.

[lbajolet-hashicorp]

Regarding the acceleration methods, there's two alternatives that come to mind:

• Either we do as @vheon proposed, and allow the : syntax and validate that each are supported.
• We add a new attribute (accelerators for example), which will let us define multiple in order of preference, and pass it to qemu as a string with : in between each provided accelerator.

I agree with @nywilken though, this should probably be tracked in a separate issue.

Regarding none, as it stands, it's essentially to explicitely not define the accelerator attribute, which amounts to qemu's defualt behaviour, i.e. tcg.
When specifying a series of accelerators then, none should not be a valid choice, and should be rejected (or at worst ignored with a warning specifying why).

[lbajolet-hashicorp]

Hi @ciarancourtney,

Thanks for opening this, and sorry it was left open for so long.

I've taken a new look at the code, rebased it, and re-generated the docs from the changes in the code.
For future reference, if you want to suggest more changes to the documentation later, plase run make generate so the partials are up-to-date, otherwise we have some actions that will fail.

I'll merge this once tests go green, and we can keep an issue open for the accelerator choices. As pointed out in your commit, the : notation is now deprecated (refer to this thread for more information), so we should probably consider using multiple -accel options if/when we want to support this feature.

This being said, thanks again for opening this one @ciarancourtney!