diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index f718e55ee5..479d2517d3 100644 --- a/virtcontainers/qemu.go +++ b/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, b) + q.state.Bridges = append(q.state.Bridges, types.NewBridge(types.Type(bridge.Type), bridge.ID, bridge.DeviceAddr, bridge.Addr)) } for _, cpu := range s.HotpluggedVCPUs { diff --git a/virtcontainers/qemu_amd64.go b/virtcontainers/qemu_amd64.go index 6ab9c42502..7014083f31 100644 --- a/virtcontainers/qemu_amd64.go +++ b/virtcontainers/qemu_amd64.go @@ -122,8 +122,8 @@ func (q *qemuAmd64) capabilities() types.Capabilities { return caps } -func (q *qemuAmd64) bridges(number uint32) []types.Bridge { - return genericBridges(number, q.machineType) +func (q *qemuAmd64) bridges(number uint32) { + 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 { - return genericAppendBridges(devices, bridges, q.machineType) +func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { + return genericAppendBridges(devices, q.Bridges, q.machineType) } diff --git a/virtcontainers/qemu_amd64_test.go b/virtcontainers/qemu_amd64_test.go index eaec34cac7..176b97edb5 100644 --- a/virtcontainers/qemu_amd64_test.go +++ b/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{ diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index 1e0965ff3b..004006e96e 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/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(number uint32) []types.Bridge + // bridges sets the number bridges for the machine type + 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 { - var bridges []types.Bridge - +func (q *qemuArchBase) bridges(number uint32) { 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 { - for idx, b := range bridges { +func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { + 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) +} diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index ec91abf87a..ce853f90cc 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ 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{ diff --git a/virtcontainers/qemu_arm64.go b/virtcontainers/qemu_arm64.go index 24ebb9afd2..9e8549bbbe 100644 --- a/virtcontainers/qemu_arm64.go +++ b/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 { - return genericBridges(number, q.machineType) +func (q *qemuArm64) bridges(number uint32) { + 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 { - return genericAppendBridges(devices, bridges, q.machineType) +func (q *qemuArm64) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { + return genericAppendBridges(devices, q.Bridges, q.machineType) } func (q *qemuArm64) appendImage(devices []govmmQemu.Device, path string) ([]govmmQemu.Device, error) { diff --git a/virtcontainers/qemu_arm64_test.go b/virtcontainers/qemu_arm64_test.go index 46d75641ec..ff20d79419 100644 --- a/virtcontainers/qemu_arm64_test.go +++ b/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{ diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index b629717014..e86d17d885 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -105,8 +105,8 @@ func (q *qemuPPC64le) capabilities() types.Capabilities { return caps } -func (q *qemuPPC64le) bridges(number uint32) []types.Bridge { - return genericBridges(number, q.machineType) +func (q *qemuPPC64le) bridges(number uint32) { + 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 { - return genericAppendBridges(devices, bridges, q.machineType) +func (q *qemuPPC64le) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { + return genericAppendBridges(devices, q.Bridges, q.machineType) } diff --git a/virtcontainers/qemu_s390x.go b/virtcontainers/qemu_s390x.go index deec51b215..498e83f6b6 100644 --- a/virtcontainers/qemu_s390x.go +++ b/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 { - return genericBridges(number, q.machineType) +func (q *qemuS390x) bridges(number uint32) { + 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 { - return genericAppendBridges(devices, bridges, q.machineType) +func (q *qemuS390x) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { + return genericAppendBridges(devices, q.Bridges, q.machineType) } // appendConsole appends a console to devices. diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index e95f5d044a..62799b9b2f 100644 --- a/virtcontainers/qemu_test.go +++ b/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)