device: use const strings for block-driver option instead of hard coding

Currently, the block driver option is specifed by hard coding, maybe it
is better to use const string variables instead of hard coded strings.
Another modification is to remove duplicate consts for virtio driver in
manager.go.

Fixes: #3321

Signed-off-by: Jason Zhang <zhanghj.lc@inspur.com>
This commit is contained in:
zhanghj 2022-03-14 09:17:04 +08:00 committed by Jason Zhang
parent 358081c4ae
commit efa19c41eb
13 changed files with 53 additions and 60 deletions

View File

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

View File

@ -51,7 +51,7 @@ const (
// VirtioBlock means use virtio-blk for hotplugging drives
VirtioBlock = "virtio-blk"
// VirtioBlockCCW means use virtio-blk for hotplugging drives
// VirtioBlockCCW means use virtio-blk-ccw for hotplugging drives
VirtioBlockCCW = "virtio-blk-ccw"
// VirtioSCSI means use virtio-scsi for hotplugging drives
@ -72,6 +72,12 @@ const (
VirtioFSNydus = "virtio-fs-nydus"
)
const (
// Define the string key for DriverOptions in DeviceInfo struct
FsTypeOpt = "fstype"
BlockDriverOpt = "block-driver"
)
const (
// The OCI spec requires the major-minor number to be provided for a
// 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.
type DeviceInfo struct {
// 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
// Hostpath is device path on host

View File

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

View File

@ -70,13 +70,13 @@ func (device *BlockDevice) Attach(ctx context.Context, devReceiver api.DeviceRec
ReadOnly: device.DeviceInfo.ReadOnly,
}
if fs, ok := device.DeviceInfo.DriverOptions["fstype"]; ok {
if fs, ok := device.DeviceInfo.DriverOptions[config.FsTypeOpt]; ok {
drive.Format = fs
}
customOptions := device.DeviceInfo.DriverOptions
if customOptions == nil ||
customOptions["block-driver"] == "virtio-scsi" {
customOptions[config.BlockDriverOpt] == config.VirtioSCSI {
// User has not chosen a specific block device type
// Default to SCSI
scsiAddr, err := utils.GetSCSIAddress(index)
@ -85,15 +85,15 @@ func (device *BlockDevice) Attach(ctx context.Context, devReceiver api.DeviceRec
}
drive.SCSIAddr = scsiAddr
} else if customOptions["block-driver"] != "nvdimm" {
} else if customOptions[config.BlockDriverOpt] != config.Nvdimm {
var globalIdx int
switch customOptions["block-driver"] {
case "virtio-blk":
switch customOptions[config.BlockDriverOpt] {
case config.VirtioBlock:
globalIdx = index
case "virtio-blk-ccw":
case config.VirtioBlockCCW:
globalIdx = index
case "virtio-mmio":
case config.VirtioMmio:
//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
@ -111,7 +111,7 @@ func (device *BlockDevice) Attach(ctx context.Context, devReceiver api.DeviceRec
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
if err = devReceiver.HotplugAddDevice(ctx, device, config.DeviceBlock); err != nil {
return err

View File

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

View File

@ -21,19 +21,6 @@ import (
"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 (
// ErrIDExhausted represents that devices are too many
// and no more IDs can be generated
@ -69,16 +56,16 @@ func NewDeviceManager(blockDriver string, vhostUserStoreEnabled bool, vhostUserS
vhostUserStorePath: vhostUserStorePath,
devices: make(map[string]api.Device),
}
if blockDriver == VirtioMmio {
dm.blockDriver = VirtioMmio
} else if blockDriver == VirtioBlock {
dm.blockDriver = VirtioBlock
} else if blockDriver == Nvdimm {
dm.blockDriver = Nvdimm
} else if blockDriver == VirtioBlockCCW {
dm.blockDriver = VirtioBlockCCW
if blockDriver == config.VirtioMmio {
dm.blockDriver = config.VirtioMmio
} else if blockDriver == config.VirtioBlock {
dm.blockDriver = config.VirtioBlock
} else if blockDriver == config.Nvdimm {
dm.blockDriver = config.Nvdimm
} else if blockDriver == config.VirtioBlockCCW {
dm.blockDriver = config.VirtioBlockCCW
} else {
dm.blockDriver = VirtioSCSI
dm.blockDriver = config.VirtioSCSI
}
drivers.AllPCIeDevs = make(map[string]bool)
@ -132,13 +119,13 @@ func (dm *deviceManager) createDevice(devInfo config.DeviceInfo) (dev api.Device
if devInfo.DriverOptions == nil {
devInfo.DriverOptions = make(map[string]string)
}
devInfo.DriverOptions["block-driver"] = dm.blockDriver
devInfo.DriverOptions[config.BlockDriverOpt] = dm.blockDriver
return drivers.NewVhostUserBlkDevice(&devInfo), nil
} else if isBlock(devInfo) {
if devInfo.DriverOptions == nil {
devInfo.DriverOptions = make(map[string]string)
}
devInfo.DriverOptions["block-driver"] = dm.blockDriver
devInfo.DriverOptions[config.BlockDriverOpt] = dm.blockDriver
return drivers.NewBlockDevice(&devInfo), nil
} else {
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("", "")
dm := &deviceManager{
blockDriver: VirtioBlock,
blockDriver: config.VirtioBlock,
devices: make(map[string]api.Device),
vhostUserStoreEnabled: true,
vhostUserStorePath: tmpDir,

View File

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

View File

@ -547,7 +547,7 @@ type DeviceInfo struct {
ID string
// 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
}
```
@ -835,7 +835,7 @@ type DeviceInfo struct {
ID string
// 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
}
```

View File

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

View File

@ -86,7 +86,7 @@ type VhostUserDeviceAttrs struct {
// Refs: virtcontainers/device/drivers/generic.go:GenericDevice
type DeviceState struct {
// 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
// VhostUserDeviceAttrs is specific for vhost-user device driver

View File

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

View File

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