Merge pull request #9597 from romantomjak/multiqueue-net-option

builder/proxmox: add support for multiple NIC packet queues

builder/proxmox/config.go

@@ -68,11 +68,12 @@ type Config struct {
 }
 
 type nicConfig struct {
-	Model      string `mapstructure:"model"`
+	Model        string `mapstructure:"model"`
-	MACAddress string `mapstructure:"mac_address"`
+	PacketQueues int    `mapstructure:"packet_queues"`
-	Bridge     string `mapstructure:"bridge"`
+	MACAddress   string `mapstructure:"mac_address"`
-	VLANTag    string `mapstructure:"vlan_tag"`
+	Bridge       string `mapstructure:"bridge"`
-	Firewall   bool   `mapstructure:"firewall"`
+	VLANTag      string `mapstructure:"vlan_tag"`
+	Firewall     bool   `mapstructure:"firewall"`
 }
 type diskConfig struct {
 	Type            string `mapstructure:"type"`
@@ -179,7 +180,7 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) {
 		// (currently zfspool|lvm|rbd|cephfs), the format parameter is mandatory. Make sure this is still up to date
 		// when updating the vendored code!
 		if !contains([]string{"zfspool", "lvm", "rbd", "cephfs"}, c.Disks[idx].StoragePoolType) && c.Disks[idx].DiskFormat == "" {
-			errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf("disk format must be specified for pool type %q", c.Disks[idx].StoragePoolType)))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("disk format must be specified for pool type %q", c.Disks[idx].StoragePoolType))
 		}
 	}
 	if c.SCSIController == "" {
@@ -222,7 +223,7 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) {
 		errs = packer.MultiErrorAppend(errs, errors.New("proxmox_url must be specified"))
 	}
 	if c.proxmoxURL, err = url.Parse(c.ProxmoxURLRaw); err != nil {
-		errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf("Could not parse proxmox_url: %s", err)))
+		errs = packer.MultiErrorAppend(errs, fmt.Errorf("Could not parse proxmox_url: %s", err))
 	}
 	if c.Node == "" {
 		errs = packer.MultiErrorAppend(errs, errors.New("node must be specified"))
@@ -232,15 +233,18 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) {
 	}
 	for idx := range c.NICs {
 		if c.NICs[idx].Bridge == "" {
-			errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf("network_adapters[%d].bridge must be specified", idx)))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("network_adapters[%d].bridge must be specified", idx))
+		}
+		if c.NICs[idx].Model != "virtio" && c.NICs[idx].PacketQueues > 0 {
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("network_adapters[%d].packet_queues can only be set for 'virtio' driver", idx))
 		}
 	}
 	for idx := range c.Disks {
 		if c.Disks[idx].StoragePool == "" {
-			errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf("disks[%d].storage_pool must be specified", idx)))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("disks[%d].storage_pool must be specified", idx))
 		}
 		if c.Disks[idx].StoragePoolType == "" {
-			errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf("disks[%d].storage_pool_type must be specified", idx)))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("disks[%d].storage_pool_type must be specified", idx))
 		}
 	}
 

builder/proxmox/config.hcl2spec.go

