From dd927921c1f11d54cf380ec6f173d26502398996 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 26 Apr 2018 09:37:14 -0700 Subject: [PATCH 1/5] qemu: Return bridge itself with addDeviceToBridge instead of bridge bus Change the function to return the bridge itself that the device is attached to. This will allow bridge address to be used for determining the PCI slot of the device within the guest. Signed-off-by: Archana Shinde --- virtcontainers/qemu.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 7f006a5d66..abe57f5b2c 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -565,7 +565,7 @@ func (q *qemu) qmpSetup() (*govmmQemu.QMP, error) { return qmp, nil } -func (q *qemu) addDeviceToBridge(ID string) (string, string, error) { +func (q *qemu) addDeviceToBridge(ID string) (string, Bridge, error) { var err error var addr uint32 @@ -573,11 +573,11 @@ func (q *qemu) addDeviceToBridge(ID string) (string, string, error) { for _, b := range q.state.Bridges { addr, err = b.addDevice(ID) if err == nil { - return fmt.Sprintf("0x%x", addr), b.ID, nil + return fmt.Sprintf("%02x", addr), b, nil } } - return "", "", err + return "", Bridge{}, err } func (q *qemu) removeDeviceFromBridge(ID string) error { @@ -616,12 +616,12 @@ func (q *qemu) hotplugBlockDevice(drive Drive, op operation) error { if q.config.BlockDeviceDriver == VirtioBlock { driver := "virtio-blk-pci" - addr, bus, err := q.addDeviceToBridge(drive.ID) + addr, bridge, err := q.addDeviceToBridge(drive.ID) if err != nil { return err } - if err = q.qmpMonitorCh.qmp.ExecutePCIDeviceAdd(q.qmpMonitorCh.ctx, drive.ID, devID, driver, addr, bus); err != nil { + if err = q.qmpMonitorCh.qmp.ExecutePCIDeviceAdd(q.qmpMonitorCh.ctx, drive.ID, devID, driver, addr, bridge.ID); err != nil { return err } } else { @@ -676,12 +676,12 @@ func (q *qemu) hotplugVFIODevice(device VFIODevice, op operation) error { devID := "vfio-" + device.DeviceInfo.ID if op == addDevice { - addr, bus, err := q.addDeviceToBridge(devID) + addr, bridge, err := q.addDeviceToBridge(devID) if err != nil { return err } - if err := q.qmpMonitorCh.qmp.ExecutePCIVFIODeviceAdd(q.qmpMonitorCh.ctx, devID, device.BDF, addr, bus); err != nil { + if err := q.qmpMonitorCh.qmp.ExecutePCIVFIODeviceAdd(q.qmpMonitorCh.ctx, devID, device.BDF, addr, bridge.ID); err != nil { return err } } else { From 718dbd2a71ba8b6ce44849ec2c21fdd18c0cb42e Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 26 Apr 2018 09:52:42 -0700 Subject: [PATCH 2/5] device: Assign pci address for block devices Introduce a new field in Drive to store the PCI address if the drive is attached using virtio-blk. Assign PCI address in the format bridge-addr/device-addr. Since we need to assign the address while hotplugging, pass Drive by address. Signed-off-by: Archana Shinde --- virtcontainers/container.go | 4 ++-- virtcontainers/device.go | 7 +++++-- virtcontainers/qemu.go | 7 +++++-- virtcontainers/sandbox.go | 3 +++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index c861bf4c43..b7a8103300 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -788,7 +788,7 @@ func (c *Container) hotplugDrive() error { Index: driveIndex, } - if err := c.sandbox.hypervisor.hotplugAddDevice(drive, blockDev); err != nil { + if err := c.sandbox.hypervisor.hotplugAddDevice(&drive, blockDev); err != nil { return err } c.setStateHotpluggedDrive(true) @@ -813,7 +813,7 @@ func (c *Container) removeDrive() (err error) { c.Logger().Info("unplugging block device") devID := makeNameID("drive", c.id) - drive := Drive{ + drive := &Drive{ ID: devID, } diff --git a/virtcontainers/device.go b/virtcontainers/device.go index b47d59bcfa..77aeb64417 100644 --- a/virtcontainers/device.go +++ b/virtcontainers/device.go @@ -334,6 +334,9 @@ type BlockDevice struct { // Path at which the device appears inside the VM, outside of the container mount namespace VirtPath string + + // PCI Slot of the block device + PCIAddr string } func newBlockDevice(devInfo DeviceInfo) *BlockDevice { @@ -380,7 +383,7 @@ func (device *BlockDevice) attach(h hypervisor, c *Container) (err error) { deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Attaching block device") - if err = h.hotplugAddDevice(drive, blockDev); err != nil { + if err = h.hotplugAddDevice(&drive, blockDev); err != nil { return err } @@ -404,7 +407,7 @@ func (device BlockDevice) detach(h hypervisor) error { if device.DeviceInfo.Hotplugged { deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Unplugging block device") - drive := Drive{ + drive := &Drive{ ID: makeNameID("drive", device.DeviceInfo.ID), } diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index abe57f5b2c..fa1680d640 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -593,7 +593,7 @@ func (q *qemu) removeDeviceFromBridge(ID string) error { return err } -func (q *qemu) hotplugBlockDevice(drive Drive, op operation) error { +func (q *qemu) hotplugBlockDevice(drive *Drive, op operation) error { defer func(qemu *qemu) { if q.qmpMonitorCh.qmp != nil { q.qmpMonitorCh.qmp.Shutdown() @@ -621,6 +621,9 @@ func (q *qemu) hotplugBlockDevice(drive Drive, op operation) error { return err } + // PCI address is in the format bridge-addr/device-addr eg. "03/02" + drive.PCIAddr = fmt.Sprintf("%02x", bridge.Addr) + "/" + addr + if err = q.qmpMonitorCh.qmp.ExecutePCIDeviceAdd(q.qmpMonitorCh.ctx, drive.ID, devID, driver, addr, bridge.ID); err != nil { return err } @@ -700,7 +703,7 @@ func (q *qemu) hotplugVFIODevice(device VFIODevice, op operation) error { func (q *qemu) hotplugDevice(devInfo interface{}, devType deviceType, op operation) error { switch devType { case blockDev: - drive := devInfo.(Drive) + drive := devInfo.(*Drive) return q.hotplugBlockDevice(drive, op) case cpuDev: vcpus := devInfo.(uint32) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 8cc92dc435..daa7a7d13f 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -237,6 +237,9 @@ type Drive struct { // Index assigned to the drive. In case of virtio-scsi, this is used as SCSI LUN index Index int + + // PCIAddr is the PCI address used to identify the slot at which the drive is attached. + PCIAddr string } // EnvVar is a key/value structure representing a command From 85865f1a2c5bfd8f784c1bb449f9bef3e3b2488d Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 26 Apr 2018 10:28:40 -0700 Subject: [PATCH 3/5] bridge: Store the bridge address to state We need to store the bridge address to state to use it for assigning addresses to devices attached to teh bridge. So we need to make sure that the bridge pointer is assigned the address. Signed-off-by: Archana Shinde --- virtcontainers/qemu_amd64.go | 4 ++-- virtcontainers/qemu_arch_base.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/virtcontainers/qemu_amd64.go b/virtcontainers/qemu_amd64.go index d9b613a6b6..2e3406a921 100644 --- a/virtcontainers/qemu_amd64.go +++ b/virtcontainers/qemu_amd64.go @@ -205,7 +205,7 @@ func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device, bridges []Bridge) t = govmmQemu.PCIEBridge } - b.Addr = bridgePCIStartAddr + idx + bridges[idx].Addr = bridgePCIStartAddr + idx devices = append(devices, govmmQemu.BridgeDevice{ @@ -215,7 +215,7 @@ func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device, bridges []Bridge) // Each bridge is required to be assigned a unique chassis id > 0 Chassis: (idx + 1), SHPC: true, - Addr: strconv.FormatInt(int64(b.Addr), 10), + Addr: strconv.FormatInt(int64(bridges[idx].Addr), 10), }, ) } diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index 225428a283..18336ae474 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -328,7 +328,7 @@ func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device, bridges []Bridg t = govmmQemu.PCIEBridge } - b.Addr = bridgePCIStartAddr + idx + bridges[idx].Addr = bridgePCIStartAddr + idx devices = append(devices, govmmQemu.BridgeDevice{ @@ -338,7 +338,7 @@ func (q *qemuArchBase) appendBridges(devices []govmmQemu.Device, bridges []Bridg // Each bridge is required to be assigned a unique chassis id > 0 Chassis: (idx + 1), SHPC: true, - Addr: strconv.FormatInt(int64(b.Addr), 10), + Addr: strconv.FormatInt(int64(bridges[idx].Addr), 10), }, ) } From da08a65de3b70811aed3f3cc8f8511fc6fffa209 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 26 Apr 2018 10:34:31 -0700 Subject: [PATCH 4/5] device: Assign pci address to block device for kata_agent Store PCI address for a block device on hotplugging it via virtio-blk. This address will be passed by kata agent in the device "Id" field. The agent within the guest can then use this to identify the PCI slot in the guest and create the device node based on it. Signed-off-by: Archana Shinde --- virtcontainers/device.go | 1 + virtcontainers/kata_agent.go | 2 +- virtcontainers/kata_agent_test.go | 10 +++++----- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/virtcontainers/device.go b/virtcontainers/device.go index 77aeb64417..6b852bcbbb 100644 --- a/virtcontainers/device.go +++ b/virtcontainers/device.go @@ -391,6 +391,7 @@ func (device *BlockDevice) attach(h hypervisor, c *Container) (err error) { if c.sandbox.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock { device.VirtPath = filepath.Join("/dev", driveName) + device.PCIAddr = drive.PCIAddr } else { scsiAddr, err := getSCSIAddress(index) if err != nil { diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 2c2e8d464e..aa94ed3161 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -629,7 +629,7 @@ func (k *kataAgent) appendDevices(deviceList []*grpc.Device, devices []Device) [ if d.SCSIAddr == "" { kataDevice.Type = kataBlkDevType - kataDevice.VmPath = d.VirtPath + kataDevice.Id = d.PCIAddr } else { kataDevice.Type = kataSCSIDevType kataDevice.Id = d.SCSIAddr diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index d59ef9e486..6b58080a11 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -24,9 +24,9 @@ import ( ) var ( - testKataProxyURLTempl = "unix://%s/kata-proxy-test.sock" - testBlockDeviceVirtPath = "testBlockDeviceVirtPath" - testBlockDeviceCtrPath = "testBlockDeviceCtrPath" + testKataProxyURLTempl = "unix://%s/kata-proxy-test.sock" + testBlockDeviceCtrPath = "testBlockDeviceCtrPath" + testPCIAddr = "04/02" ) func proxyHandlerDiscard(c net.Conn) { @@ -366,16 +366,16 @@ func TestAppendDevices(t *testing.T) { expected := []*pb.Device{ { Type: kataBlkDevType, - VmPath: testBlockDeviceVirtPath, ContainerPath: testBlockDeviceCtrPath, + Id: testPCIAddr, }, } ctrDevices := []Device{ &BlockDevice{ - VirtPath: testBlockDeviceVirtPath, DeviceInfo: DeviceInfo{ ContainerPath: testBlockDeviceCtrPath, }, + PCIAddr: testPCIAddr, }, } From 717bc4cd26586e9d2aa7329303ff631a3178058f Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 26 Apr 2018 11:16:56 -0700 Subject: [PATCH 5/5] virtcontainers: Pass the PCI address for block based rootfs Store the PCI address of rootfs in case the rootfs is block based and passed using virtio-block. This helps up get rid of prdicting the device name inside the container for the block device. The agent will determine the device node name using the PCI address. Fixes #266 Signed-off-by: Archana Shinde --- virtcontainers/container.go | 16 ++++++++++++++++ virtcontainers/kata_agent.go | 10 +--------- virtcontainers/sandbox.go | 3 +++ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index b7a8103300..49faf41699 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -212,6 +212,17 @@ func (c *Container) setStateHotpluggedDrive(hotplugged bool) error { return nil } +func (c *Container) setContainerRootfsPCIAddr(addr string) error { + c.state.RootfsPCIAddr = addr + + err := c.sandbox.storage.storeContainerResource(c.sandbox.id, c.id, stateFileType, c.state) + if err != nil { + return err + } + + return nil +} + // GetAnnotations returns container's annotations func (c *Container) GetAnnotations() map[string]string { return c.config.Annotations @@ -791,6 +802,11 @@ func (c *Container) hotplugDrive() error { if err := c.sandbox.hypervisor.hotplugAddDevice(&drive, blockDev); err != nil { return err } + + if drive.PCIAddr != "" { + c.setContainerRootfsPCIAddr(drive.PCIAddr) + } + c.setStateHotpluggedDrive(true) if err := c.setStateBlockIndex(driveIndex); err != nil { diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index aa94ed3161..8e7c67aa38 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -40,7 +40,6 @@ var ( kataGuestSharedDir = "/run/kata-containers/shared/containers/" mountGuest9pTag = "kataShared" type9pFs = "9p" - devPath = "/dev" vsockSocketScheme = "vsock" kata9pDevType = "9p" kataBlkDevType = "blk" @@ -692,15 +691,8 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, // If virtio-scsi driver, the agent will be able to find the // device based on the provided address. if sandbox.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock { - // driveName is the predicted virtio-block guest name (the vd* in /dev/vd*). - driveName, err := getVirtDriveName(c.state.BlockIndex) - if err != nil { - return nil, err - } - virtPath := filepath.Join(devPath, driveName) - rootfs.Driver = kataBlkDevType - rootfs.Source = virtPath + rootfs.Source = c.state.RootfsPCIAddr } else { scsiAddr, err := getSCSIAddress(c.state.BlockIndex) if err != nil { diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index daa7a7d13f..52142e22e5 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -60,6 +60,9 @@ type State struct { // Bool to indicate if the drive for a container was hotplugged. HotpluggedDrive bool `json:"hotpluggedDrive"` + + // PCI slot at which the block device backing the container rootfs is attached. + RootfsPCIAddr string `json:"rootfsPCIAddr"` } // valid checks that the sandbox state is valid.