[lxc-devel] [lxd/master] VM: Bus allocation comments, constants and var naming

tomponline on Github lxc-bot at linuxcontainers.org
Fri Jun 12 13:48:43 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 387 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200612/46a800d1/attachment.bin>
-------------- next part --------------
From ed37a05870e842d5b6150c0c37f9530e2a8146bb Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 12 Jun 2020 14:46:15 +0100
Subject: [PATCH 1/2] lxd/instance/drivers/driver/qemu/bus: Adds comments,
 clarifies var names, and constants for defined multi-function groups

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu_bus.go | 38 +++++++++++++++----------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu_bus.go b/lxd/instance/drivers/driver_qemu_bus.go
index c38e47d572..9dbac794b4 100644
--- a/lxd/instance/drivers/driver_qemu_bus.go
+++ b/lxd/instance/drivers/driver_qemu_bus.go
@@ -5,6 +5,10 @@ import (
 	"strings"
 )
 
+const busFunctionGroupNone = ""           // Add a non multi-function port.
+const busFunctionGroupGeneric = "generic" // Add multi-function port to generic group (used for internal devices).
+const busFunctionGroup9p = "9p"           // Add multi-function port to 9p group (used for 9p shares).
+
 type qemuBusEntry struct {
 	bridgeDev int // Device number on the root bridge.
 	bridgeFn  int // Function number on the root bridge.
@@ -44,24 +48,23 @@ func (a *qemuBus) allocateRoot() *qemuBusEntry {
 	return a.rootPort
 }
 
-// allocate() does any needed port allocation and  returns the bus name,
+// allocate() does any needed port allocation and returns the bus name,
 // address and whether the device needs to be configured as multi-function.
 //
-// The group parameter allows for grouping devices together as a single
-// multi-function device. It automatically keeps track of the number of
-// functions already used and will allocate a new device as needed.
-func (a *qemuBus) allocate(group string) (string, string, bool) {
+// The multiFunctionGroup parameter allows for grouping devices together as one or more multi-function devices.
+// It automatically keeps track of the number of functions already used and will allocate new ports as needed.
+func (a *qemuBus) allocate(multiFunctionGroup string) (string, string, bool) {
 	if a.name == "ccw" {
 		return "", "", false
 	}
 
-	// Find a device group if specified.
+	// Find a device multi-function group if specified.
 	var p *qemuBusEntry
-	if group != "" {
+	if multiFunctionGroup != "" {
 		var ok bool
-		p, ok = a.entries[group]
+		p, ok = a.entries[multiFunctionGroup]
 		if ok {
-			// Check if group is full.
+			// Check if existing multi-function group is full.
 			if p.fn == 7 {
 				p.fn = 0
 				if a.name == "pci" {
@@ -76,7 +79,7 @@ func (a *qemuBus) allocate(group string) (string, string, bool) {
 				p.fn++
 			}
 		} else {
-			// Create a new group.
+			// Create a new multi-function group.
 			p = &qemuBusEntry{}
 
 			if a.name == "pci" {
@@ -88,10 +91,10 @@ func (a *qemuBus) allocate(group string) (string, string, bool) {
 				p.bridgeFn = r.bridgeFn
 			}
 
-			a.entries[group] = p
+			a.entries[multiFunctionGroup] = p
 		}
 	} else {
-		// Create a new temporary group.
+		// Create a temporary single function group.
 		p = &qemuBusEntry{}
 
 		if a.name == "pci" {
@@ -104,7 +107,8 @@ func (a *qemuBus) allocate(group string) (string, string, bool) {
 		}
 	}
 
-	multi := p.fn == 0 && group != ""
+	// The first device added to a multi-function port needs to specify the multi-function feature.
+	multi := p.fn == 0 && multiFunctionGroup != ""
 
 	if a.name == "pci" {
 		return "pci.0", fmt.Sprintf("%x.%d", p.bridgeDev, p.fn), multi
@@ -113,8 +117,10 @@ func (a *qemuBus) allocate(group string) (string, string, bool) {
 	if a.name == "pcie" {
 		if p.fn == 0 {
 			qemuPCIe.Execute(a.sb, map[string]interface{}{
-				"index":         a.portNum,
-				"addr":          fmt.Sprintf("%x.%d", p.bridgeDev, p.bridgeFn),
+				"index": a.portNum,
+				"addr":  fmt.Sprintf("%x.%d", p.bridgeDev, p.bridgeFn),
+
+				// First root port added on a bridge bus address needs multi-function enabled.
 				"multifunction": p.bridgeFn == 0,
 			})
 			p.dev = fmt.Sprintf("qemu_pcie%d", a.portNum)
@@ -127,6 +133,8 @@ func (a *qemuBus) allocate(group string) (string, string, bool) {
 	return "", "", false
 }
 
+// qemuNewBus instantiates a new qemu bus allocator. Accepts the type name of the bus and the qemu config builder
+// which it will use to write root port config entries too as ports are allocated.
 func qemuNewBus(name string, sb *strings.Builder) *qemuBus {
 	a := &qemuBus{
 		name: name,

From fc08574e1230e2a12e8809a627a6a80d50d9b6c5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 12 Jun 2020 14:46:57 +0100
Subject: [PATCH 2/2] lxd/instance/drivers/driver/qemu: Switches to
 multi-function group constants and adds comments

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu.go | 31 ++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 7c12346a98..7272f1ec68 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -1588,8 +1588,11 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 	// Setup the bus allocator.
 	bus := qemuNewBus(busName, sb)
 
-	// Now add the fixed set of devices.
-	devBus, devAddr, multi := bus.allocate("generic")
+	// Now add the fixed set of devices. The multi-function groups used for these fixed internal devices are
+	// specifically chosen to ensure that we consume exactly 4 PCI bus ports (on PCIe bus). This ensures that
+	// the first user device NIC added will use the 5th PCI bus port and will be consistently named enp5s0
+	// (which we need to maintain due for compatiblity with network configuration in our existing VM images).
+	devBus, devAddr, multi := bus.allocate(busFunctionGroupGeneric)
 	err = qemuBalloon.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1600,7 +1603,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("generic")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroupGeneric)
 	err = qemuRNG.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1611,7 +1614,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("generic")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroupGeneric)
 	err = qemuKeyboard.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1622,7 +1625,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("generic")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroupGeneric)
 	err = qemuTablet.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1633,7 +1636,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("generic")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroupGeneric)
 	err = qemuVsock.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1646,7 +1649,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("generic")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroupGeneric)
 	err = qemuSerial.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1659,7 +1662,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroupNone)
 	err = qemuSCSI.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1670,7 +1673,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("9p")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroup9p)
 	err = qemuDriveConfig.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1683,7 +1686,7 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 		return "", err
 	}
 
-	devBus, devAddr, multi = bus.allocate("")
+	devBus, devAddr, multi = bus.allocate(busFunctionGroupNone)
 	err = qemuGPU.Execute(sb, map[string]interface{}{
 		"bus":           bus.name,
 		"devBus":        devBus,
@@ -1704,6 +1707,10 @@ func (vm *qemu) generateQemuConfigFile(busName string, devConfs []*deviceConfig.
 
 	// Record the mounts we are going to do inside the VM using the agent.
 	agentMounts := []instancetype.VMAgentMount{}
+
+	// These devices are sorted so that NICs are added first to ensure that the first NIC can use the 5th
+	// PCI bus port and will be consistently named enp5s0 for compatiblity with network configuration in our
+	// existing VM images.
 	for _, runConf := range devConfs {
 		// Add drive devices.
 		if len(runConf.Mounts) > 0 {
@@ -1898,7 +1905,7 @@ func (vm *qemu) addDriveDirConfig(sb *strings.Builder, bus *qemuBus, fdFiles *[]
 	// Record the 9p mount for the agent.
 	*agentMounts = append(*agentMounts, agentMount)
 
-	devBus, devAddr, multi := bus.allocate("9p")
+	devBus, devAddr, multi := bus.allocate(busFunctionGroup9p)
 
 	// For read only shares, do not use proxy.
 	if shared.StringInSlice("ro", driveConf.Opts) {
@@ -2016,7 +2023,7 @@ func (vm *qemu) addNetDevConfig(sb *strings.Builder, bus *qemuBus, bootIndexes m
 		tpl = qemuNetdevPhysical
 	}
 
-	devBus, devAddr, multi := bus.allocate("")
+	devBus, devAddr, multi := bus.allocate(busFunctionGroupNone)
 	tplFields["devBus"] = devBus
 	tplFields["devAddr"] = devAddr
 	tplFields["multifunction"] = multi


More information about the lxc-devel mailing list