builder: don't enable iothread for all scsi

When a SCSI disk is specified for a configuration, the iothread argument
should only be set if requested, and not all the time.

Because of a priority problem though, all the SCSI disks ended-up with
iothread set, even if it was not requested.

This commit fixes that, and ensures it is fixed by adding some tests for
it.

builder/proxmox/common/step_start_vm.go

@@ -286,7 +286,7 @@ func generateProxmoxDisks(disks []diskConfig) proxmox.QemuDevices {
 		setDeviceParamIfDefined(devs[idx], "cache", disks[idx].CacheMode)
 		setDeviceParamIfDefined(devs[idx], "format", disks[idx].DiskFormat)
 
-		if devs[idx]["type"] == "scsi" || devs[idx]["type"] == "virtio" &&
+		if (devs[idx]["type"] == "scsi" || devs[idx]["type"] == "virtio") &&
 			disks[idx].IOThread {
 			devs[idx]["iothread"] = "true"
 		}

builder/proxmox/common/step_start_vm_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/Telmate/proxmox-api-go/proxmox"
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/packer-plugin-sdk/common"
 	"github.com/hashicorp/packer-plugin-sdk/multistep"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
@@ -498,3 +499,123 @@ func TestStartVM_AssertInitialQuemuConfig(t *testing.T) {
 		})
 	}
 }
+
+/*
+	setDeviceParamIfDefined(devs[idx], "type", disks[idx].Type)
+	setDeviceParamIfDefined(devs[idx], "size", disks[idx].Size)
+	setDeviceParamIfDefined(devs[idx], "storage", disks[idx].StoragePool)
+	setDeviceParamIfDefined(devs[idx], "cache", disks[idx].CacheMode)
+	setDeviceParamIfDefined(devs[idx], "format", disks[idx].DiskFormat)
+
+	if (devs[idx]["type"] == "scsi" || devs[idx]["type"] == "virtio") &&
+		disks[idx].IOThread {
+		devs[idx]["iothread"] = "true"
+	}
+
+	if disks[idx].Discard {
+		devs[idx]["discard"] = "on"
+	} else {
+		devs[idx]["discard"] = "ignore"
+	}
+
+	// Add SSD flag only if true
+	if disks[idx].SSD {
+		devs[idx]["ssd"] = "1"
+	}
+*/
+
+func TestGenerateProxmoxDisks(t *testing.T) {
+	tests := []struct {
+		name         string
+		disks        []diskConfig
+		expectOutput proxmox.QemuDevices
+	}{
+		{
+			"plain config, no special option set",
+			[]diskConfig{
+				{
+					Type:        "scsi",
+					StoragePool: "local-lvm",
+					Size:        "10G",
+					CacheMode:   "none",
+					DiskFormat:  "qcow2",
+					IOThread:    false,
+					Discard:     false,
+					SSD:         false,
+				},
+			},
+			proxmox.QemuDevices{
+				0: proxmox.QemuDevice{
+					"type":    "scsi",
+					"discard": "ignore",
+					"size":    "10G",
+					"storage": "local-lvm",
+					"cache":   "none",
+					"format":  "qcow2",
+				},
+			},
+		},
+		{
+			"scsi + iothread, iothread should be true",
+			[]diskConfig{
+				{
+					Type:        "scsi",
+					StoragePool: "local-lvm",
+					Size:        "10G",
+					CacheMode:   "none",
+					DiskFormat:  "qcow2",
+					IOThread:    true,
+					Discard:     false,
+					SSD:         false,
+				},
+			},
+			proxmox.QemuDevices{
+				0: proxmox.QemuDevice{
+					"type":     "scsi",
+					"discard":  "ignore",
+					"size":     "10G",
+					"storage":  "local-lvm",
+					"cache":    "none",
+					"format":   "qcow2",
+					"iothread": "true",
+				},
+			},
+		},
+		{
+			"virtio + iothread, iothread should be true",
+			[]diskConfig{
+				{
+					Type:        "virtio",
+					StoragePool: "local-lvm",
+					Size:        "10G",
+					CacheMode:   "none",
+					DiskFormat:  "qcow2",
+					IOThread:    true,
+					Discard:     false,
+					SSD:         false,
+				},
+			},
+			proxmox.QemuDevices{
+				0: proxmox.QemuDevice{
+					"type":     "virtio",
+					"discard":  "ignore",
+					"size":     "10G",
+					"storage":  "local-lvm",
+					"cache":    "none",
+					"format":   "qcow2",
+					"iothread": "true",
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			devs := generateProxmoxDisks(tt.disks)
+			diff := cmp.Diff(devs, tt.expectOutput)
+			if diff != "" {
+				t.Errorf("mismatch in produced qemu disks specs: %s", diff)
+			}
+		})
+	}
+}

go.mod

@@ -35,7 +35,7 @@ require (
 	github.com/gofrs/uuid v4.0.0+incompatible // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/go-cmp v0.5.9 // indirect
+	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.2.0 // indirect
 	github.com/googleapis/gax-go/v2 v2.6.0 // indirect

go.sum

@@ -115,6 +115,8 @@ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
+github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/martian/v3 v3.2.1 h1:d8MncMlErDFTwQGBK1xhv026j9kqhvw1Qv9IbWT1VLQ=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=