Add support for scsi for hotplugging block devices

api_test.go

@@ -876,12 +876,13 @@ func TestStatusPodSuccessfulStateReady(t *testing.T) {
 
 	config := newTestPodConfigNoop()
 	hypervisorConfig := HypervisorConfig{
-		KernelPath:     filepath.Join(testDir, testKernel),
-		ImagePath:      filepath.Join(testDir, testImage),
-		HypervisorPath: filepath.Join(testDir, testHypervisor),
-		DefaultVCPUs:   defaultVCPUs,
-		DefaultMemSz:   defaultMemSzMiB,
-		DefaultBridges: defaultBridges,
+		KernelPath:        filepath.Join(testDir, testKernel),
+		ImagePath:         filepath.Join(testDir, testImage),
+		HypervisorPath:    filepath.Join(testDir, testHypervisor),
+		DefaultVCPUs:      defaultVCPUs,
+		DefaultMemSz:      defaultMemSzMiB,
+		DefaultBridges:    defaultBridges,
+		BlockDeviceDriver: defaultBlockDriver,
 	}
 
 	expectedStatus := PodStatus{
@@ -930,12 +931,13 @@ func TestStatusPodSuccessfulStateRunning(t *testing.T) {
 
 	config := newTestPodConfigNoop()
 	hypervisorConfig := HypervisorConfig{
-		KernelPath:     filepath.Join(testDir, testKernel),
-		ImagePath:      filepath.Join(testDir, testImage),
-		HypervisorPath: filepath.Join(testDir, testHypervisor),
-		DefaultVCPUs:   defaultVCPUs,
-		DefaultMemSz:   defaultMemSzMiB,
-		DefaultBridges: defaultBridges,
+		KernelPath:        filepath.Join(testDir, testKernel),
+		ImagePath:         filepath.Join(testDir, testImage),
+		HypervisorPath:    filepath.Join(testDir, testHypervisor),
+		DefaultVCPUs:      defaultVCPUs,
+		DefaultMemSz:      defaultMemSzMiB,
+		DefaultBridges:    defaultBridges,
+		BlockDeviceDriver: defaultBlockDriver,
 	}
 
 	expectedStatus := PodStatus{

container.go

@@ -679,24 +679,25 @@ func (c *Container) hotplugDrive() error {
 		"fs-type":     fsType,
 	}).Info("Block device detected")
 
+	driveIndex, err := c.pod.getAndSetPodBlockIndex()
+	if err != nil {
+		return err
+	}
+
 	// Add drive with id as container id
 	devID := makeNameID("drive", c.id)
 	drive := Drive{
 		File:   devicePath,
 		Format: "raw",
 		ID:     devID,
+		Index:  driveIndex,
 	}
 
 	if err := c.pod.hypervisor.hotplugAddDevice(drive, blockDev); err != nil {
 		return err
 	}
 	c.setStateHotpluggedDrive(true)
 
-	driveIndex, err := c.pod.getAndSetPodBlockIndex()
-	if err != nil {
-		return err
-	}
-
 	if err := c.setStateBlockIndex(driveIndex); err != nil {
 		return err
 	}

device.go

@@ -340,7 +340,11 @@ type BlockDevice struct {
 	DeviceType string
 	DeviceInfo DeviceInfo
 
-	// Path at which the device appears inside the VM, outside of the container mount namespace.
+	// SCSI Address of the block device, in case the device is attached using SCSI driver
+	// SCSI address is in the format SCSI-Id:LUN
+	SCSIAddr string
+
+	// Path at which the device appears inside the VM, outside of the container mount namespace
 	VirtPath string
 }
 
@@ -359,12 +363,6 @@ func (device *BlockDevice) attach(h hypervisor, c *Container) (err error) {
 
 	device.DeviceInfo.ID = hex.EncodeToString(randBytes)
 
-	drive := Drive{
-		File:   device.DeviceInfo.HostPath,
-		Format: "raw",
-		ID:     makeNameID("drive", device.DeviceInfo.ID),
-	}
-
 	// Increment the block index for the pod. This is used to determine the name
 	// for the block device in the case where the block device is used as container
 	// rootfs and the predicted block device name needs to be provided to the agent.
@@ -380,6 +378,13 @@ func (device *BlockDevice) attach(h hypervisor, c *Container) (err error) {
 		return err
 	}
 
+	drive := Drive{
+		File:   device.DeviceInfo.HostPath,
+		Format: "raw",
+		ID:     makeNameID("drive", device.DeviceInfo.ID),
+		Index:  index,
+	}
+
 	driveName, err := getVirtDriveName(index)
 	if err != nil {
 		return err
@@ -393,7 +398,17 @@ func (device *BlockDevice) attach(h hypervisor, c *Container) (err error) {
 
 	device.DeviceInfo.Hotplugged = true
 
-	device.VirtPath = filepath.Join("/dev", driveName)
+	if c.pod.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock {
+		device.VirtPath = filepath.Join("/dev", driveName)
+	} else {
+		scsiAddr, err := getSCSIAddress(index)
+		if err != nil {
+			return err
+		}
+
+		device.SCSIAddr = scsiAddr
+	}
+
 	return nil
 }
 

device_test.go

@@ -322,10 +322,19 @@ func TestAttachBlockDevice(t *testing.T) {
 	fs := &filesystem{}
 	hypervisor := &mockHypervisor{}
 
+	hConfig := HypervisorConfig{
+		BlockDeviceDriver: VirtioBlock,
+	}
+
+	config := &PodConfig{
+		HypervisorConfig: hConfig,
+	}
+
 	pod := &Pod{
 		id:         testPodID,
 		storage:    fs,
 		hypervisor: hypervisor,
+		config:     config,
 	}
 
 	contID := "100"
@@ -376,4 +385,18 @@ func TestAttachBlockDevice(t *testing.T) {
 
 	err = device.detach(hypervisor)
 	assert.Nil(t, err)
+
+	container.pod.config.HypervisorConfig.BlockDeviceDriver = VirtioSCSI
+	err = device.attach(hypervisor, &container)
+	assert.Nil(t, err)
+
+	err = device.detach(hypervisor)
+	assert.Nil(t, err)
+
+	container.state.State = StateReady
+	err = device.attach(hypervisor, &container)
+	assert.Nil(t, err)
+
+	err = device.detach(hypervisor)
+	assert.Nil(t, err)
 }

hyperstart_agent.go

@@ -414,13 +414,22 @@ func (h *hyper) startOneContainer(pod Pod, c *Container) error {
 	container.SystemMountsInfo.BindMountDev = c.systemMountsInfo.BindMountDev
 
 	if c.state.Fstype != "" {
-		driveName, err := getVirtDriveName(c.state.BlockIndex)
-		if err != nil {
-			return err
+		// Pass a drive name only in case of block driver
+		if pod.config.HypervisorConfig.BlockDeviceDriver == VirtioBlock {
+			driveName, err := getVirtDriveName(c.state.BlockIndex)
+			if err != nil {
+				return err
+			}
+			container.Image = driveName
+		} else {
+			scsiAddr, err := getSCSIAddress(c.state.BlockIndex)
+			if err != nil {
+				return err
+			}
+			container.SCSIAddr = scsiAddr
 		}
 
 		container.Fstype = c.state.Fstype
-		container.Image = driveName
 	} else {
 
 		if err := bindMountContainerRootfs(defaultSharedDir, pod.id, c.id, c.rootFs, false); err != nil {
@@ -450,6 +459,7 @@ func (h *hyper) startOneContainer(pod Pod, c *Container) error {
 				Path:         d.DeviceInfo.ContainerPath,
 				AbsolutePath: true,
 				DockerVolume: false,
+				SCSIAddr:     d.SCSIAddr,
 			}
 			fsmap = append(fsmap, fsmapDesc)
 		}

hypervisor.go

@@ -47,6 +47,8 @@ const (
 	defaultMemSzMiB = 2048
 
 	defaultBridges = 1
+
+	defaultBlockDriver = VirtioSCSI
 )
 
 // deviceType describes a virtualized device type.
@@ -155,6 +157,10 @@ type HypervisorConfig struct {
 	// DisableBlockDeviceUse disallows a block device from being used.
 	DisableBlockDeviceUse bool
 
+	// BlockDeviceDriver specifies the driver to be used for block device
+	// either VirtioSCSI or VirtioBlock with the default driver being defaultBlockDriver
+	BlockDeviceDriver string
+
 	// KernelParams are additional guest kernel parameters.
 	KernelParams []Param
 
@@ -227,6 +233,10 @@ func (conf *HypervisorConfig) valid() (bool, error) {
 		conf.DefaultBridges = defaultBridges
 	}
 
+	if conf.BlockDeviceDriver == "" {
+		conf.BlockDeviceDriver = defaultBlockDriver
+	}
+
 	return true, nil
 }
 

hypervisor_test.go

@@ -174,12 +174,13 @@ func TestHypervisorConfigDefaults(t *testing.T) {
 	testHypervisorConfigValid(t, hypervisorConfig, true)
 
 	hypervisorConfigDefaultsExpected := &HypervisorConfig{
-		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath: "",
-		DefaultVCPUs:   defaultVCPUs,
-		DefaultMemSz:   defaultMemSzMiB,
-		DefaultBridges: defaultBridges,
+		KernelPath:        fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:         fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath:    "",
+		DefaultVCPUs:      defaultVCPUs,
+		DefaultMemSz:      defaultMemSzMiB,
+		DefaultBridges:    defaultBridges,
+		BlockDeviceDriver: defaultBlockDriver,
 	}
 	if reflect.DeepEqual(hypervisorConfig, hypervisorConfigDefaultsExpected) == false {
 		t.Fatal()

mount.go

@@ -321,3 +321,30 @@ func bindUnmountAllRootfs(sharedDir string, pod Pod) {
 		}
 	}
 }
+
+const maxSCSIDevices = 65535
+
+// getSCSIIdLun gets the SCSI id and lun, based on the index of the drive being inserted.
+// qemu code suggests that scsi-id can take values from 0 to 255 inclusive, while lun can
+// take values from 0 to 16383 inclusive. But lun values over 255 do not seem to follow
+// consistent SCSI addressing. Hence we limit to 255.
+func getSCSIIdLun(index int) (int, int, error) {
+	if index < 0 {
+		return -1, -1, fmt.Errorf("Index cannot be negative")
+	}
+
+	if index > maxSCSIDevices {
+		return -1, -1, fmt.Errorf("Index cannot be greater than %d, maximum of %d devices are supported", maxSCSIDevices, maxSCSIDevices)
+	}
+
+	return index / 256, index % 256, nil
+}
+
+func getSCSIAddress(index int) (string, error) {
+	scsiID, lun, err := getSCSIIdLun(index)
+	if err != nil {
+		return "", err
+	}
+
+	return fmt.Sprintf("%d:%d", scsiID, lun), nil
+}

mount_test.go

@@ -19,6 +19,7 @@ package virtcontainers
 import (
 	"bytes"
 	"fmt"
+	"github.com/stretchr/testify/assert"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -282,3 +283,52 @@ func TestGetVirtDriveName(t *testing.T) {
 		}
 	}
 }
+
+func TestGetSCSIIdLun(t *testing.T) {
+	tests := []struct {
+		index          int
+		expectedScsiID int
+		expectedLun    int
+	}{
+		{0, 0, 0},
+		{1, 0, 1},
+		{2, 0, 2},
+		{255, 0, 255},
+		{256, 1, 0},
+		{257, 1, 1},
+		{258, 1, 2},
+		{512, 2, 0},
+		{513, 2, 1},
+	}
+
+	for _, test := range tests {
+		scsiID, lun, err := getSCSIIdLun(test.index)
+		assert.Nil(t, err)
+
+		if scsiID != test.expectedScsiID && lun != test.expectedLun {
+			t.Fatalf("Expecting scsi-id:lun %d:%d,  Got %d:%d", test.expectedScsiID, test.expectedLun, scsiID, lun)
+		}
+	}
+
+	_, _, err := getSCSIIdLun(maxSCSIDevices + 1)
+	assert.NotNil(t, err)
+}
+
+func TestGetSCSIAddress(t *testing.T) {
+	tests := []struct {
+		index               int
+		expectedSCSIAddress string
+	}{
+		{0, "0:0"},
+		{200, "0:200"},
+		{255, "0:255"},
+		{258, "1:2"},
+		{512, "2:0"},
+	}
+
+	for _, test := range tests {
+		scsiAddr, err := getSCSIAddress(test.index)
+		assert.Nil(t, err)
+		assert.Equal(t, scsiAddr, test.expectedSCSIAddress)
+	}
+}

pkg/hyperstart/types.go

@@ -133,6 +133,7 @@ type FsmapDescriptor struct {
 	ReadOnly     bool   `json:"readOnly"`
 	DockerVolume bool   `json:"dockerVolume"`
 	AbsolutePath bool   `json:"absolutePath"`
+	SCSIAddr     string `json:"scsiAddr"`
 }
 
 // EnvironmentVar holds an environment variable and its value.
@@ -207,7 +208,7 @@ type Container struct {
 	Rootfs           string              `json:"rootfs"`
 	Fstype           string              `json:"fstype,omitempty"`
 	Image            string              `json:"image"`
-	Addr             string              `json:"addr,omitempty"`
+	SCSIAddr         string              `json:"scsiAddr,omitempty"`
 	Volumes          []*VolumeDescriptor `json:"volumes,omitempty"`
 	Fsmap            []*FsmapDescriptor  `json:"fsmap,omitempty"`
 	Sysctl           map[string]string   `json:"sysctl,omitempty"`

pod.go

@@ -245,6 +245,9 @@ type Drive struct {
 
 	// ID is used to identify this drive in the hypervisor options.
 	ID string
+
+	// Index assigned to the drive. In case of virtio-scsi, this is used as SCSI LUN index
+	Index int
 }
 
 // EnvVar is a key/value structure representing a command