Merge pull request #3322 from Kvasscn/kata_dev_block_driver_option

device: using const strings for block-driver option instead of hard coding
This commit is contained in:
James O. D. Hunt 2022-03-21 10:56:25 +00:00 committed by GitHub
commit f8fb0d3bb6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 53 additions and 60 deletions

View File

@ -86,7 +86,7 @@ func TestContainerRemoveDrive(t *testing.T) {
sandbox := &Sandbox{ sandbox := &Sandbox{
ctx: context.Background(), ctx: context.Background(),
id: "sandbox", id: "sandbox",
devManager: manager.NewDeviceManager(manager.VirtioSCSI, false, "", nil), devManager: manager.NewDeviceManager(config.VirtioSCSI, false, "", nil),
config: &SandboxConfig{}, config: &SandboxConfig{},
} }
@ -320,7 +320,7 @@ func TestContainerAddDriveDir(t *testing.T) {
sandbox := &Sandbox{ sandbox := &Sandbox{
ctx: context.Background(), ctx: context.Background(),
id: testSandboxID, id: testSandboxID,
devManager: manager.NewDeviceManager(manager.VirtioSCSI, false, "", nil), devManager: manager.NewDeviceManager(config.VirtioSCSI, false, "", nil),
hypervisor: &mockHypervisor{}, hypervisor: &mockHypervisor{},
agent: &mockAgent{}, agent: &mockAgent{},
config: &SandboxConfig{ config: &SandboxConfig{

View File

@ -51,7 +51,7 @@ const (
// VirtioBlock means use virtio-blk for hotplugging drives // VirtioBlock means use virtio-blk for hotplugging drives
VirtioBlock = "virtio-blk" VirtioBlock = "virtio-blk"
// VirtioBlockCCW means use virtio-blk for hotplugging drives // VirtioBlockCCW means use virtio-blk-ccw for hotplugging drives
VirtioBlockCCW = "virtio-blk-ccw" VirtioBlockCCW = "virtio-blk-ccw"
// VirtioSCSI means use virtio-scsi for hotplugging drives // VirtioSCSI means use virtio-scsi for hotplugging drives
@ -72,6 +72,12 @@ const (
VirtioFSNydus = "virtio-fs-nydus" VirtioFSNydus = "virtio-fs-nydus"
) )
const (
// Define the string key for DriverOptions in DeviceInfo struct
FsTypeOpt = "fstype"
BlockDriverOpt = "block-driver"
)
const ( const (
// The OCI spec requires the major-minor number to be provided for a // The OCI spec requires the major-minor number to be provided for a
// device. We have chosen the below major numbers to represent // device. We have chosen the below major numbers to represent
@ -97,7 +103,7 @@ var getSysDevPath = getSysDevPathImpl
// DeviceInfo is an embedded type that contains device data common to all types of devices. // DeviceInfo is an embedded type that contains device data common to all types of devices.
type DeviceInfo struct { type DeviceInfo struct {
// DriverOptions is specific options for each device driver // DriverOptions is specific options for each device driver
// for example, for BlockDevice, we can set DriverOptions["blockDriver"]="virtio-blk" // for example, for BlockDevice, we can set DriverOptions["block-driver"]="virtio-blk"
DriverOptions map[string]string DriverOptions map[string]string
// Hostpath is device path on host // Hostpath is device path on host

View File

@ -81,8 +81,8 @@ func PmemDeviceInfo(source, destination string) (*DeviceInfo, error) {
fstype = "ext4" fstype = "ext4"
} }
pmemLog.WithField("fstype", fstype).Debug("filesystem for mount point") pmemLog.WithField(FsTypeOpt, fstype).Debug("filesystem for mount point")
device.DriverOptions["fstype"] = fstype device.DriverOptions[FsTypeOpt] = fstype
return device, nil return device, nil
} }

View File

@ -70,13 +70,13 @@ func (device *BlockDevice) Attach(ctx context.Context, devReceiver api.DeviceRec
ReadOnly: device.DeviceInfo.ReadOnly, ReadOnly: device.DeviceInfo.ReadOnly,
} }
if fs, ok := device.DeviceInfo.DriverOptions["fstype"]; ok { if fs, ok := device.DeviceInfo.DriverOptions[config.FsTypeOpt]; ok {
drive.Format = fs drive.Format = fs
} }
customOptions := device.DeviceInfo.DriverOptions customOptions := device.DeviceInfo.DriverOptions
if customOptions == nil || if customOptions == nil ||
customOptions["block-driver"] == "virtio-scsi" { customOptions[config.BlockDriverOpt] == config.VirtioSCSI {
// User has not chosen a specific block device type // User has not chosen a specific block device type
// Default to SCSI // Default to SCSI
scsiAddr, err := utils.GetSCSIAddress(index) scsiAddr, err := utils.GetSCSIAddress(index)
@ -85,15 +85,15 @@ func (device *BlockDevice) Attach(ctx context.Context, devReceiver api.DeviceRec
} }
drive.SCSIAddr = scsiAddr drive.SCSIAddr = scsiAddr
} else if customOptions["block-driver"] != "nvdimm" { } else if customOptions[config.BlockDriverOpt] != config.Nvdimm {
var globalIdx int var globalIdx int
switch customOptions["block-driver"] { switch customOptions[config.BlockDriverOpt] {
case "virtio-blk": case config.VirtioBlock:
globalIdx = index globalIdx = index
case "virtio-blk-ccw": case config.VirtioBlockCCW:
globalIdx = index globalIdx = index
case "virtio-mmio": case config.VirtioMmio:
//With firecracker the rootfs for the VM itself //With firecracker the rootfs for the VM itself
//sits at /dev/vda and consumes the first index. //sits at /dev/vda and consumes the first index.
//Longer term block based VM rootfs should be added //Longer term block based VM rootfs should be added
@ -111,7 +111,7 @@ func (device *BlockDevice) Attach(ctx context.Context, devReceiver api.DeviceRec
drive.VirtPath = filepath.Join("/dev", driveName) drive.VirtPath = filepath.Join("/dev", driveName)
} }
deviceLogger().WithField("device", device.DeviceInfo.HostPath).WithField("VirtPath", drive.VirtPath).Infof("Attaching %s device", customOptions["block-driver"]) deviceLogger().WithField("device", device.DeviceInfo.HostPath).WithField("VirtPath", drive.VirtPath).Infof("Attaching %s device", customOptions[config.BlockDriverOpt])
device.BlockDrive = drive device.BlockDrive = drive
if err = devReceiver.HotplugAddDevice(ctx, device, config.DeviceBlock); err != nil { if err = devReceiver.HotplugAddDevice(ctx, device, config.DeviceBlock); err != nil {
return err return err

View File

@ -100,14 +100,14 @@ func isVirtioBlkBlockDriver(customOptions map[string]string) bool {
if customOptions == nil { if customOptions == nil {
// User has not chosen a specific block device type // User has not chosen a specific block device type
// Default to SCSI // Default to SCSI
blockDriverOption = "virtio-scsi" blockDriverOption = config.VirtioSCSI
} else { } else {
blockDriverOption = customOptions["block-driver"] blockDriverOption = customOptions[config.BlockDriverOpt]
} }
if blockDriverOption == "virtio-blk" || if blockDriverOption == config.VirtioBlock ||
blockDriverOption == "virtio-blk-ccw" || blockDriverOption == config.VirtioBlockCCW ||
blockDriverOption == "virtio-mmio" { blockDriverOption == config.VirtioMmio {
return true return true
} }

View File

@ -21,19 +21,6 @@ import (
"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils"
) )
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"
// VirtioBlockCCW indicates block driver is virtio-blk-ccw based
VirtioBlockCCW string = "virtio-blk-ccw"
// VirtioSCSI indicates block driver is virtio-scsi based
VirtioSCSI string = "virtio-scsi"
// Nvdimm indicates block driver is nvdimm based
Nvdimm string = "nvdimm"
)
var ( var (
// ErrIDExhausted represents that devices are too many // ErrIDExhausted represents that devices are too many
// and no more IDs can be generated // and no more IDs can be generated
@ -69,16 +56,16 @@ func NewDeviceManager(blockDriver string, vhostUserStoreEnabled bool, vhostUserS
vhostUserStorePath: vhostUserStorePath, vhostUserStorePath: vhostUserStorePath,
devices: make(map[string]api.Device), devices: make(map[string]api.Device),
} }
if blockDriver == VirtioMmio { if blockDriver == config.VirtioMmio {
dm.blockDriver = VirtioMmio dm.blockDriver = config.VirtioMmio
} else if blockDriver == VirtioBlock { } else if blockDriver == config.VirtioBlock {
dm.blockDriver = VirtioBlock dm.blockDriver = config.VirtioBlock
} else if blockDriver == Nvdimm { } else if blockDriver == config.Nvdimm {
dm.blockDriver = Nvdimm dm.blockDriver = config.Nvdimm
} else if blockDriver == VirtioBlockCCW { } else if blockDriver == config.VirtioBlockCCW {
dm.blockDriver = VirtioBlockCCW dm.blockDriver = config.VirtioBlockCCW
} else { } else {
dm.blockDriver = VirtioSCSI dm.blockDriver = config.VirtioSCSI
} }
drivers.AllPCIeDevs = make(map[string]bool) drivers.AllPCIeDevs = make(map[string]bool)
@ -132,13 +119,13 @@ func (dm *deviceManager) createDevice(devInfo config.DeviceInfo) (dev api.Device
if devInfo.DriverOptions == nil { if devInfo.DriverOptions == nil {
devInfo.DriverOptions = make(map[string]string) devInfo.DriverOptions = make(map[string]string)
} }
devInfo.DriverOptions["block-driver"] = dm.blockDriver devInfo.DriverOptions[config.BlockDriverOpt] = dm.blockDriver
return drivers.NewVhostUserBlkDevice(&devInfo), nil return drivers.NewVhostUserBlkDevice(&devInfo), nil
} else if isBlock(devInfo) { } else if isBlock(devInfo) {
if devInfo.DriverOptions == nil { if devInfo.DriverOptions == nil {
devInfo.DriverOptions = make(map[string]string) devInfo.DriverOptions = make(map[string]string)
} }
devInfo.DriverOptions["block-driver"] = dm.blockDriver devInfo.DriverOptions[config.BlockDriverOpt] = dm.blockDriver
return drivers.NewBlockDevice(&devInfo), nil return drivers.NewBlockDevice(&devInfo), nil
} else { } else {
deviceLogger().WithField("device", devInfo.HostPath).Info("Device has not been passed to the container") deviceLogger().WithField("device", devInfo.HostPath).Info("Device has not been passed to the container")

View File

@ -31,7 +31,7 @@ func TestAttachVhostUserBlkDevice(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "") tmpDir, err := os.MkdirTemp("", "")
dm := &deviceManager{ dm := &deviceManager{
blockDriver: VirtioBlock, blockDriver: config.VirtioBlock,
devices: make(map[string]api.Device), devices: make(map[string]api.Device),
vhostUserStoreEnabled: true, vhostUserStoreEnabled: true,
vhostUserStorePath: tmpDir, vhostUserStorePath: tmpDir,

View File

@ -26,7 +26,7 @@ const dirMode = os.FileMode(0750) | os.ModeDir
func TestNewDevice(t *testing.T) { func TestNewDevice(t *testing.T) {
dm := &deviceManager{ dm := &deviceManager{
blockDriver: VirtioBlock, blockDriver: config.VirtioBlock,
devices: make(map[string]api.Device), devices: make(map[string]api.Device),
} }
savedSysDevPrefix := config.SysDevPrefix savedSysDevPrefix := config.SysDevPrefix
@ -96,7 +96,7 @@ func TestNewDevice(t *testing.T) {
func TestAttachVFIODevice(t *testing.T) { func TestAttachVFIODevice(t *testing.T) {
dm := &deviceManager{ dm := &deviceManager{
blockDriver: VirtioBlock, blockDriver: config.VirtioBlock,
devices: make(map[string]api.Device), devices: make(map[string]api.Device),
} }
tmpDir, err := os.MkdirTemp("", "") tmpDir, err := os.MkdirTemp("", "")
@ -155,7 +155,7 @@ func TestAttachVFIODevice(t *testing.T) {
func TestAttachGenericDevice(t *testing.T) { func TestAttachGenericDevice(t *testing.T) {
dm := &deviceManager{ dm := &deviceManager{
blockDriver: VirtioBlock, blockDriver: config.VirtioBlock,
devices: make(map[string]api.Device), devices: make(map[string]api.Device),
} }
path := "/dev/tty2" path := "/dev/tty2"
@ -180,7 +180,7 @@ func TestAttachGenericDevice(t *testing.T) {
func TestAttachBlockDevice(t *testing.T) { func TestAttachBlockDevice(t *testing.T) {
dm := &deviceManager{ dm := &deviceManager{
blockDriver: VirtioBlock, blockDriver: config.VirtioBlock,
devices: make(map[string]api.Device), devices: make(map[string]api.Device),
} }
path := "/dev/hda" path := "/dev/hda"
@ -203,7 +203,7 @@ func TestAttachBlockDevice(t *testing.T) {
assert.Nil(t, err) assert.Nil(t, err)
// test virtio SCSI driver // test virtio SCSI driver
dm.blockDriver = VirtioSCSI dm.blockDriver = config.VirtioSCSI
device, err = dm.NewDevice(deviceInfo) device, err = dm.NewDevice(deviceInfo)
assert.Nil(t, err) assert.Nil(t, err)
err = device.Attach(context.Background(), devReceiver) err = device.Attach(context.Background(), devReceiver)
@ -214,7 +214,7 @@ func TestAttachBlockDevice(t *testing.T) {
} }
func TestAttachDetachDevice(t *testing.T) { func TestAttachDetachDevice(t *testing.T) {
dm := NewDeviceManager(VirtioSCSI, false, "", nil) dm := NewDeviceManager(config.VirtioSCSI, false, "", nil)
path := "/dev/hda" path := "/dev/hda"
deviceInfo := config.DeviceInfo{ deviceInfo := config.DeviceInfo{

View File

@ -547,7 +547,7 @@ type DeviceInfo struct {
ID string ID string
// DriverOptions is specific options for each device driver // DriverOptions is specific options for each device driver
// for example, for BlockDevice, we can set DriverOptions["blockDriver"]="virtio-blk" // for example, for BlockDevice, we can set DriverOptions["block-driver"]="virtio-blk"
DriverOptions map[string]string DriverOptions map[string]string
} }
``` ```
@ -835,7 +835,7 @@ type DeviceInfo struct {
ID string ID string
// DriverOptions is specific options for each device driver // DriverOptions is specific options for each device driver
// for example, for BlockDevice, we can set DriverOptions["blockDriver"]="virtio-blk" // for example, for BlockDevice, we can set DriverOptions["block-driver"]="virtio-blk"
DriverOptions map[string]string DriverOptions map[string]string
} }
``` ```

View File

@ -390,10 +390,10 @@ func TestHandleBlockVolume(t *testing.T) {
mounts = append(mounts, vMount, bMount, dMount) mounts = append(mounts, vMount, bMount, dMount)
tmpDir := "/vhost/user/dir" tmpDir := "/vhost/user/dir"
dm := manager.NewDeviceManager(manager.VirtioBlock, true, tmpDir, devices) dm := manager.NewDeviceManager(config.VirtioBlock, true, tmpDir, devices)
sConfig := SandboxConfig{} sConfig := SandboxConfig{}
sConfig.HypervisorConfig.BlockDeviceDriver = manager.VirtioBlock sConfig.HypervisorConfig.BlockDeviceDriver = config.VirtioBlock
sandbox := Sandbox{ sandbox := Sandbox{
id: "100", id: "100",
containers: containers, containers: containers,

View File

@ -86,7 +86,7 @@ type VhostUserDeviceAttrs struct {
// Refs: virtcontainers/device/drivers/generic.go:GenericDevice // Refs: virtcontainers/device/drivers/generic.go:GenericDevice
type DeviceState struct { type DeviceState struct {
// DriverOptions is specific options for each device driver // DriverOptions is specific options for each device driver
// for example, for BlockDevice, we can set DriverOptions["blockDriver"]="virtio-blk" // for example, for BlockDevice, we can set DriverOptions["block-driver"]="virtio-blk"
DriverOptions map[string]string DriverOptions map[string]string
// VhostUserDeviceAttrs is specific for vhost-user device driver // VhostUserDeviceAttrs is specific for vhost-user device driver

View File

@ -10,11 +10,11 @@ import (
"os" "os"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config"
"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/manager" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/manager"
"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist"
"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types"
"github.com/stretchr/testify/assert"
) )
func TestSandboxRestore(t *testing.T) { func TestSandboxRestore(t *testing.T) {
@ -32,7 +32,7 @@ func TestSandboxRestore(t *testing.T) {
sandbox := Sandbox{ sandbox := Sandbox{
id: "test-exp", id: "test-exp",
containers: container, containers: container,
devManager: manager.NewDeviceManager(manager.VirtioSCSI, false, "", nil), devManager: manager.NewDeviceManager(config.VirtioSCSI, false, "", nil),
hypervisor: &mockHypervisor{}, hypervisor: &mockHypervisor{},
network: network, network: network,
ctx: context.Background(), ctx: context.Background(),

View File

@ -548,7 +548,7 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) {
config.SysIOMMUPath = savedIOMMUPath config.SysIOMMUPath = savedIOMMUPath
}() }()
dm := manager.NewDeviceManager(manager.VirtioSCSI, false, "", nil) dm := manager.NewDeviceManager(config.VirtioSCSI, false, "", nil)
path := filepath.Join(vfioPath, testFDIOGroup) path := filepath.Join(vfioPath, testFDIOGroup)
deviceInfo := config.DeviceInfo{ deviceInfo := config.DeviceInfo{
HostPath: path, HostPath: path,
@ -599,7 +599,7 @@ func TestSandboxAttachDevicesVhostUserBlk(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "") tmpDir, err := os.MkdirTemp("", "")
assert.Nil(t, err) assert.Nil(t, err)
os.RemoveAll(tmpDir) os.RemoveAll(tmpDir)
dm := manager.NewDeviceManager(manager.VirtioSCSI, true, tmpDir, nil) dm := manager.NewDeviceManager(config.VirtioSCSI, true, tmpDir, nil)
vhostUserDevNodePath := filepath.Join(tmpDir, "/block/devices/") vhostUserDevNodePath := filepath.Join(tmpDir, "/block/devices/")
vhostUserSockPath := filepath.Join(tmpDir, "/block/sockets/") vhostUserSockPath := filepath.Join(tmpDir, "/block/sockets/")