virtcontainers: address some comments

* Move makeNameID() func to virtcontainers/utils file as it's a generic
function for making name and ID.
* Move bindDevicetoVFIO() and bindDevicetoHost() to vfio driver package.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

virtcontainers/container.go

@@ -830,7 +830,7 @@ func (c *Container) hotplugDrive() error {
 	}
 
 	// Add drive with id as container id
-	devID := makeNameID("drive", c.id)
+	devID := utils.MakeNameID("drive", c.id, maxDevIDSize)
 	drive := drivers.Drive{
 		File:   devicePath,
 		Format: "raw",
@@ -867,7 +867,7 @@ func (c *Container) removeDrive() (err error) {
 	if c.isDriveUsed() && c.state.HotpluggedDrive {
 		c.Logger().Info("unplugging block device")
 
-		devID := makeNameID("drive", c.id)
+		devID := utils.MakeNameID("drive", c.id, maxDevIDSize)
 		drive := &drivers.Drive{
 			ID: devID,
 		}

virtcontainers/device/api/interface.go

@@ -14,7 +14,7 @@ import (
 
 var devLogger = logrus.FieldLogger(logrus.New())
 
-// SetLogger sets the logger for virtcontainers package.
+// SetLogger sets the logger for device api package.
 func SetLogger(logger logrus.FieldLogger) {
 	devLogger = logger
 }
@@ -30,7 +30,7 @@ type DeviceReceiver interface {
 	HotplugAddDevice(Device, config.DeviceType) error
 	HotplugRemoveDevice(Device, config.DeviceType) error
 
-	// this is only for virtio-blk support
+	// this is only for virtio-blk and virtio-scsi support
 	GetAndSetSandboxBlockIndex() (int, error)
 	DecrementSandboxBlockIndex() error
 

virtcontainers/device/drivers/block.go

@@ -15,6 +15,8 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/utils"
 )
 
+const maxDevIDSize = 31
+
 // Drive represents a block storage drive which may be used in case the storage
 // driver has an underlying block storage device.
 type Drive struct {
@@ -89,7 +91,7 @@ func (device *BlockDevice) Attach(devReceiver api.DeviceReceiver) (err error) {
 	drive := Drive{
 		File:   device.DeviceInfo.HostPath,
 		Format: "raw",
-		ID:     makeNameID("drive", device.DeviceInfo.ID),
+		ID:     utils.MakeNameID("drive", device.DeviceInfo.ID, maxDevIDSize),
 		Index:  index,
 	}
 

virtcontainers/device/drivers/utils.go

@@ -7,8 +7,6 @@
 package drivers
 
 import (
-	"fmt"
-
 	"github.com/sirupsen/logrus"
 
 	"github.com/kata-containers/runtime/virtcontainers/device/api"
@@ -17,17 +15,3 @@ import (
 func deviceLogger() *logrus.Entry {
 	return api.DeviceLogger()
 }
-
-// FIXME: this is duplicate code from virtcontainers/hypervisor.go
-const maxDevIDSize = 31
-
-// FIXME: this is duplicate code from virtcontainers/hypervisor.go
-//Generic function for creating a named-id for passing on the hypervisor commandline
-func makeNameID(namedType string, id string) string {
-	nameID := fmt.Sprintf("%s-%s", namedType, id)
-	if len(nameID) > maxDevIDSize {
-		nameID = nameID[:maxDevIDSize]
-	}
-
-	return nameID
-}

virtcontainers/device/drivers/vfio.go

@@ -20,6 +20,14 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/utils"
 )
 
+// bind/unbind paths to aid in SRIOV VF bring-up/restore
+var (
+	pciDriverUnbindPath = "/sys/bus/pci/devices/%s/driver/unbind"
+	pciDriverBindPath   = "/sys/bus/pci/drivers/%s/bind"
+	vfioNewIDPath       = "/sys/bus/pci/drivers/vfio-pci/new_id"
+	vfioRemoveIDPath    = "/sys/bus/pci/drivers/vfio-pci/remove_id"
+)
+
 // VFIODevice is a vfio device meant to be passed to the hypervisor
 // to be used by the Virtual Machine.
 type VFIODevice struct {
@@ -101,3 +109,70 @@ func getBDF(deviceSysStr string) (string, error) {
 	tokens = strings.SplitN(deviceSysStr, ":", 2)
 	return tokens[1], nil
 }
+
+// BindDevicetoVFIO binds the device to vfio driver after unbinding from host.
+// Will be called by a network interface or a generic pcie device.
+func BindDevicetoVFIO(bdf, hostDriver, vendorDeviceID string) error {
+
+	// Unbind from the host driver
+	unbindDriverPath := fmt.Sprintf(pciDriverUnbindPath, bdf)
+	deviceLogger().WithFields(logrus.Fields{
+		"device-bdf":  bdf,
+		"driver-path": unbindDriverPath,
+	}).Info("Unbinding device from driver")
+
+	if err := utils.WriteToFile(unbindDriverPath, []byte(bdf)); err != nil {
+		return err
+	}
+
+	// Add device id to vfio driver.
+	deviceLogger().WithFields(logrus.Fields{
+		"vendor-device-id": vendorDeviceID,
+		"vfio-new-id-path": vfioNewIDPath,
+	}).Info("Writing vendor-device-id to vfio new-id path")
+
+	if err := utils.WriteToFile(vfioNewIDPath, []byte(vendorDeviceID)); err != nil {
+		return err
+	}
+
+	// Bind to vfio-pci driver.
+	bindDriverPath := fmt.Sprintf(pciDriverBindPath, "vfio-pci")
+
+	api.DeviceLogger().WithFields(logrus.Fields{
+		"device-bdf":  bdf,
+		"driver-path": bindDriverPath,
+	}).Info("Binding device to vfio driver")
+
+	// Device may be already bound at this time because of earlier write to new_id, ignore error
+	utils.WriteToFile(bindDriverPath, []byte(bdf))
+
+	return nil
+}
+
+// BindDevicetoHost binds the device to the host driver driver after unbinding from vfio-pci.
+func BindDevicetoHost(bdf, hostDriver, vendorDeviceID string) error {
+	// Unbind from vfio-pci driver
+	unbindDriverPath := fmt.Sprintf(pciDriverUnbindPath, bdf)
+	api.DeviceLogger().WithFields(logrus.Fields{
+		"device-bdf":  bdf,
+		"driver-path": unbindDriverPath,
+	}).Info("Unbinding device from driver")
+
+	if err := utils.WriteToFile(unbindDriverPath, []byte(bdf)); err != nil {
+		return err
+	}
+
+	// To prevent new VFs from binding to VFIO-PCI, remove_id
+	if err := utils.WriteToFile(vfioRemoveIDPath, []byte(vendorDeviceID)); err != nil {
+		return err
+	}
+
+	// Bind back to host driver
+	bindDriverPath := fmt.Sprintf(pciDriverBindPath, hostDriver)
+	api.DeviceLogger().WithFields(logrus.Fields{
+		"device-bdf":  bdf,
+		"driver-path": bindDriverPath,
+	}).Info("Binding back device to host driver")
+
+	return utils.WriteToFile(bindDriverPath, []byte(bdf))
+}

virtcontainers/hypervisor.go

@@ -116,16 +116,6 @@ func newHypervisor(hType HypervisorType) (hypervisor, error) {
 	}
 }
 
-//Generic function for creating a named-id for passing on the hypervisor commandline
-func makeNameID(namedType string, id string) string {
-	nameID := fmt.Sprintf("%s-%s", namedType, id)
-	if len(nameID) > maxDevIDSize {
-		nameID = nameID[:maxDevIDSize]
-	}
-
-	return nameID
-}
-
 // Param is a key/value representation for hypervisor and kernel parameters.
 type Param struct {
 	Key   string

virtcontainers/network.go

@@ -25,7 +25,6 @@ import (
 	"github.com/vishvananda/netns"
 	"golang.org/x/sys/unix"
 
-	"github.com/kata-containers/runtime/virtcontainers/device/api"
 	"github.com/kata-containers/runtime/virtcontainers/device/drivers"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/uuid"
 	"github.com/kata-containers/runtime/virtcontainers/utils"
@@ -1350,85 +1349,12 @@ func createPhysicalEndpoint(netInfo NetworkInfo) (*PhysicalEndpoint, error) {
 	return physicalEndpoint, nil
 }
 
-// bind/unbind paths to aid in SRIOV VF bring-up/restore
-var pciDriverUnbindPath = "/sys/bus/pci/devices/%s/driver/unbind"
-var pciDriverBindPath = "/sys/bus/pci/drivers/%s/bind"
-var vfioNewIDPath = "/sys/bus/pci/drivers/vfio-pci/new_id"
-var vfioRemoveIDPath = "/sys/bus/pci/drivers/vfio-pci/remove_id"
-
-// bindDevicetoVFIO binds the device to vfio driver after unbinding from host.
-// Will be called by a network interface or a generic pcie device.
-func bindDevicetoVFIO(bdf, hostDriver, vendorDeviceID string) error {
-
-	// Unbind from the host driver
-	unbindDriverPath := fmt.Sprintf(pciDriverUnbindPath, bdf)
-	api.DeviceLogger().WithFields(logrus.Fields{
-		"device-bdf":  bdf,
-		"driver-path": unbindDriverPath,
-	}).Info("Unbinding device from driver")
-
-	if err := utils.WriteToFile(unbindDriverPath, []byte(bdf)); err != nil {
-		return err
-	}
-
-	// Add device id to vfio driver.
-	api.DeviceLogger().WithFields(logrus.Fields{
-		"vendor-device-id": vendorDeviceID,
-		"vfio-new-id-path": vfioNewIDPath,
-	}).Info("Writing vendor-device-id to vfio new-id path")
-
-	if err := utils.WriteToFile(vfioNewIDPath, []byte(vendorDeviceID)); err != nil {
-		return err
-	}
-
-	// Bind to vfio-pci driver.
-	bindDriverPath := fmt.Sprintf(pciDriverBindPath, "vfio-pci")
-
-	api.DeviceLogger().WithFields(logrus.Fields{
-		"device-bdf":  bdf,
-		"driver-path": bindDriverPath,
-	}).Info("Binding device to vfio driver")
-
-	// Device may be already bound at this time because of earlier write to new_id, ignore error
-	utils.WriteToFile(bindDriverPath, []byte(bdf))
-
-	return nil
-}
-
-// bindDevicetoHost binds the device to the host driver driver after unbinding from vfio-pci.
-func bindDevicetoHost(bdf, hostDriver, vendorDeviceID string) error {
-	// Unbind from vfio-pci driver
-	unbindDriverPath := fmt.Sprintf(pciDriverUnbindPath, bdf)
-	api.DeviceLogger().WithFields(logrus.Fields{
-		"device-bdf":  bdf,
-		"driver-path": unbindDriverPath,
-	}).Info("Unbinding device from driver")
-
-	if err := utils.WriteToFile(unbindDriverPath, []byte(bdf)); err != nil {
-		return err
-	}
-
-	// To prevent new VFs from binding to VFIO-PCI, remove_id
-	if err := utils.WriteToFile(vfioRemoveIDPath, []byte(vendorDeviceID)); err != nil {
-		return err
-	}
-
-	// Bind back to host driver
-	bindDriverPath := fmt.Sprintf(pciDriverBindPath, hostDriver)
-	api.DeviceLogger().WithFields(logrus.Fields{
-		"device-bdf":  bdf,
-		"driver-path": bindDriverPath,
-	}).Info("Binding back device to host driver")
-
-	return utils.WriteToFile(bindDriverPath, []byte(bdf))
-}
-
 func bindNICToVFIO(endpoint *PhysicalEndpoint) error {
-	return bindDevicetoVFIO(endpoint.BDF, endpoint.Driver, endpoint.VendorDeviceID)
+	return drivers.BindDevicetoVFIO(endpoint.BDF, endpoint.Driver, endpoint.VendorDeviceID)
 }
 
 func bindNICToHost(endpoint *PhysicalEndpoint) error {
-	return bindDevicetoHost(endpoint.BDF, endpoint.Driver, endpoint.VendorDeviceID)
+	return drivers.BindDevicetoHost(endpoint.BDF, endpoint.Driver, endpoint.VendorDeviceID)
 }
 
 // Long term, this should be made more configurable.  For now matching path

virtcontainers/qemu_arch_base.go

@@ -290,7 +290,7 @@ func (q *qemuArchBase) appendImage(devices []govmmQemu.Device, path string) ([]g
 		return nil, err
 	}
 
-	id := makeNameID("image", hex.EncodeToString(randBytes))
+	id := utils.MakeNameID("image", hex.EncodeToString(randBytes), maxDevIDSize)
 
 	drive := drivers.Drive{
 		File:   path,
@@ -466,16 +466,16 @@ func (q *qemuArchBase) appendVhostUserDevice(devices []govmmQemu.Device, vhostUs
 	// TODO: find a way to remove dependency of drivers package
 	switch vhostUserDevice := vhostUserDevice.(type) {
 	case *drivers.VhostUserNetDevice:
-		qemuVhostUserDevice.TypeDevID = makeNameID("net", vhostUserDevice.ID)
+		qemuVhostUserDevice.TypeDevID = utils.MakeNameID("net", vhostUserDevice.ID, maxDevIDSize)
 		qemuVhostUserDevice.Address = vhostUserDevice.MacAddress
 	case *drivers.VhostUserSCSIDevice:
-		qemuVhostUserDevice.TypeDevID = makeNameID("scsi", vhostUserDevice.ID)
+		qemuVhostUserDevice.TypeDevID = utils.MakeNameID("scsi", vhostUserDevice.ID, maxDevIDSize)
 	case *drivers.VhostUserBlkDevice:
 	}
 
 	qemuVhostUserDevice.VhostUserType = govmmQemu.VhostUserDeviceType(vhostUserDevice.Type())
 	qemuVhostUserDevice.SocketPath = vhostUserDevice.Attrs().SocketPath
-	qemuVhostUserDevice.CharDevID = makeNameID("char", vhostUserDevice.Attrs().ID)
+	qemuVhostUserDevice.CharDevID = utils.MakeNameID("char", vhostUserDevice.Attrs().ID, maxDevIDSize)
 
 	devices = append(devices, qemuVhostUserDevice)
 

virtcontainers/sandbox.go

@@ -1322,10 +1322,17 @@ func togglePauseSandbox(sandboxID string, pause bool) (*Sandbox, error) {
 func (s *Sandbox) HotplugAddDevice(device api.Device, devType config.DeviceType) error {
 	switch devType {
 	case config.DeviceVFIO:
-		return s.hypervisor.hotplugAddDevice(device, vfioDev)
+		vfioDevice, ok := device.(*drivers.VFIODevice)
+		if !ok {
+			return fmt.Errorf("device type mismatch, expect device type to be %s", devType)
+		}
+		return s.hypervisor.hotplugAddDevice(*vfioDevice, vfioDev)
 	case config.DeviceBlock:
-		device := device.(*drivers.BlockDevice)
+		blockDevice, ok := device.(*drivers.BlockDevice)
-		return s.hypervisor.hotplugAddDevice(device.BlockDrive, blockDev)
+		if !ok {
+			return fmt.Errorf("device type mismatch, expect device type to be %s", devType)
+		}
+		return s.hypervisor.hotplugAddDevice(blockDevice.BlockDrive, blockDev)
 	case config.DeviceGeneric:
 		// TODO: what?
 		return nil
@@ -1338,10 +1345,17 @@ func (s *Sandbox) HotplugAddDevice(device api.Device, devType config.DeviceType)
 func (s *Sandbox) HotplugRemoveDevice(device api.Device, devType config.DeviceType) error {
 	switch devType {
 	case config.DeviceVFIO:
-		return s.hypervisor.hotplugRemoveDevice(device, vfioDev)
+		vfioDevice, ok := device.(*drivers.VFIODevice)
+		if !ok {
+			return fmt.Errorf("device type mismatch, expect device type to be %s", devType)
+		}
+		return s.hypervisor.hotplugRemoveDevice(*vfioDevice, vfioDev)
 	case config.DeviceBlock:
-		device := device.(*drivers.BlockDevice)
+		blockDevice, ok := device.(*drivers.BlockDevice)
-		return s.hypervisor.hotplugRemoveDevice(device.BlockDrive, blockDev)
+		if !ok {
+			return fmt.Errorf("device type mismatch, expect device type to be %s", devType)
+		}
+		return s.hypervisor.hotplugRemoveDevice(blockDevice.BlockDrive, blockDev)
 	case config.DeviceGeneric:
 		// TODO: what?
 		return nil

virtcontainers/utils/utils.go

@@ -164,3 +164,13 @@ func GetSCSIAddress(index int) (string, error) {
 
 	return fmt.Sprintf("%d:%d", scsiID, lun), nil
 }
+
+// MakeNameID is generic function for creating a named-id for passing on the hypervisor commandline
+func MakeNameID(namedType, id string, maxLen int) string {
+	nameID := fmt.Sprintf("%s-%s", namedType, id)
+	if len(nameID) > maxLen {
+		nameID = nameID[:maxLen]
+	}
+
+	return nameID
+}