Revert "Merge pull request #584 from amshinde/switch-to-scsi"

This reverts commit 494b46c, reversing
changes made to e475a8f.

api_test.go

@@ -876,13 +876,12 @@ 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,
-		BlockDeviceDriver: defaultBlockDriver,
+		KernelPath:     filepath.Join(testDir, testKernel),
+		ImagePath:      filepath.Join(testDir, testImage),
+		HypervisorPath: filepath.Join(testDir, testHypervisor),
+		DefaultVCPUs:   defaultVCPUs,
+		DefaultMemSz:   defaultMemSzMiB,
+		DefaultBridges: defaultBridges,
 	}
 
 	expectedStatus := PodStatus{
@@ -931,13 +930,12 @@ 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,
-		BlockDeviceDriver: defaultBlockDriver,
+		KernelPath:     filepath.Join(testDir, testKernel),
+		ImagePath:      filepath.Join(testDir, testImage),
+		HypervisorPath: filepath.Join(testDir, testHypervisor),
+		DefaultVCPUs:   defaultVCPUs,
+		DefaultMemSz:   defaultMemSzMiB,
+		DefaultBridges: defaultBridges,
 	}
 
 	expectedStatus := PodStatus{

container.go

@@ -680,25 +680,24 @@ 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,11 +340,7 @@ type BlockDevice struct {
 	DeviceType string
 	DeviceInfo DeviceInfo
 
-	// 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
+	// Path at which the device appears inside the VM, outside of the container mount namespace.
 	VirtPath string
 }
 
@@ -363,6 +359,12 @@ 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.
@@ -378,13 +380,6 @@ 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
@@ -398,17 +393,7 @@ func (device *BlockDevice) attach(h hypervisor, c *Container) (err error) {
 
 	device.DeviceInfo.Hotplugged = true
 
-	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
-	}
-
+	device.VirtPath = filepath.Join("/dev", driveName)
 	return nil
 }
 

device_test.go

@@ -322,19 +322,10 @@ 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"
@@ -385,18 +376,4 @@ 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,22 +414,13 @@ func (h *hyper) startOneContainer(pod Pod, c *Container) error {
 	container.SystemMountsInfo.BindMountDev = c.systemMountsInfo.BindMountDev
 
 	if c.state.Fstype != "" {
-		// 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
+		driveName, err := getVirtDriveName(c.state.BlockIndex)
+		if err != nil {
+			return err
 		}
 
 		container.Fstype = c.state.Fstype
+		container.Image = driveName
 	} else {
 
 		if err := bindMountContainerRootfs(defaultSharedDir, pod.id, c.id, c.rootFs, false); err != nil {
@@ -459,7 +450,6 @@ 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,8 +47,6 @@ const (
 	defaultMemSzMiB = 2048
 
 	defaultBridges = 1
-
-	defaultBlockDriver = VirtioSCSI
 )
 
 // deviceType describes a virtualized device type.
@@ -157,10 +155,6 @@ 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
 
@@ -233,10 +227,6 @@ func (conf *HypervisorConfig) valid() (bool, error) {
 		conf.DefaultBridges = defaultBridges
 	}
 
-	if conf.BlockDeviceDriver == "" {
-		conf.BlockDeviceDriver = defaultBlockDriver
-	}
-
 	return true, nil
 }
 

hypervisor_test.go

@@ -174,13 +174,12 @@ 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,
-		BlockDeviceDriver: defaultBlockDriver,
+		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath: "",
+		DefaultVCPUs:   defaultVCPUs,
+		DefaultMemSz:   defaultMemSzMiB,
+		DefaultBridges: defaultBridges,
 	}
 	if reflect.DeepEqual(hypervisorConfig, hypervisorConfigDefaultsExpected) == false {
 		t.Fatal()

mount.go

@@ -321,30 +321,3 @@ 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,7 +19,6 @@ package virtcontainers
 import (
 	"bytes"
 	"fmt"
-	"github.com/stretchr/testify/assert"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -283,52 +282,3 @@ 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,7 +133,6 @@ 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.
@@ -208,7 +207,7 @@ type Container struct {
 	Rootfs           string              `json:"rootfs"`
 	Fstype           string              `json:"fstype,omitempty"`
 	Image            string              `json:"image"`
-	SCSIAddr         string              `json:"scsiAddr,omitempty"`
+	Addr             string              `json:"addr,omitempty"`
 	Volumes          []*VolumeDescriptor `json:"volumes,omitempty"`
 	Fsmap            []*FsmapDescriptor  `json:"fsmap,omitempty"`
 	Sysctl           map[string]string   `json:"sysctl,omitempty"`

pod.go

@@ -245,9 +245,6 @@ 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