From 26ec5356686b32d213b11368515c06ef5848c1dc 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: 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/qemu.go | 6 ++--- virtcontainers/qemu_arch_base.go | 8 ------ virtcontainers/sandbox_test.go | 10 +++---- 10 files changed, 82 insertions(+), 34 deletions(-) diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go index 88ca34fefd..446048a907 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" @@ -293,15 +294,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 5895512f35..704160920d 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 cbbe7bb316..ba0592b729 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 5841b938da..066c30581c 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 1ed48489a7..f7c0697fa0 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 0be3ea96d2..f994128e62 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. @@ -43,7 +45,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 3d4924b0ea..6248d83210 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -59,6 +59,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"} @@ -847,10 +848,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 } @@ -899,7 +905,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 { @@ -1102,9 +1111,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/qemu.go b/virtcontainers/qemu.go index 1d152f653b..9dbb57ee14 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -370,7 +370,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) } @@ -744,7 +744,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 { @@ -774,7 +774,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 864edfe376..53e952410c 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 af89517042..d3c30391f7 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1554,7 +1554,7 @@ func TestAttachBlockDevice(t *testing.T) { hypervisor := &mockHypervisor{} hConfig := HypervisorConfig{ - BlockDeviceDriver: VirtioBlock, + BlockDeviceDriver: config.VirtioBlock, } sconfig := &SandboxConfig{ @@ -1600,7 +1600,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) @@ -1620,7 +1620,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) @@ -1640,14 +1640,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,