The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7525
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Some small clarification comments, and tweaks to constants and var naming for clarity.
From ed37a05870e842d5b6150c0c37f9530e2a8146bb Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
