Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

[WIP] Break the hypervisor dependency from the endpoint interface #1233

Closed
wants to merge 2 commits into from
Closed

[WIP] Break the hypervisor dependency from the endpoint interface #1233

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4368961
virtcontainers: types: Add device types and structures
Feb 13, 2019
c885892
virtcontainers: hypervisor: Simplify the device handling interface
Feb 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion virtcontainers/bridgedmacvlan_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (endpoint *BridgedMacvlanEndpoint) Attach(h hypervisor) error {
return err
}

return h.addDevice(endpoint, types.NetDev)
return h.addDevice(types.Device{endpoint, types.NetDev})
}

// Detach for the virtual endpoint tears down the tap and bridge
Expand Down
34 changes: 15 additions & 19 deletions virtcontainers/fc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ func (fc *firecracker) startSandbox(timeout int) error {
fc.createDiskPool()

for _, d := range fc.pendingDevices {
if err = fc.addDevice(d.Info, d.Type); err != nil {
if err = fc.addDevice(d); err != nil {
return err
}
}
Expand Down Expand Up @@ -589,60 +589,56 @@ func (fc *firecracker) fcUpdateBlockDrive(drive config.BlockDrive) error {

// addDevice will add extra devices to firecracker. Limited to configure before the
// virtual machine starts. Devices include drivers and network interfaces only.
func (fc *firecracker) addDevice(devInfo interface{}, devType types.DeviceType) error {
func (fc *firecracker) addDevice(device types.Device) error {
span, _ := fc.trace("addDevice")
defer span.Finish()

fc.state.RLock()
defer fc.state.RUnlock()

if fc.state.state == notReady {
dev := types.Device{
Info: devInfo,
Type: devType,
}
fc.Logger().Info("FC not ready, queueing device")
fc.pendingDevices = append(fc.pendingDevices, dev)
fc.pendingDevices = append(fc.pendingDevices, device)
return nil
}

switch v := devInfo.(type) {
switch v := device.Info.(type) {
case Endpoint:
fc.Logger().WithField("device-type-endpoint", devInfo).Info("Adding device")
fc.Logger().WithField("device-type-endpoint", device.Info).Info("Adding device")
return fc.fcAddNetDevice(v)
case config.BlockDrive:
fc.Logger().WithField("device-type-blockdrive", devInfo).Info("Adding device")
fc.Logger().WithField("device-type-blockdrive", device.Info).Info("Adding device")
return fc.fcAddBlockDrive(v)
case kataVSOCK:
fc.Logger().WithField("device-type-vsock", devInfo).Info("Adding device")
fc.Logger().WithField("device-type-vsock", device.Info).Info("Adding device")
return fc.fcAddVsock(v)
default:
fc.Logger().WithField("unknown-device-type", devInfo).Error("Adding device")
fc.Logger().WithField("unknown-device-type", device.Info).Error("Adding device")
break
}

return nil
}

// hotplugAddDevice supported in Firecracker VMM
func (fc *firecracker) hotplugAddDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error) {
func (fc *firecracker) hotplugAddDevice(device types.Device) (interface{}, error) {
span, _ := fc.trace("hotplugAddDevice")
defer span.Finish()

switch devType {
switch device.Type {
case types.BlockDev:
//The drive placeholder has to exist prior to Update
return nil, fc.fcUpdateBlockDrive(*devInfo.(*config.BlockDrive))
return nil, fc.fcUpdateBlockDrive(*device.Info.(*config.BlockDrive))
default:
fc.Logger().WithFields(logrus.Fields{"devInfo": devInfo,
"types.DeviceType": devType}).Warn("hotplugAddDevice: unsupported device")
fc.Logger().WithFields(logrus.Fields{"devInfo": device.Info,
"types.DeviceType": device.Type}).Warn("hotplugAddDevice: unsupported device")
return nil, fmt.Errorf("hotplugAddDevice: unsupported device: devInfo:%v, types.DeviceType%v",
devInfo, devType)
device.Info, device.Type)
}
}

// hotplugRemoveDevice supported in Firecracker VMM, but no-op
func (fc *firecracker) hotplugRemoveDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error) {
func (fc *firecracker) hotplugRemoveDevice(types.Device) (interface{}, error) {
return nil, nil
}

Expand Down
4 changes: 2 additions & 2 deletions virtcontainers/hyperstart_agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func (h *hyper) getSharePath(id string) string {

func (h *hyper) configure(hv hypervisor, id, sharePath string, builtin bool, config interface{}) error {
for _, socket := range h.sockets {
err := hv.addDevice(socket, types.SerialPortDev)
err := hv.addDevice(types.Device{socket, types.SerialPortDev})
if err != nil {
return err
}
Expand All @@ -324,7 +324,7 @@ func (h *hyper) configure(hv hypervisor, id, sharePath string, builtin bool, con
return err
}

return hv.addDevice(sharedVolume, types.FsDev)
return hv.addDevice(types.Device{sharedVolume, types.FsDev})
}

func (h *hyper) createSandbox(sandbox *Sandbox) (err error) {
Expand Down
6 changes: 3 additions & 3 deletions virtcontainers/hypervisor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,9 @@ type hypervisor interface {
pauseSandbox() error
saveSandbox() error
resumeSandbox() error
addDevice(devInfo interface{}, devType types.DeviceType) error
hotplugAddDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error)
hotplugRemoveDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error)
addDevice(types.Device) error
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate how this change will help decouple the hypervisor from the endpoint dependency?
From what I understand here, we're simply moving the device info as part of the device structure itself, but the detection of the device types such as Endpoint is still happening inside each hypervisor implementation, right?

hotplugAddDevice(types.Device) (interface{}, error)
hotplugRemoveDevice(types.Device) (interface{}, error)
resizeMemory(memMB uint32, memoryBlockSizeMB uint32) (uint32, error)
resizeVCPUs(vcpus uint32) (uint32, uint32, error)
getSandboxConsole(sandboxID string) (string, error)
Expand Down
2 changes: 1 addition & 1 deletion virtcontainers/ipvlan_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (endpoint *IPVlanEndpoint) Attach(h hypervisor) error {
return err
}

return h.addDevice(endpoint, types.NetDev)
return h.addDevice(types.Device{endpoint, types.NetDev})
}

// Detach for the virtual endpoint tears down the tap and bridge
Expand Down
6 changes: 3 additions & 3 deletions virtcontainers/kata_agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func (k *kataAgent) configure(h hypervisor, id, sharePath string, builtin bool,

switch s := k.vmSocket.(type) {
case types.Socket:
err := h.addDevice(s, types.SerialPortDev)
err := h.addDevice(types.Device{s, types.SerialPortDev})
if err != nil {
return err
}
Expand All @@ -247,7 +247,7 @@ func (k *kataAgent) configure(h hypervisor, id, sharePath string, builtin bool,
return err
}
s.port = uint32(vSockPort)
if err := h.addDevice(s, types.VSockPCIDev); err != nil {
if err := h.addDevice(types.Device{s, types.VSockPCIDev}); err != nil {
return err
}
k.vmSocket = s
Expand Down Expand Up @@ -277,7 +277,7 @@ func (k *kataAgent) configure(h hypervisor, id, sharePath string, builtin bool,
return err
}

return h.addDevice(sharedVolume, types.FsDev)
return h.addDevice(types.Device{sharedVolume, types.FsDev})
}

func (k *kataAgent) createSandbox(sandbox *Sandbox) error {
Expand Down
2 changes: 1 addition & 1 deletion virtcontainers/macvtap_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (endpoint *MacvtapEndpoint) Attach(h hypervisor) error {
endpoint.VhostFds = vhostFds
}

return h.addDevice(endpoint, types.NetDev)
return h.addDevice(types.Device{endpoint, types.NetDev})
}

// Detach for macvtap endpoint does nothing.
Expand Down
16 changes: 8 additions & 8 deletions virtcontainers/mock_hypervisor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,25 @@ func (m *mockHypervisor) saveSandbox() error {
return nil
}

func (m *mockHypervisor) addDevice(devInfo interface{}, devType types.DeviceType) error {
func (m *mockHypervisor) addDevice(device types.Device) error {
return nil
}

func (m *mockHypervisor) hotplugAddDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error) {
switch devType {
func (m *mockHypervisor) hotplugAddDevice(device types.Device) (interface{}, error) {
switch device.Type {
case types.CPUDev:
return devInfo.(uint32), nil
return device.Info.(uint32), nil
case types.MemoryDev:
memdev := devInfo.(*types.MemoryDevice)
memdev := device.Info.(*types.MemoryDevice)
return memdev.SizeMB, nil
}
return nil, nil
}

func (m *mockHypervisor) hotplugRemoveDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error) {
switch devType {
func (m *mockHypervisor) hotplugRemoveDevice(device types.Device) (interface{}, error) {
switch device.Type {
case types.CPUDev:
return devInfo.(uint32), nil
return device.Info.(uint32), nil
case types.MemoryDev:
return 0, nil
}
Expand Down
2 changes: 1 addition & 1 deletion virtcontainers/mock_hypervisor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestMockHypervisorStopSandbox(t *testing.T) {
func TestMockHypervisorAddDevice(t *testing.T) {
var m *mockHypervisor

if err := m.addDevice(nil, types.ImgDev); err != nil {
if err := m.addDevice(types.Device{nil, types.ImgDev}); err != nil {
t.Fatal(err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion virtcontainers/physical_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (endpoint *PhysicalEndpoint) Attach(h hypervisor) error {
BDF: endpoint.BDF,
}

return h.addDevice(d, types.VFIODev)
return h.addDevice(types.Device{d, types.VFIODev})
}

// Detach for physical endpoint unbinds the physical network interface from vfio-pci
Expand Down
20 changes: 10 additions & 10 deletions virtcontainers/qemu.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,23 +1012,23 @@ func (q *qemu) hotplugDevice(devInfo interface{}, devType types.DeviceType, op o
}
}

func (q *qemu) hotplugAddDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error) {
func (q *qemu) hotplugAddDevice(device types.Device) (interface{}, error) {
span, _ := q.trace("hotplugAddDevice")
defer span.Finish()

data, err := q.hotplugDevice(devInfo, devType, addDevice)
data, err := q.hotplugDevice(device.Info, device.Type, addDevice)
if err != nil {
return data, err
}

return data, q.store.Store(store.Hypervisor, q.state)
}

func (q *qemu) hotplugRemoveDevice(devInfo interface{}, devType types.DeviceType) (interface{}, error) {
func (q *qemu) hotplugRemoveDevice(device types.Device) (interface{}, error) {
span, _ := q.trace("hotplugRemoveDevice")
defer span.Finish()

data, err := q.hotplugDevice(devInfo, devType, removeDevice)
data, err := q.hotplugDevice(device.Info, device.Type, removeDevice)
if err != nil {
return data, err
}
Expand Down Expand Up @@ -1236,12 +1236,12 @@ func (q *qemu) resumeSandbox() error {
}

// addDevice will add extra devices to Qemu command line.
func (q *qemu) addDevice(devInfo interface{}, devType types.DeviceType) error {
func (q *qemu) addDevice(device types.Device) error {
var err error
span, _ := q.trace("addDevice")
defer span.Finish()

switch v := devInfo.(type) {
switch v := device.Info.(type) {
case types.Volume:
q.qemuConfig.Devices = q.arch.append9PVolume(q.qemuConfig.Devices, v)
case types.Socket:
Expand Down Expand Up @@ -1365,7 +1365,7 @@ func (q *qemu) resizeMemory(reqMemMB uint32, memoryBlockSizeMB uint32) (uint32,
addMemDevice := &types.MemoryDevice{
SizeMB: int(memHotplugMB),
}
data, err := q.hotplugAddDevice(addMemDevice, types.MemoryDev)
data, err := q.hotplugAddDevice(types.Device{addMemDevice, types.MemoryDev})
if err != nil {
return currentMemory, err
}
Expand All @@ -1385,7 +1385,7 @@ func (q *qemu) resizeMemory(reqMemMB uint32, memoryBlockSizeMB uint32) (uint32,
addMemDevice := &types.MemoryDevice{
SizeMB: int(memHotunplugMB),
}
data, err := q.hotplugRemoveDevice(addMemDevice, types.MemoryDev)
data, err := q.hotplugRemoveDevice(types.Device{addMemDevice, types.MemoryDev})
if err != nil {
return currentMemory, err
}
Expand Down Expand Up @@ -1529,7 +1529,7 @@ func (q *qemu) resizeVCPUs(reqVCPUs uint32) (currentVCPUs uint32, newVCPUs uint3
case currentVCPUs < reqVCPUs:
//hotplug
addCPUs := reqVCPUs - currentVCPUs
data, err := q.hotplugAddDevice(addCPUs, types.CPUDev)
data, err := q.hotplugAddDevice(types.Device{addCPUs, types.CPUDev})
if err != nil {
return currentVCPUs, newVCPUs, err
}
Expand All @@ -1541,7 +1541,7 @@ func (q *qemu) resizeVCPUs(reqVCPUs uint32) (currentVCPUs uint32, newVCPUs uint3
case currentVCPUs > reqVCPUs:
//hotunplug
removeCPUs := currentVCPUs - reqVCPUs
data, err := q.hotplugRemoveDevice(removeCPUs, types.CPUDev)
data, err := q.hotplugRemoveDevice(types.Device{removeCPUs, types.CPUDev})
if err != nil {
return currentVCPUs, newVCPUs, err
}
Expand Down
16 changes: 3 additions & 13 deletions virtcontainers/qemu_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func testQemuAddDevice(t *testing.T, devInfo interface{}, devType types.DeviceTy
arch: &qemuArchBase{},
}

err := q.addDevice(devInfo, devType)
err := q.addDevice(types.Device{devInfo, devType})
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -392,19 +392,9 @@ func TestHotplugUnsupportedDeviceType(t *testing.T) {
}
q.store = vcStore

_, err = q.hotplugAddDevice(
&types.MemoryDevice{
Slot: 0,
SizeMB: 128,
},
types.FsDev)
_, err = q.hotplugAddDevice(types.Device{&types.MemoryDevice{0, 128}, types.FsDev})
assert.Error(err)
_, err = q.hotplugRemoveDevice(
&types.MemoryDevice{
Slot: 0,
SizeMB: 128,
},
types.FsDev)
_, err = q.hotplugRemoveDevice(types.Device{&types.MemoryDevice{0, 128}, types.FsDev})
assert.Error(err)
}

Expand Down
10 changes: 5 additions & 5 deletions virtcontainers/sandbox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1500,7 +1500,7 @@ func (s *Sandbox) HotplugAddDevice(device api.Device, devType config.DeviceType)

// adding a group of VFIO devices
for _, dev := range vfioDevices {
if _, err := s.hypervisor.hotplugAddDevice(dev, types.VFIODev); err != nil {
if _, err := s.hypervisor.hotplugAddDevice(types.Device{dev, types.VFIODev}); err != nil {
s.Logger().
WithFields(logrus.Fields{
"sandbox": s.id,
Expand All @@ -1516,7 +1516,7 @@ func (s *Sandbox) HotplugAddDevice(device api.Device, devType config.DeviceType)
if !ok {
return fmt.Errorf("device type mismatch, expect device type to be %s", devType)
}
_, err := s.hypervisor.hotplugAddDevice(blockDevice.BlockDrive, types.BlockDev)
_, err := s.hypervisor.hotplugAddDevice(types.Device{blockDevice.BlockDrive, types.BlockDev})
return err
case config.DeviceGeneric:
// TODO: what?
Expand All @@ -1537,7 +1537,7 @@ func (s *Sandbox) HotplugRemoveDevice(device api.Device, devType config.DeviceTy

// remove a group of VFIO devices
for _, dev := range vfioDevices {
if _, err := s.hypervisor.hotplugRemoveDevice(dev, types.VFIODev); err != nil {
if _, err := s.hypervisor.hotplugRemoveDevice(types.Device{dev, types.VFIODev}); err != nil {
s.Logger().WithError(err).
WithFields(logrus.Fields{
"sandbox": s.id,
Expand All @@ -1553,7 +1553,7 @@ func (s *Sandbox) HotplugRemoveDevice(device api.Device, devType config.DeviceTy
if !ok {
return fmt.Errorf("device type mismatch, expect device type to be %s", devType)
}
_, err := s.hypervisor.hotplugRemoveDevice(blockDrive, types.BlockDev)
_, err := s.hypervisor.hotplugRemoveDevice(types.Device{blockDrive, types.BlockDev})
return err
case config.DeviceGeneric:
// TODO: what?
Expand All @@ -1580,7 +1580,7 @@ func (s *Sandbox) DecrementSandboxBlockIndex() error {
func (s *Sandbox) AppendDevice(device api.Device) error {
switch device.DeviceType() {
case config.VhostUserSCSI, config.VhostUserNet, config.VhostUserBlk:
return s.hypervisor.addDevice(device.GetDeviceInfo().(*config.VhostUserDeviceAttrs), types.VhostuserDev)
return s.hypervisor.addDevice(types.Device{device.GetDeviceInfo().(*config.VhostUserDeviceAttrs), types.VhostuserDev})
}
return fmt.Errorf("unsupported device type")
}
Expand Down
4 changes: 2 additions & 2 deletions virtcontainers/tap_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (endpoint *TapEndpoint) HotAttach(h hypervisor) error {
return err
}

if _, err := h.hotplugAddDevice(endpoint, types.NetDev); err != nil {
if _, err := h.hotplugAddDevice(types.Device{endpoint, types.NetDev}); err != nil {
networkLogger().WithError(err).Error("Error attach tap ep")
return err
}
Expand All @@ -104,7 +104,7 @@ func (endpoint *TapEndpoint) HotDetach(h hypervisor, netNsCreated bool, netNsPat
networkLogger().WithError(err).Warn("Error un-bridging tap ep")
}

if _, err := h.hotplugRemoveDevice(endpoint, types.NetDev); err != nil {
if _, err := h.hotplugRemoveDevice(types.Device{endpoint, types.NetDev}); err != nil {
networkLogger().WithError(err).Error("Error detach tap ep")
return err
}
Expand Down
Loading