Skip to content

Commit

Permalink
Support for multiple SCSI controllers (microsoft#1328)
Browse files Browse the repository at this point in the history 
* Support for multiple SCSI controllers

Enable using upto 4 SCSI controllers for LCOW UVMs. HCS currently doesn't respect the
SCSI controller number provided with the Add SCSI disk requests. Hence, the SCSI disk can
show up at some different controller inside the LCOW UVM. To avoid this, now we use GUIDs
to represent each controller and use that GUID with the Add SCSI disk request.
GCS code is also modified to identify the controller number from the controller GUID. Now if a LCOW pod is created with an annotation that sets VPMEM device count to 0, we will automatically enable 4 SCSI controllers. Even the rootfs.vhd will be attached via SCSI in that scenario. 

Signed-off-by: Amit Barve <ambarve@microsoft.com>
  • Loading branch information
@ambarve
ambarve authored Mar 28, 2022
1 parent cc9e1f8 commit 4656282
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 104 deletions.
107 changes: 96 additions & 11 deletions guest/storage/scsi/scsi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import (
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/pkg/errors"
Expand All @@ -20,6 +23,7 @@ import (
dm "github.com/Microsoft/hcsshim/internal/guest/storage/devicemapper"
"github.com/Microsoft/hcsshim/internal/log"
"github.com/Microsoft/hcsshim/internal/oc"
"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
"github.com/Microsoft/hcsshim/pkg/securitypolicy"
)
Expand All @@ -39,19 +43,53 @@ var (
)

const (
scsiDevicesPath = "/sys/bus/scsi/devices"
verityDeviceFmt = "verity-scsi-contr%d-lun%d-%s"
scsiDevicesPath = "/sys/bus/scsi/devices"
vmbusDevicesPath = "/sys/bus/vmbus/devices"
verityDeviceFmt = "verity-scsi-contr%d-lun%d-%s"
)

// Mount creates a mount from the SCSI device on `controller` index `lun` to
// fetchActualControllerNumber retrieves the actual controller number assigned to a SCSI controller
// with number `passedController`.
// When HCS creates the UVM it adds 4 SCSI controllers to the UVM but the 1st SCSI
// controller according to HCS can actually show up as 2nd, 3rd or 4th controller inside
// the UVM. So the i'th controller from HCS' perspective could actually be j'th controller
// inside the UVM. However, we can refer to the SCSI controllers with their GUIDs (that
// are hardcoded) and then using that GUID find out the SCSI controller number inside the
// guest. This function does exactly that.
func fetchActualControllerNumber(ctx context.Context, passedController uint8) (uint8, error) {
// find the controller number by looking for a file named host<N> (e.g host1, host3 etc.)
// `N` is the controller number.
// Full file path would be /sys/bus/vmbus/devices/<controller-guid>/host<N>.
controllerDirPath := path.Join(vmbusDevicesPath, guestrequest.ScsiControllerGuids[passedController])
entries, err := ioutil.ReadDir(controllerDirPath)
if err != nil {
return 0, err
}

for _, entry := range entries {
baseName := path.Base(entry.Name())
if !strings.HasPrefix(baseName, "host") {
continue
}
controllerStr := baseName[len("host"):]
controllerNum, err := strconv.ParseUint(controllerStr, 10, 8)
if err != nil {
return 0, fmt.Errorf("failed to parse controller number from %s: %w", baseName, err)
}
return uint8(controllerNum), nil
}
return 0, fmt.Errorf("host<N> directory not found inside %s", controllerDirPath)
}

// mount creates a mount from the SCSI device on `controller` index `lun` to
// `target`
//
// `target` will be created. On mount failure the created `target` will be
// automatically cleaned up.
//
// If `encrypted` is set to true, the SCSI device will be encrypted using
// dm-crypt.
func Mount(
func mount(
ctx context.Context,
controller,
lun uint8,
Expand Down Expand Up @@ -159,10 +197,30 @@ func Mount(
return nil
}

// Unmount unmounts a SCSI device mounted at `target`.
// Mount is just a wrapper over actual mount call. This wrapper finds out the controller
// number from the controller GUID string and calls mount.
func Mount(
ctx context.Context,
controller,
lun uint8,
target string,
readonly bool,
encrypted bool,
options []string,
verityInfo *guestresource.DeviceVerityInfo,
securityPolicy securitypolicy.SecurityPolicyEnforcer,
) (err error) {
cNum, err := fetchActualControllerNumber(ctx, controller)
if err != nil {
return err
}
return mount(ctx, cNum, lun, target, readonly, encrypted, options, verityInfo, securityPolicy)
}

// unmount unmounts a SCSI device mounted at `target`.
//
// If `encrypted` is true, it removes all its associated dm-crypto state.
func Unmount(
func unmount(
ctx context.Context,
controller,
lun uint8,
Expand Down Expand Up @@ -206,6 +264,24 @@ func Unmount(
return nil
}

// Unmount is just a wrapper over actual unmount call. This wrapper finds out the controller
// number from the controller GUID string and calls mount.
func Unmount(
ctx context.Context,
controller,
lun uint8,
target string,
encrypted bool,
verityInfo *guestresource.DeviceVerityInfo,
securityPolicy securitypolicy.SecurityPolicyEnforcer,
) (err error) {
cNum, err := fetchActualControllerNumber(ctx, controller)
if err != nil {
return err
}
return unmount(ctx, cNum, lun, target, encrypted, verityInfo, securityPolicy)
}

// ControllerLunToName finds the `/dev/sd*` path to the SCSI device on
// `controller` index `lun`.
func ControllerLunToName(ctx context.Context, controller, lun uint8) (_ string, err error) {
Expand All @@ -217,8 +293,7 @@ func ControllerLunToName(ctx context.Context, controller, lun uint8) (_ string,
trace.Int64Attribute("controller", int64(controller)),
trace.Int64Attribute("lun", int64(lun)))

scsiID := fmt.Sprintf("0:0:%d:%d", controller, lun)

scsiID := fmt.Sprintf("%d:0:0:%d", controller, lun)
// Devices matching the given SCSI code should each have a subdirectory
// under /sys/bus/scsi/devices/<scsiID>/block.
blockPath := filepath.Join(scsiDevicesPath, scsiID, "block")
Expand Down Expand Up @@ -249,11 +324,11 @@ func ControllerLunToName(ctx context.Context, controller, lun uint8) (_ string,
return devicePath, nil
}

// UnplugDevice finds the SCSI device on `controller` index `lun` and issues a
// unplugDevice finds the SCSI device on `controller` index `lun` and issues a
// guest initiated unplug.
//
// If the device is not attached returns no error.
func UnplugDevice(ctx context.Context, controller, lun uint8) (err error) {
func unplugDevice(ctx context.Context, controller, lun uint8) (err error) {
_, span := trace.StartSpan(ctx, "scsi::UnplugDevice")
defer span.End()
defer func() { oc.SetSpanStatus(span, err) }()
Expand All @@ -262,7 +337,7 @@ func UnplugDevice(ctx context.Context, controller, lun uint8) (err error) {
trace.Int64Attribute("controller", int64(controller)),
trace.Int64Attribute("lun", int64(lun)))

scsiID := fmt.Sprintf("0:0:%d:%d", controller, lun)
scsiID := fmt.Sprintf("%d:0:0:%d", controller, lun)
f, err := os.OpenFile(filepath.Join(scsiDevicesPath, scsiID, "delete"), os.O_WRONLY, 0644)
if err != nil {
if os.IsNotExist(err) {
Expand All @@ -277,3 +352,13 @@ func UnplugDevice(ctx context.Context, controller, lun uint8) (err error) {
}
return nil
}

// UnplugDevice is just a wrapper over actual unplugDevice call. This wrapper finds out the controller
// number from the controller GUID string and calls unplugDevice.
func UnplugDevice(ctx context.Context, controller, lun uint8) (err error) {
cNum, err := fetchActualControllerNumber(ctx, controller)
if err != nil {
return err
}
return unplugDevice(ctx, cNum, lun)
}
38 changes: 19 additions & 19 deletions guest/storage/scsi/scsi_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func Test_Mount_Mkdir_Fails_Error(t *testing.T) {
return "", nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -74,7 +74,7 @@ func Test_Mount_Mkdir_ExpectedPath(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -111,7 +111,7 @@ func Test_Mount_Mkdir_ExpectedPerm(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -148,7 +148,7 @@ func Test_Mount_ControllerLunToName_Valid_Controller(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
expectedController,
0,
Expand Down Expand Up @@ -185,7 +185,7 @@ func Test_Mount_ControllerLunToName_Valid_Lun(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
expectedLun,
Expand Down Expand Up @@ -225,7 +225,7 @@ func Test_Mount_Calls_RemoveAll_OnMountFailure(t *testing.T) {
return expectedErr
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -263,7 +263,7 @@ func Test_Mount_Valid_Source(t *testing.T) {
}
return nil
}
err := Mount(context.Background(), 0, 0, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
err := mount(context.Background(), 0, 0, "/fake/path", false, false, nil, nil, openDoorSecurityPolicyEnforcer())
if err != nil {
t.Fatalf("expected nil err, got: %v", err)
}
Expand All @@ -290,7 +290,7 @@ func Test_Mount_Valid_Target(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -326,7 +326,7 @@ func Test_Mount_Valid_FSType(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -362,7 +362,7 @@ func Test_Mount_Valid_Flags(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -398,7 +398,7 @@ func Test_Mount_Readonly_Valid_Flags(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -433,7 +433,7 @@ func Test_Mount_Valid_Data(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -469,7 +469,7 @@ func Test_Mount_Readonly_Valid_Data(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -506,7 +506,7 @@ func Test_Read_Only_Security_Policy_Enforcement_Mount_Calls(t *testing.T) {
}

enforcer := mountMonitoringSecurityPolicyEnforcer()
err := Mount(context.Background(), 0, 0, target, true, false, nil, nil, enforcer)
err := mount(context.Background(), 0, 0, target, true, false, nil, nil, enforcer)
if err != nil {
t.Fatalf("expected nil err, got: %v", err)
}
Expand Down Expand Up @@ -549,7 +549,7 @@ func Test_Read_Write_Security_Policy_Enforcement_Mount_Calls(t *testing.T) {
}

enforcer := mountMonitoringSecurityPolicyEnforcer()
err := Mount(context.Background(), 0, 0, target, false, false, nil, nil, enforcer)
err := mount(context.Background(), 0, 0, target, false, false, nil, nil, enforcer)
if err != nil {
t.Fatalf("expected nil err, got: %v", err)
}
Expand Down Expand Up @@ -592,12 +592,12 @@ func Test_Security_Policy_Enforcement_Unmount_Calls(t *testing.T) {
}

enforcer := mountMonitoringSecurityPolicyEnforcer()
err := Mount(context.Background(), 0, 0, target, true, false, nil, nil, enforcer)
err := mount(context.Background(), 0, 0, target, true, false, nil, nil, enforcer)
if err != nil {
t.Fatalf("expected nil err, got: %v", err)
}

err = Unmount(context.Background(), 0, 0, target, false, nil, enforcer)
err = unmount(context.Background(), 0, 0, target, false, nil, enforcer)
if err != nil {
t.Fatalf("expected nil err, got: %v", err)
}
Expand Down Expand Up @@ -668,7 +668,7 @@ func Test_CreateVerityTarget_And_Mount_Called_With_Correct_Parameters(t *testing
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down Expand Up @@ -717,7 +717,7 @@ func Test_osMkdirAllFails_And_RemoveDevice_Called(t *testing.T) {
return nil
}

if err := Mount(
if err := mount(
context.Background(),
0,
0,
Expand Down
13 changes: 13 additions & 0 deletions protocol/guestrequest/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,16 @@ type RS4NetworkModifyRequest struct {
RequestType RequestType `json:"RequestType,omitempty"`
Settings interface{} `json:"Settings,omitempty"`
}

var (
// V5 GUIDs for SCSI controllers
// These GUIDs are created with namespace GUID "d422512d-2bf2-4752-809d-7b82b5fcb1b4"
// and index as names. For example, first GUID is created like this:
// guid.NewV5("d422512d-2bf2-4752-809d-7b82b5fcb1b4", []byte("0"))
ScsiControllerGuids = []string{
"df6d0690-79e5-55b6-a5ec-c1e2f77f580a",
"0110f83b-de10-5172-a266-78bca56bf50a",
"b5d2d8d4-3a75-51bf-945b-3444dc6b8579",
"305891a9-b251-5dfe-91a2-c25d9212275b",
}
)
15 changes: 15 additions & 0 deletions protocol/guestrequest/types_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package guestrequest

import (
"github.com/Microsoft/go-winio/pkg/guid"
"testing"
)

func TestGuidValidity(t *testing.T) {
for _, g := range ScsiControllerGuids {
_, err := guid.FromString(g)
if err != nil {
t.Fatalf("GUID parsing failed: %s", err)
}
}
}
5 changes: 5 additions & 0 deletions uvm/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package uvm

import (
"errors"

"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
)

const (
Expand All @@ -21,4 +23,7 @@ const (
var (
errNotSupported = errors.New("not supported")
errBadUVMOpts = errors.New("UVM options incorrect")

// Maximum number of SCSI controllers allowed
MaxSCSIControllers = uint32(len(guestrequest.ScsiControllerGuids))
)
Loading

0 comments on commit 4656282

Please sign in to comment.