From 0d84d799eae145be1bcfe2b9e46fb74abe057bda Mon Sep 17 00:00:00 2001 From: Manohar Castelino Date: Wed, 5 Dec 2018 18:03:20 -0800 Subject: [PATCH] virtio-mmio: Add support for virtio-mmio Start adding support for virtio-mmio devices starting with block. The devices show within the vm as vda, vdb,... based on order of insertion and such within the VM resemble virtio-blk devices. They need to be explicitly differentiated to ensure that the agent logic within the VM can discover and mount them appropropriately. The agent uses PCI location to discover them for virtio-blk. For virtio-mmio we need to use the predicted device name for now. Note: Kata used a disk for the VM rootfs in the case of Firecracker. (Instead of initrd or virtual-nvdimm). The Kata code today does not handle this case properly. For now as Firecracker is the only Hypervisor in Kata that uses virtio-mmio directly offset the drive index to comprehend this. Longer term we should track if the rootfs is setup as a block device explicitly. Fixes: #1046 Signed-off-by: Sebastien Boeuf Signed-off-by: Manohar Castelino --- pkg/katautils/config.go | 11 +++++--- virtcontainers/device/config/config.go | 14 ++++++++++ virtcontainers/device/drivers/block.go | 34 ++++++++++++++++++------ virtcontainers/device/manager/manager.go | 6 ++++- virtcontainers/hyperstart_agent.go | 3 ++- virtcontainers/hypervisor.go | 4 ++- virtcontainers/kata_agent.go | 20 +++++++++++--- virtcontainers/kata_agent_test.go | 9 ++++++- virtcontainers/qemu.go | 6 ++--- virtcontainers/qemu_arch_base.go | 8 ------ virtcontainers/sandbox_test.go | 10 +++---- 11 files changed, 90 insertions(+), 35 deletions(-) diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go index 204a3e5b3..2231859b8 100644 --- a/pkg/katautils/config.go +++ b/pkg/katautils/config.go @@ -15,6 +15,7 @@ import ( "github.com/BurntSushi/toml" vc "github.com/kata-containers/runtime/virtcontainers" + "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" "github.com/kata-containers/runtime/virtcontainers/utils" "github.com/sirupsen/logrus" @@ -292,15 +293,19 @@ func (h hypervisor) defaultBridges() uint32 { } func (h hypervisor) blockDeviceDriver() (string, error) { + supportedBlockDrivers := []string{config.VirtioSCSI, config.VirtioBlock, config.VirtioMmio} + if h.BlockDeviceDriver == "" { return defaultBlockDeviceDriver, nil } - if h.BlockDeviceDriver != vc.VirtioSCSI && h.BlockDeviceDriver != vc.VirtioBlock { - return "", fmt.Errorf("Invalid value %s provided for hypervisor block storage driver, can be either %s or %s", h.BlockDeviceDriver, vc.VirtioSCSI, vc.VirtioBlock) + for _, b := range supportedBlockDrivers { + if b == h.BlockDeviceDriver { + return h.BlockDeviceDriver, nil + } } - return h.BlockDeviceDriver, nil + return "", fmt.Errorf("Invalid hypervisor block storage driver %v specified (supported drivers: %v)", h.BlockDeviceDriver, supportedBlockDrivers) } func (h hypervisor) msize9p() uint32 { diff --git a/virtcontainers/device/config/config.go b/virtcontainers/device/config/config.go index 5895512f3..704160920 100644 --- a/virtcontainers/device/config/config.go +++ b/virtcontainers/device/config/config.go @@ -38,6 +38,17 @@ const ( VhostUserBlk = "vhost-user-blk-pci" ) +const ( + // VirtioMmio means use virtio-mmio for mmio based drives + VirtioMmio = "virtio-mmio" + + // VirtioBlock means use virtio-blk for hotplugging drives + VirtioBlock = "virtio-blk" + + // VirtioSCSI means use virtio-scsi for hotplugging drives + VirtioSCSI = "virtio-scsi" +) + // Defining these as a variable instead of a const, to allow // overriding this in the tests. @@ -98,6 +109,9 @@ type BlockDrive struct { // Index assigned to the drive. In case of virtio-scsi, this is used as SCSI LUN index Index int + // MmioAddr is used to identify the slot at which the drive is attached (order?). + MmioAddr string + // PCIAddr is the PCI address used to identify the slot at which the drive is attached. PCIAddr string diff --git a/virtcontainers/device/drivers/block.go b/virtcontainers/device/drivers/block.go index cbbe7bb31..ba0592b72 100644 --- a/virtcontainers/device/drivers/block.go +++ b/virtcontainers/device/drivers/block.go @@ -67,21 +67,39 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) { Index: index, } - driveName, err := utils.GetVirtDriveName(index) - if err != nil { - return err - } - customOptions := device.DeviceInfo.DriverOptions - if customOptions != nil && customOptions["block-driver"] == "virtio-blk" { - drive.VirtPath = filepath.Join("/dev", driveName) - } else { + if customOptions == nil || + customOptions["block-driver"] == "virtio-scsi" { + // User has not chosen a specific block device type + // Default to SCSI scsiAddr, err := utils.GetSCSIAddress(index) if err != nil { return err } drive.SCSIAddr = scsiAddr + } else { + var globalIdx int + + switch customOptions["block-driver"] { + case "virtio-blk": + globalIdx = index + case "virtio-mmio": + //With firecracker the rootfs for the VM itself + //sits at /dev/vda and consumes the first index. + //Longer term block based VM rootfs should be added + //as a regular block device which eliminates the + //offset. + //https://github.com/kata-containers/runtime/issues/1061 + globalIdx = index + 1 + } + + driveName, err := utils.GetVirtDriveName(globalIdx) + if err != nil { + return err + } + + drive.VirtPath = filepath.Join("/dev", driveName) } deviceLogger().WithField("device", device.DeviceInfo.HostPath).Info("Attaching block device") diff --git a/virtcontainers/device/manager/manager.go b/virtcontainers/device/manager/manager.go index 5841b938d..066c30581 100644 --- a/virtcontainers/device/manager/manager.go +++ b/virtcontainers/device/manager/manager.go @@ -20,6 +20,8 @@ import ( ) const ( + // VirtioMmio indicates block driver is virtio-mmio based + VirtioMmio string = "virtio-mmio" // VirtioBlock indicates block driver is virtio-blk based VirtioBlock string = "virtio-blk" // VirtioSCSI indicates block driver is virtio-scsi based @@ -55,7 +57,9 @@ func NewDeviceManager(blockDriver string, devices []api.Device) api.DeviceManage dm := &deviceManager{ devices: make(map[string]api.Device), } - if blockDriver == VirtioBlock { + if blockDriver == VirtioMmio { + dm.blockDriver = VirtioMmio + } else if blockDriver == VirtioBlock { dm.blockDriver = VirtioBlock } else { dm.blockDriver = VirtioSCSI diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 67aff379f..ae1c60ecf 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -522,7 +522,8 @@ func (h *hyper) startOneContainer(sandbox *Sandbox, c *Container) error { if c.state.Fstype != "" { // Pass a drive name only in case of block driver - if sandbox.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock { + if sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioBlock || + sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioMmio { driveName, err := utils.GetVirtDriveName(c.state.BlockIndex) if err != nil { return err diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 8387674a7..4e3e22660 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -13,6 +13,8 @@ import ( "runtime" "strconv" "strings" + + "github.com/kata-containers/runtime/virtcontainers/device/config" ) // HypervisorType describes an hypervisor type. @@ -38,7 +40,7 @@ const ( defaultBridges = 1 - defaultBlockDriver = VirtioSCSI + defaultBlockDriver = config.VirtioSCSI ) // In some architectures the maximum number of vCPUs depends on the number of physical cores. diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 333a2d742..325d8b14c 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -57,6 +57,7 @@ var ( // CAP_NET_BIND_SERVICE capability may bind to these port numbers. vSockPort = 1024 kata9pDevType = "9p" + kataMmioBlkDevType = "mmioblk" kataBlkDevType = "blk" kataSCSIDevType = "scsi" sharedDir9pOptions = []string{"trans=virtio,version=9p2000.L,cache=mmap", "nodev"} @@ -831,10 +832,15 @@ func (k *kataAgent) appendDevices(deviceList []*grpc.Device, c *Container) []*gr ContainerPath: dev.ContainerPath, } - if d.SCSIAddr == "" { + switch c.sandbox.config.HypervisorConfig.BlockDeviceDriver { + case config.VirtioMmio: + kataDevice.Type = kataMmioBlkDevType + kataDevice.Id = d.VirtPath + kataDevice.VmPath = d.VirtPath + case config.VirtioBlock: kataDevice.Type = kataBlkDevType kataDevice.Id = d.PCIAddr - } else { + case config.VirtioSCSI: kataDevice.Type = kataSCSIDevType kataDevice.Id = d.SCSIAddr } @@ -883,7 +889,10 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat return nil, fmt.Errorf("malformed block drive") } - if sandbox.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock { + if sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioMmio { + rootfs.Driver = kataMmioBlkDevType + rootfs.Source = blockDrive.VirtPath + } else if sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioBlock { rootfs.Driver = kataBlkDevType rootfs.Source = blockDrive.PCIAddr } else { @@ -1086,9 +1095,12 @@ func (k *kataAgent) handleBlockVolumes(c *Container) []*grpc.Storage { k.Logger().Error("malformed block drive") continue } - if c.sandbox.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock { + if c.sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioBlock { vol.Driver = kataBlkDevType vol.Source = blockDrive.PCIAddr + } else if c.sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioMmio { + vol.Driver = kataMmioBlkDevType + vol.Source = blockDrive.VirtPath } else { vol.Driver = kataSCSIDevType vol.Source = blockDrive.SCSIAddr diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index b65b4086d..88b43e652 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -402,9 +402,16 @@ func TestAppendDevices(t *testing.T) { }, } + sandboxConfig := &SandboxConfig{ + HypervisorConfig: HypervisorConfig{ + BlockDeviceDriver: config.VirtioBlock, + }, + } + c := &Container{ sandbox: &Sandbox{ - devManager: manager.NewDeviceManager("virtio-scsi", ctrDevices), + devManager: manager.NewDeviceManager("virtio-blk", ctrDevices), + config: sandboxConfig, }, } c.devices = append(c.devices, ContainerDevice{ diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 051db4cb2..029c1794e 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -372,7 +372,7 @@ func (q *qemu) buildDevices(initrdPath string) ([]govmmQemu.Device, *govmmQemu.I } var ioThread *govmmQemu.IOThread - if q.config.BlockDeviceDriver == VirtioSCSI { + if q.config.BlockDeviceDriver == config.VirtioSCSI { devices, ioThread = q.arch.appendSCSIController(devices, q.config.EnableIOThreads) } @@ -746,7 +746,7 @@ func (q *qemu) hotplugBlockDevice(drive *config.BlockDrive, op operation) error return err } - if q.config.BlockDeviceDriver == VirtioBlock { + if q.config.BlockDeviceDriver == config.VirtioBlock { driver := "virtio-blk-pci" addr, bridge, err := q.addDeviceToBridge(drive.ID) if err != nil { @@ -776,7 +776,7 @@ func (q *qemu) hotplugBlockDevice(drive *config.BlockDrive, op operation) error } } } else { - if q.config.BlockDeviceDriver == VirtioBlock { + if q.config.BlockDeviceDriver == config.VirtioBlock { if err := q.removeDeviceFromBridge(drive.ID); err != nil { return err } diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index 864edfe37..53e952410 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -128,14 +128,6 @@ const ( // 0 is reserved. const bridgePCIStartAddr = 2 -const ( - // VirtioBlock means use virtio-blk for hotplugging drives - VirtioBlock = "virtio-blk" - - // VirtioSCSI means use virtio-scsi for hotplugging drives - VirtioSCSI = "virtio-scsi" -) - const ( // QemuPCLite is the QEMU pc-lite machine type for amd64 QemuPCLite = "pc-lite" diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index ce4daad4c..f63f2e3fc 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1553,7 +1553,7 @@ func TestAttachBlockDevice(t *testing.T) { hypervisor := &mockHypervisor{} hConfig := HypervisorConfig{ - BlockDeviceDriver: VirtioBlock, + BlockDeviceDriver: config.VirtioBlock, } sconfig := &SandboxConfig{ @@ -1598,7 +1598,7 @@ func TestAttachBlockDevice(t *testing.T) { DevType: "b", } - dm := manager.NewDeviceManager(VirtioBlock, nil) + dm := manager.NewDeviceManager(config.VirtioBlock, nil) device, err := dm.NewDevice(deviceInfo) assert.Nil(t, err) _, ok := device.(*drivers.BlockDevice) @@ -1618,7 +1618,7 @@ func TestAttachBlockDevice(t *testing.T) { err = device.Detach(sandbox) assert.Nil(t, err) - container.sandbox.config.HypervisorConfig.BlockDeviceDriver = VirtioSCSI + container.sandbox.config.HypervisorConfig.BlockDeviceDriver = config.VirtioSCSI err = device.Attach(sandbox) assert.Nil(t, err) @@ -1638,14 +1638,14 @@ func TestPreAddDevice(t *testing.T) { hypervisor := &mockHypervisor{} hConfig := HypervisorConfig{ - BlockDeviceDriver: VirtioBlock, + BlockDeviceDriver: config.VirtioBlock, } sconfig := &SandboxConfig{ HypervisorConfig: hConfig, } - dm := manager.NewDeviceManager(VirtioBlock, nil) + dm := manager.NewDeviceManager(config.VirtioBlock, nil) // create a sandbox first sandbox := &Sandbox{ id: testSandboxID,