@@ -249,11 +249,12 @@ func (*FlatdiskConfig) HCL2Spec() map[string]hcldec.Spec {
 // FlatnicConfig is an auto-generated flat version of nicConfig.
 // Where the contents of a field with a `mapstructure:,squash` tag are bubbled up.
 type FlatnicConfig struct {
-	Model      *string `mapstructure:"model" cty:"model" hcl:"model"`
+	Model        *string `mapstructure:"model" cty:"model" hcl:"model"`
-	MACAddress *string `mapstructure:"mac_address" cty:"mac_address" hcl:"mac_address"`
+	PacketQueues *int    `mapstructure:"packet_queues" cty:"packet_queues" hcl:"packet_queues"`
-	Bridge     *string `mapstructure:"bridge" cty:"bridge" hcl:"bridge"`
+	MACAddress   *string `mapstructure:"mac_address" cty:"mac_address" hcl:"mac_address"`
-	VLANTag    *string `mapstructure:"vlan_tag" cty:"vlan_tag" hcl:"vlan_tag"`
+	Bridge       *string `mapstructure:"bridge" cty:"bridge" hcl:"bridge"`
-	Firewall   *bool   `mapstructure:"firewall" cty:"firewall" hcl:"firewall"`
+	VLANTag      *string `mapstructure:"vlan_tag" cty:"vlan_tag" hcl:"vlan_tag"`
+	Firewall     *bool   `mapstructure:"firewall" cty:"firewall" hcl:"firewall"`
 }
 
 // FlatMapstructure returns a new FlatnicConfig.
@@ -268,11 +269,12 @@ func (*nicConfig) FlatMapstructure() interface{ HCL2Spec() map[string]hcldec.Spe
 // The decoded values from this spec will then be applied to a FlatnicConfig.
 func (*FlatnicConfig) HCL2Spec() map[string]hcldec.Spec {
 	s := map[string]hcldec.Spec{
-		"model":       &hcldec.AttrSpec{Name: "model", Type: cty.String, Required: false},
+		"model":         &hcldec.AttrSpec{Name: "model", Type: cty.String, Required: false},
-		"mac_address": &hcldec.AttrSpec{Name: "mac_address", Type: cty.String, Required: false},
+		"packet_queues": &hcldec.AttrSpec{Name: "packet_queues", Type: cty.Number, Required: false},
-		"bridge":      &hcldec.AttrSpec{Name: "bridge", Type: cty.String, Required: false},
+		"mac_address":   &hcldec.AttrSpec{Name: "mac_address", Type: cty.String, Required: false},
-		"vlan_tag":    &hcldec.AttrSpec{Name: "vlan_tag", Type: cty.String, Required: false},
+		"bridge":        &hcldec.AttrSpec{Name: "bridge", Type: cty.String, Required: false},
-		"firewall":    &hcldec.AttrSpec{Name: "firewall", Type: cty.Bool, Required: false},
+		"vlan_tag":      &hcldec.AttrSpec{Name: "vlan_tag", Type: cty.String, Required: false},
+		"firewall":      &hcldec.AttrSpec{Name: "firewall", Type: cty.Bool, Required: false},
 	}
 	return s
 }

builder/proxmox/config_test.go

@@ -8,6 +8,17 @@ import (
 	"github.com/hashicorp/packer/template"
 )
 
+func mandatoryConfig(t *testing.T) map[string]interface{} {
+	return map[string]interface{}{
+		"proxmox_url":  "https://my-proxmox.my-domain:8006/api2/json",
+		"username":     "apiuser@pve",
+		"password":     "supersecret",
+		"iso_file":     "local:iso/Fedora-Server-dvd-x86_64-29-1.2.iso",
+		"node":         "my-proxmox",
+		"ssh_username": "root",
+	}
+}
+
 func TestRequiredParameters(t *testing.T) {
 	var c Config
 	_, err := c.Prepare(make(map[string]interface{}))
@@ -139,34 +150,61 @@ func TestBasicExampleFromDocsIsValid(t *testing.T) {
 }
 
 func TestAgentSetToFalse(t *testing.T) {
-	// only the mandatory attributes are specified
+	cfg := mandatoryConfig(t)
-	const config = `{
+	cfg["qemu_agent"] = false
-		"builders": [
-			{
-				"type": "proxmox",
-				"proxmox_url": "https://my-proxmox.my-domain:8006/api2/json",
-				"username": "apiuser@pve",
-				"password": "supersecret",
-				"iso_file": "local:iso/Fedora-Server-dvd-x86_64-29-1.2.iso",
-				"ssh_username": "root",
-				"node": "my-proxmox",
-				"qemu_agent": false
-			}
-		]
-	}`
 
-	tpl, err := template.Parse(strings.NewReader(config))
+	var c Config
-	if err != nil {
+	warn, err := c.Prepare(cfg)
-		t.Fatal(err)
-	}
-
-	b := &Builder{}
-	_, warn, err := b.Prepare(tpl.Builders["proxmox"].Config)
 	if err != nil {
 		t.Fatal(err, warn)
 	}
 
-	if b.config.Agent != false {
+	if c.Agent != false {
-		t.Errorf("Expected Agent to be false, got %t", b.config.Agent)
+		t.Errorf("Expected Agent to be false, got %t", c.Agent)
+	}
+}
+
+func TestPacketQueueSupportForNetworkAdapters(t *testing.T) {
+	drivertests := []struct {
+		expectedToFail bool
+		model          string
+	}{
+		{expectedToFail: false, model: "virtio"},
+		{expectedToFail: true, model: "e1000"},
+		{expectedToFail: true, model: "e1000-82540em"},
+		{expectedToFail: true, model: "e1000-82544gc"},
+		{expectedToFail: true, model: "e1000-82545em"},
+		{expectedToFail: true, model: "i82551"},
+		{expectedToFail: true, model: "i82557b"},
+		{expectedToFail: true, model: "i82559er"},
+		{expectedToFail: true, model: "ne2k_isa"},
+		{expectedToFail: true, model: "ne2k_pci"},
+		{expectedToFail: true, model: "pcnet"},
+		{expectedToFail: true, model: "rtl8139"},
+		{expectedToFail: true, model: "vmxnet3"},
+	}
+
+	for _, tt := range drivertests {
+		device := make(map[string]interface{})
+		device["bridge"] = "vmbr0"
+		device["model"] = tt.model
+		device["packet_queues"] = 2
+
+		devices := make([]map[string]interface{}, 0)
+		devices = append(devices, device)
+
+		cfg := mandatoryConfig(t)
+		cfg["network_adapters"] = devices
+
+		var c Config
+		_, err := c.Prepare(cfg)
+
+		if tt.expectedToFail == true && err == nil {
+			t.Error("expected config preparation to fail, but no error occured")
+		}
+
+		if tt.expectedToFail == false && err != nil {
+			t.Errorf("expected config preparation to succeed, but %s", err.Error())
+		}
 	}
 }

builder/proxmox/step_start_vm.go

@@ -112,6 +112,10 @@ func generateProxmoxNetworkAdapters(nics []nicConfig) proxmox.QemuDevices {
 		setDeviceParamIfDefined(devs[idx], "bridge", nics[idx].Bridge)
 		setDeviceParamIfDefined(devs[idx], "tag", nics[idx].VLANTag)
 		setDeviceParamIfDefined(devs[idx], "firewall", strconv.FormatBool(nics[idx].Firewall))
+
+		if nics[idx].PacketQueues > 0 {
+			devs[idx]["queues"] = nics[idx].PacketQueues
+		}
 	}
 	return devs
 }

website/pages/docs/builders/proxmox.mdx

@@ -141,6 +141,15 @@ builder.
 
   - `firewall` (bool) - If the interface should be protected by the firewall.
     Defaults to `false`.
+  
+  - `packet_queues` (int) - Number of packet queues to be used on the device.
+    Values greater than 1 indicate that the multiqueue feature is activated.
+    For best performance, set this to the number of cores available to the
+    virtual machine. CPU load on the host and guest systems will increase as
+    the traffic increases, so activate this option only when the VM has to
+    handle a great number of incoming connections, such as when the VM is
+    operating as a router, reverse proxy or a busy HTTP server. Requires
+    `virtio` network adapter. Defaults to `0`.
 
 - `disks` (array of objects) - Disks attached to the virtual machine.
   Example: