virtcontainers: Move bridge var in qemu type

In this way it is possible to set bridge variable for each arch when
instantiating the hypervisor.

Fixes: #1153

Signed-off-by: Alice Frosi <afrosi@de.ibm.com>
Co-authored-by: Jan Schintag <jan.schintag@de.ibm.com>

virtcontainers/qemu.go

@@ -266,9 +266,11 @@ func (q *qemu) setup(id string, hypervisorConfig *HypervisorConfig, vcStore *sto
 		create = true
 	}
 
+	q.arch.setBridges(q.state.Bridges)
+
 	if create {
 		q.Logger().Debug("Creating bridges")
-		q.state.Bridges = q.arch.bridges(q.config.DefaultBridges)
+		q.arch.bridges(q.config.DefaultBridges)
 
 		q.Logger().Debug("Creating UUID")
 		q.state.UUID = uuid.Generate().String()
@@ -403,7 +405,7 @@ func (q *qemu) buildDevices(initrdPath string) ([]govmmQemu.Device, *govmmQemu.I
 
 	// Add bridges before any other devices. This way we make sure that
 	// bridge gets the first available PCI address i.e bridgePCIStartAddr
-	devices = q.arch.appendBridges(devices, q.state.Bridges)
+	devices = q.arch.appendBridges(devices)
 
 	devices = q.arch.appendConsole(devices, console)
 
@@ -1986,6 +1988,7 @@ func (q *qemu) toGrpc() ([]byte, error) {
 
 func (q *qemu) storeState() error {
 	if q.store != nil {
+		q.state.Bridges = q.arch.getBridges()
 		if err := q.store.Store(store.Hypervisor, q.state); err != nil {
 			return err
 		}
@@ -2004,7 +2007,7 @@ func (q *qemu) save() (s persistapi.HypervisorState) {
 	s.HotpluggedMemory = q.state.HotpluggedMemory
 	s.HotplugVFIOOnRootBus = q.state.HotplugVFIOOnRootBus
 
-	for _, bridge := range q.state.Bridges {
+	for _, bridge := range q.arch.getBridges() {
 		s.Bridges = append(s.Bridges, persistapi.Bridge{
 			DeviceAddr: bridge.Devices,
 			Type:       string(bridge.Type),
@@ -2028,8 +2031,7 @@ func (q *qemu) load(s persistapi.HypervisorState) {
 	q.state.VirtiofsdPid = s.VirtiofsdPid
 
 	for _, bridge := range s.Bridges {
-		b := types.NewBridge(types.Type(bridge.Type), bridge.ID, bridge.DeviceAddr, bridge.Addr)
+		q.state.Bridges = append(q.state.Bridges, types.NewBridge(types.Type(bridge.Type), bridge.ID, bridge.DeviceAddr, bridge.Addr))
-		q.state.Bridges = append(q.state.Bridges, b)
 	}
 
 	for _, cpu := range s.HotpluggedVCPUs {

virtcontainers/qemu_amd64.go

@@ -122,8 +122,8 @@ func (q *qemuAmd64) capabilities() types.Capabilities {
 	return caps
 }
 
-func (q *qemuAmd64) bridges(number uint32) []types.Bridge {
+func (q *qemuAmd64) bridges(number uint32) {
-	return genericBridges(number, q.machineType)
+	q.Bridges = genericBridges(number, q.machineType)
 }
 
 func (q *qemuAmd64) cpuModel() string {
@@ -173,6 +173,6 @@ func (q *qemuAmd64) appendImage(devices []govmmQemu.Device, path string) ([]govm
 }
 
 // appendBridges appends to devices the given bridges
-func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device, bridges []types.Bridge) []govmmQemu.Device {
+func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device {
-	return genericAppendBridges(devices, bridges, q.machineType)
+	return genericAppendBridges(devices, q.Bridges, q.machineType)
 }

virtcontainers/qemu_amd64_test.go

@@ -40,7 +40,8 @@ func TestQemuAmd64Bridges(t *testing.T) {
 	amd64 := newTestQemu(QemuPC)
 	len := 5
 
-	bridges := amd64.bridges(uint32(len))
+	amd64.bridges(uint32(len))
+	bridges := amd64.getBridges()
 	assert.Len(bridges, len)
 
 	for i, b := range bridges {
@@ -51,7 +52,8 @@ func TestQemuAmd64Bridges(t *testing.T) {
 	}
 
 	amd64 = newTestQemu(QemuQ35)
-	bridges = amd64.bridges(uint32(len))
+	amd64.bridges(uint32(len))
+	bridges = amd64.getBridges()
 	assert.Len(bridges, len)
 
 	for i, b := range bridges {
@@ -62,7 +64,8 @@ func TestQemuAmd64Bridges(t *testing.T) {
 	}
 
 	amd64 = newTestQemu(QemuQ35 + QemuPC)
-	bridges = amd64.bridges(uint32(len))
+	amd64.bridges(uint32(len))
+	bridges = amd64.getBridges()
 	assert.Nil(bridges)
 }
 
@@ -143,10 +146,11 @@ func TestQemuAmd64AppendBridges(t *testing.T) {
 	// check PC
 	amd64 := newTestQemu(QemuPC)
 
-	bridges := amd64.bridges(1)
+	amd64.bridges(1)
+	bridges := amd64.getBridges()
 	assert.Len(bridges, 1)
 
-	devices = amd64.appendBridges(devices, bridges)
+	devices = amd64.appendBridges(devices)
 	assert.Len(devices, 1)
 
 	expectedOut := []govmmQemu.Device{
@@ -165,11 +169,12 @@ func TestQemuAmd64AppendBridges(t *testing.T) {
 	// Check Q35
 	amd64 = newTestQemu(QemuQ35)
 
-	bridges = amd64.bridges(1)
+	amd64.bridges(1)
+	bridges = amd64.getBridges()
 	assert.Len(bridges, 1)
 
 	devices = []govmmQemu.Device{}
-	devices = amd64.appendBridges(devices, bridges)
+	devices = amd64.appendBridges(devices)
 	assert.Len(devices, 1)
 
 	expectedOut = []govmmQemu.Device{

virtcontainers/qemu_arch_base.go

@@ -8,6 +8,7 @@ package virtcontainers
 import (
 	"context"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"os"
 	"strconv"
@@ -48,8 +49,8 @@ type qemuArch interface {
 	//capabilities returns the capabilities supported by QEMU
 	capabilities() types.Capabilities
 
-	// bridges returns the number bridges for the machine type
+	// bridges sets the number bridges for the machine type
-	bridges(number uint32) []types.Bridge
+	bridges(number uint32)
 
 	// cpuTopology returns the CPU topology for the given amount of vcpus
 	cpuTopology(vcpus, maxvcpus uint32) govmmQemu.SMP
@@ -70,7 +71,7 @@ type qemuArch interface {
 	appendSCSIController(devices []govmmQemu.Device, enableIOThreads bool) ([]govmmQemu.Device, *govmmQemu.IOThread)
 
 	// appendBridges appends bridges to devices
-	appendBridges(devices []govmmQemu.Device, bridges []types.Bridge) []govmmQemu.Device
+	appendBridges(devices []govmmQemu.Device) []govmmQemu.Device
 
 	// append9PVolume appends a 9P volume to devices
 	append9PVolume(devices []govmmQemu.Device, volume types.Volume) []govmmQemu.Device
@@ -96,6 +97,21 @@ type qemuArch interface {
 	// appendRNGDevice appends a RNG device to devices
 	appendRNGDevice(devices []govmmQemu.Device, rngDevice config.RNGDev) []govmmQemu.Device
 
+	// addDeviceToBridge adds devices to the bus
+	addDeviceToBridge(ID string, t types.Type) (string, types.Bridge, error)
+
+	// removeDeviceFromBridge removes devices to the bus
+	removeDeviceFromBridge(ID string) error
+
+	// getBridges grants access to Bridges
+	getBridges() []types.Bridge
+
+	// setBridges grants access to Bridges
+	setBridges(bridges []types.Bridge)
+
+	// addBridge adds a new Bridge to the list of Bridges
+	addBridge(types.Bridge)
+
 	// handleImagePath handles the Hypervisor Config image path
 	handleImagePath(config HypervisorConfig)
 
@@ -117,6 +133,7 @@ type qemuArchBase struct {
 	kernelParamsNonDebug  []Param
 	kernelParamsDebug     []Param
 	kernelParams          []Param
+	Bridges               []types.Bridge
 }
 
 const (
@@ -242,14 +259,10 @@ func (q *qemuArchBase) capabilities() types.Capabilities {
 	return caps
 }
 
-func (q *qemuArchBase) bridges(number uint32) []types.Bridge {
+func (q *qemuArchBase) bridges(number uint32) {
-	var bridges []types.Bridge
-
 	for i := uint32(0); i < number; i++ {
-		bridges = append(bridges, types.NewBridge(types.PCI, fmt.Sprintf("%s-bridge-%d", types.PCI, i), make(map[uint32]string), 0))
+		q.Bridges = append(q.Bridges, types.NewBridge(types.PCI, fmt.Sprintf("%s-bridge-%d", types.PCI, i), make(map[uint32]string), 0))
 	}
-
-	return bridges
 }
 
 func (q *qemuArchBase) cpuTopology(vcpus, maxvcpus uint32) govmmQemu.SMP {
@@ -347,14 +360,14 @@ func (q *qemuArchBase) appendSCSIController(devices []govmmQemu.Device, enableIO
 }
 
 // appendBridges appends to devices the given bridges
-func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device, bridges []types.Bridge) []govmmQemu.Device {
+func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device {
-	for idx, b := range bridges {
+	for idx, b := range q.Bridges {
 		t := govmmQemu.PCIBridge
 		if b.Type == types.PCIE {
 			t = govmmQemu.PCIEBridge
 		}
 
-		bridges[idx].Addr = bridgePCIStartAddr + idx
+		q.Bridges[idx].Addr = bridgePCIStartAddr + idx
 
 		devices = append(devices,
 			govmmQemu.BridgeDevice{
@@ -364,7 +377,7 @@ func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device, bridges []types
 				// Each bridge is required to be assigned a unique chassis id > 0
 				Chassis: idx + 1,
 				SHPC:    true,
-				Addr:    strconv.FormatInt(int64(bridges[idx].Addr), 10),
+				Addr:    strconv.FormatInt(int64(q.Bridges[idx].Addr), 10),
 			},
 		)
 	}
@@ -588,3 +601,53 @@ func (q *qemuArchBase) setIgnoreSharedMemoryMigrationCaps(ctx context.Context, q
 	})
 	return err
 }
+
+func (q *qemuArchBase) addDeviceToBridge(ID string, t types.Type) (string, types.Bridge, error) {
+	var err error
+	var addr uint32
+
+	if len(q.Bridges) == 0 {
+		return "", types.Bridge{}, errors.New("failed to get available address from bridges")
+	}
+
+	// looking for an empty address in the bridges
+	for _, b := range q.Bridges {
+		if t != b.Type {
+			continue
+		}
+		addr, err = b.AddDevice(ID)
+		if err == nil {
+			switch t {
+			case types.PCI, types.PCIE:
+				return fmt.Sprintf("%02x", addr), b, nil
+			}
+		}
+	}
+
+	return "", types.Bridge{}, fmt.Errorf("no more bridge slots available")
+}
+
+func (q *qemuArchBase) removeDeviceFromBridge(ID string) error {
+	var err error
+	for _, b := range q.Bridges {
+		err = b.RemoveDevice(ID)
+		if err == nil {
+			// device was removed correctly
+			return nil
+		}
+	}
+
+	return err
+}
+
+func (q *qemuArchBase) getBridges() []types.Bridge {
+	return q.Bridges
+}
+
+func (q *qemuArchBase) setBridges(bridges []types.Bridge) {
+	q.Bridges = bridges
+}
+
+func (q *qemuArchBase) addBridge(b types.Bridge) {
+	q.Bridges = append(q.Bridges, b)
+}

virtcontainers/qemu_arch_base_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
 	"github.com/kata-containers/runtime/virtcontainers/store"
 	"github.com/kata-containers/runtime/virtcontainers/types"
+	"github.com/pkg/errors"
 )
 
 const (
@@ -147,7 +148,8 @@ func TestQemuArchBaseBridges(t *testing.T) {
 	qemuArchBase := newQemuArchBase()
 	len := 5
 
-	bridges := qemuArchBase.bridges(uint32(len))
+	qemuArchBase.bridges(uint32(len))
+	bridges := qemuArchBase.getBridges()
 	assert.Len(bridges, len)
 
 	for i, b := range bridges {
@@ -158,6 +160,35 @@ func TestQemuArchBaseBridges(t *testing.T) {
 	}
 }
 
+func TestQemuAddDeviceToBridge(t *testing.T) {
+	assert := assert.New(t)
+
+	// addDeviceToBridge successfully
+	q := newQemuArchBase()
+	q.machineType = QemuPC
+
+	q.bridges(1)
+	for i := uint32(1); i <= types.PCIBridgeMaxCapacity; i++ {
+		_, _, err := q.addDeviceToBridge(fmt.Sprintf("qemu-bridge-%d", i), types.PCI)
+		assert.Nil(err)
+	}
+
+	// fail to add device to bridge cause no more available bridge slot
+	_, _, err := q.addDeviceToBridge("qemu-bridge-31", types.PCI)
+	exceptErr := errors.New("no more bridge slots available")
+	assert.Equal(exceptErr.Error(), err.Error())
+
+	// addDeviceToBridge fails cause q.Bridges == 0
+	q = newQemuArchBase()
+	q.machineType = QemuPCLite
+	q.bridges(0)
+	_, _, err = q.addDeviceToBridge("qemu-bridge", types.PCI)
+	if assert.Error(err) {
+		exceptErr = errors.New("failed to get available address from bridges")
+		assert.Equal(exceptErr.Error(), err.Error())
+	}
+}
+
 func TestQemuArchBaseCPUTopology(t *testing.T) {
 	assert := assert.New(t)
 	qemuArchBase := newQemuArchBase()
@@ -283,10 +314,11 @@ func TestQemuArchBaseAppendBridges(t *testing.T) {
 	assert := assert.New(t)
 	qemuArchBase := newQemuArchBase()
 
-	bridges := qemuArchBase.bridges(1)
+	qemuArchBase.bridges(1)
+	bridges := qemuArchBase.getBridges()
 	assert.Len(bridges, 1)
 
-	devices = qemuArchBase.appendBridges(devices, bridges)
+	devices = qemuArchBase.appendBridges(devices)
 	assert.Len(devices, 1)
 
 	expectedOut := []govmmQemu.Device{

virtcontainers/qemu_arm64.go

@@ -14,7 +14,6 @@ import (
 	"time"
 
 	govmmQemu "github.com/intel/govmm/qemu"
-	"github.com/kata-containers/runtime/virtcontainers/types"
 	"github.com/sirupsen/logrus"
 )
 
@@ -162,13 +161,13 @@ func newQemuArch(config HypervisorConfig) qemuArch {
 	return q
 }
 
-func (q *qemuArm64) bridges(number uint32) []types.Bridge {
+func (q *qemuArm64) bridges(number uint32) {
-	return genericBridges(number, q.machineType)
+	q.Bridges = genericBridges(number, q.machineType)
 }
 
 // appendBridges appends to devices the given bridges
-func (q *qemuArm64) appendBridges(devices []govmmQemu.Device, bridges []types.Bridge) []govmmQemu.Device {
+func (q *qemuArm64) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device {
-	return genericAppendBridges(devices, bridges, q.machineType)
+	return genericAppendBridges(devices, q.Bridges, q.machineType)
 }
 
 func (q *qemuArm64) appendImage(devices []govmmQemu.Device, path string) ([]govmmQemu.Device, error) {

virtcontainers/qemu_arm64_test.go

@@ -105,11 +105,12 @@ func TestQemuArm64AppendBridges(t *testing.T) {
 
 	arm64 := newTestQemu(QemuVirt)
 
-	bridges := arm64.bridges(1)
+	arm64.bridges(1)
+	bridges := arm64.getBridges()
 	assert.Len(bridges, 1)
 
 	devices = []govmmQemu.Device{}
-	devices = arm64.appendBridges(devices, bridges)
+	devices = arm64.appendBridges(devices)
 	assert.Len(devices, 1)
 
 	expectedOut := []govmmQemu.Device{

virtcontainers/qemu_ppc64le.go

@@ -105,8 +105,8 @@ func (q *qemuPPC64le) capabilities() types.Capabilities {
 	return caps
 }
 
-func (q *qemuPPC64le) bridges(number uint32) []types.Bridge {
+func (q *qemuPPC64le) bridges(number uint32) {
-	return genericBridges(number, q.machineType)
+	q.Bridges = genericBridges(number, q.machineType)
 }
 
 func (q *qemuPPC64le) cpuModel() string {
@@ -152,6 +152,6 @@ func (q *qemuPPC64le) appendImage(devices []govmmQemu.Device, path string) ([]go
 }
 
 // appendBridges appends to devices the given bridges
-func (q *qemuPPC64le) appendBridges(devices []govmmQemu.Device, bridges []types.Bridge) []govmmQemu.Device {
+func (q *qemuPPC64le) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device {
-	return genericAppendBridges(devices, bridges, q.machineType)
+	return genericAppendBridges(devices, q.Bridges, q.machineType)
 }

virtcontainers/qemu_s390x.go

@@ -11,7 +11,6 @@ import (
 
 	govmmQemu "github.com/intel/govmm/qemu"
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
-	"github.com/kata-containers/runtime/virtcontainers/types"
 )
 
 type qemuS390x struct {
@@ -82,13 +81,13 @@ func newQemuArch(config HypervisorConfig) qemuArch {
 	return q
 }
 
-func (q *qemuS390x) bridges(number uint32) []types.Bridge {
+func (q *qemuS390x) bridges(number uint32) {
-	return genericBridges(number, q.machineType)
+	q.Bridges = genericBridges(number, q.machineType)
 }
 
 // appendBridges appends to devices the given bridges
-func (q *qemuS390x) appendBridges(devices []govmmQemu.Device, bridges []types.Bridge) []govmmQemu.Device {
+func (q *qemuS390x) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device {
-	return genericAppendBridges(devices, bridges, q.machineType)
+	return genericAppendBridges(devices, q.Bridges, q.machineType)
 }
 
 // appendConsole appends a console to devices.

virtcontainers/qemu_test.go

@@ -421,44 +421,6 @@ func TestQemuGrpc(t *testing.T) {
 	assert.True(q.id == q2.id)
 }
 
-func TestQemuAddDeviceToBridge(t *testing.T) {
-	assert := assert.New(t)
-
-	config := newQemuConfig()
-	config.DefaultBridges = defaultBridges
-
-	// addDeviceToBridge successfully
-	config.HypervisorMachineType = QemuPC
-	q := &qemu{
-		config: config,
-		arch:   newQemuArch(config),
-	}
-
-	q.state.Bridges = q.arch.bridges(q.config.DefaultBridges)
-	// get pciBridgeMaxCapacity value from virtcontainers/types/pci.go
-	const pciBridgeMaxCapacity = 30
-	for i := uint32(1); i <= pciBridgeMaxCapacity; i++ {
-		_, _, err := q.addDeviceToBridge(fmt.Sprintf("qemu-bridge-%d", i))
-		assert.Nil(err)
-	}
-
-	// fail to add device to bridge cause no more available bridge slot
-	_, _, err := q.addDeviceToBridge("qemu-bridge-31")
-	exceptErr := errors.New("no more bridge slots available")
-	assert.Equal(exceptErr.Error(), err.Error())
-
-	// addDeviceToBridge fails cause q.state.Bridges == 0
-	config.HypervisorMachineType = QemuPCLite
-	q = &qemu{
-		config: config,
-		arch:   newQemuArch(config),
-	}
-	q.state.Bridges = q.arch.bridges(q.config.DefaultBridges)
-	_, _, err = q.addDeviceToBridge("qemu-bridge")
-	exceptErr = errors.New("failed to get available address from bridges")
-	assert.Equal(exceptErr.Error(), err.Error())
-}
-
 func TestQemuFileBackedMem(t *testing.T) {
 	assert := assert.New(t)