scsi: Take optional guest path for mount

Currently the SCSI mount manager will generate a new path for each new
mount, based on a format string that it is instantiated with. However,
it turns out some code in the GCS (e.g. sandbox mounts) assumes that the
container scratch is mounted at a certain path. The long-term best
solution here is probably to pass what paths to use explicitly to the
GCS, but that would be more impactful. We need a more contained fix.

This commit addresses the issue by allowing an optional guest path to be
given for a SCSI mount. The mount manager has been changed as follows:
- If a guest path is not supplied: The mount can re-use (refcount++) any
  existing mount with the same controller/lun/options. If a new mount is
  created, the mount manager will generate a path for it.
- If a guest path is supplied: The mount can re-use (refcount++) any
  existing mount with the same controller/lun/guestpath/options. If a
  new mount is created, the mount manager will use the supplied path for
  it.

Accordingly, code calling into the mount manager has been updated to
pass an empty string for the guest path. The exception to this is the
LCOW layer mounting code, which will pass an explicit guest path for the
scratch disk. As far as I know, WCOW does not depend on a specific path
for its scratch disk.

Signed-off-by: Kevin Parsons <[email protected]>

internal/devices/drivers.go

@@ -60,6 +60,7 @@ func InstallDrivers(ctx context.Context, vm *uvm.UtilityVM, share string) (close
 		share,
 		true,
 		vm.ID(),
+		"",
 		&scsi.MountConfig{},
 	)
 	if err != nil {

internal/hcsoci/resources_lcow.go

@@ -92,6 +92,7 @@ func allocateLinuxResources(ctx context.Context, coi *createOptionsInternal, r *
 					hostPath,
 					readOnly,
 					coi.HostingSystem.ID(),
+					"",
 					&scsi.MountConfig{Options: mount.Options, BlockDev: isBlockDev},
 				)
 				if err != nil {
@@ -114,6 +115,7 @@ func allocateLinuxResources(ctx context.Context, coi *createOptionsInternal, r *
 					hostPath,
 					readOnly,
 					coi.HostingSystem.ID(),
+					"",
 					&scsi.MountConfig{Options: mount.Options, BlockDev: isBlockDev},
 				)
 				if err != nil {

internal/hcsoci/resources_wcow.go

@@ -144,6 +144,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R
 						mount.Source,
 						readOnly,
 						coi.HostingSystem.ID(),
+						"",
 						&scsi.MountConfig{},
 					)
 				case MountTypeVirtualDisk:
@@ -153,6 +154,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R
 						mount.Source,
 						readOnly,
 						coi.HostingSystem.ID(),
+						"",
 						&scsi.MountConfig{},
 					)
 				case MountTypeExtensibleVirtualDisk:
@@ -161,6 +163,7 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R
 						ctx,
 						mount.Source,
 						readOnly,
+						"",
 						&scsi.MountConfig{},
 					)
 				}

internal/layers/lcow.go

@@ -136,6 +136,7 @@ func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers
 		hostPath,
 		false,
 		vm.ID(),
+		guestRoot,
 		mConfig,
 	)
 	if err != nil {
@@ -194,6 +195,7 @@ func addLCOWLayer(ctx context.Context, vm *uvm.UtilityVM, layer *LCOWLayer) (uvm
 		layer.VHDPath,
 		true,
 		"",
+		"",
 		&scsi.MountConfig{
 			Partition: layer.Partition,
 			Options:   []string{"ro"},

internal/layers/wcow_mount.go

@@ -331,7 +331,7 @@ func mountHypervIsolatedWCIFSLayers(ctx context.Context, l *wcowWCIFSLayers, vm
 	hostPath := filepath.Join(l.scratchLayerPath, "sandbox.vhdx")
 	log.G(ctx).WithField("hostPath", hostPath).Debug("mounting scratch VHD")
 
-	scsiMount, err := vm.SCSIManager.AddVirtualDisk(ctx, hostPath, false, vm.ID(), &scsi.MountConfig{})
+	scsiMount, err := vm.SCSIManager.AddVirtualDisk(ctx, hostPath, false, vm.ID(), "", &scsi.MountConfig{})
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to add SCSI scratch VHD: %w", err)
 	}

internal/lcow/disk.go

@@ -31,7 +31,7 @@ func FormatDisk(ctx context.Context, lcowUVM *uvm.UtilityVM, destPath string) er
 	}).Debug("lcow::FormatDisk opts")
 
 	// Attach without mounting.
-	scsi, err := lcowUVM.SCSIManager.AddPhysicalDisk(ctx, destPath, false, lcowUVM.ID(), nil)
+	scsi, err := lcowUVM.SCSIManager.AddPhysicalDisk(ctx, destPath, false, lcowUVM.ID(), "", nil)
 	if err != nil {
 		return err
 	}

internal/lcow/scratch.go

@@ -75,6 +75,7 @@ func CreateScratch(ctx context.Context, lcowUVM *uvm.UtilityVM, destFile string,
 		destFile,
 		false,
 		lcowUVM.ID(),
+		"",
 		&scsi.MountConfig{
 			BlockDev: true,
 		},

internal/tools/uvmboot/mounts.go

@@ -25,6 +25,7 @@ func mountSCSI(ctx context.Context, c *cli.Context, vm *uvm.UtilityVM) error {
 			m.host,
 			!m.writable,
 			vm.ID(),
+			"",
 			&scsi.MountConfig{},
 		)
 		if err != nil {

internal/uvm/scsi/manager.go

@@ -141,6 +141,7 @@ func (m *Manager) AddVirtualDisk(
 	hostPath string,
 	readOnly bool,
 	vmID string,
+	guestPath string,
 	mc *MountConfig,
 ) (*Mount, error) {
 	if m == nil {
@@ -169,6 +170,7 @@ func (m *Manager) AddVirtualDisk(
 			readOnly: readOnly,
 			typ:      "VirtualDisk",
 		},
+		guestPath,
 		mcInternal)
 }
 
@@ -187,6 +189,7 @@ func (m *Manager) AddPhysicalDisk(
 	hostPath string,
 	readOnly bool,
 	vmID string,
+	guestPath string,
 	mc *MountConfig,
 ) (*Mount, error) {
 	if m == nil {
@@ -215,6 +218,7 @@ func (m *Manager) AddPhysicalDisk(
 			readOnly: readOnly,
 			typ:      "PassThru",
 		},
+		guestPath,
 		mcInternal)
 }
 
@@ -233,6 +237,7 @@ func (m *Manager) AddExtensibleVirtualDisk(
 	ctx context.Context,
 	hostPath string,
 	readOnly bool,
+	guestPath string,
 	mc *MountConfig,
 ) (*Mount, error) {
 	if m == nil {
@@ -260,10 +265,11 @@ func (m *Manager) AddExtensibleVirtualDisk(
 			typ:      "ExtensibleVirtualDisk",
 			evdType:  evdType,
 		},
+		guestPath,
 		mcInternal)
 }
 
-func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, mountConfig *mountConfig) (_ *Mount, err error) {
+func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, guestPath string, mountConfig *mountConfig) (_ *Mount, err error) {
 	controller, lun, err := m.attachManager.attach(ctx, attachConfig)
 	if err != nil {
 		return nil, err
@@ -274,9 +280,8 @@ func (m *Manager) add(ctx context.Context, attachConfig *attachConfig, mountConf
 		}
 	}()
 
-	var guestPath string
 	if mountConfig != nil {
-		guestPath, err = m.mountManager.mount(ctx, controller, lun, mountConfig)
+		guestPath, err = m.mountManager.mount(ctx, controller, lun, guestPath, mountConfig)
 		if err != nil {
 			return nil, err
 		}

internal/uvm/scsi/mount.go

@@ -47,14 +47,17 @@ type mountConfig struct {
 	filesystem       string
 }
 
-func (mm *mountManager) mount(ctx context.Context, controller, lun uint, c *mountConfig) (_ string, err error) {
+func (mm *mountManager) mount(ctx context.Context, controller, lun uint, path string, c *mountConfig) (_ string, err error) {
 	// Normalize the mount config for comparison.
 	// Config equality relies on the options slices being compared element-wise. Sort the options
 	// slice first so that two slices with different ordering compare as equal. We assume that
 	// order will never matter for mount options.
 	sort.Strings(c.options)
 
-	mount, existed := mm.trackMount(controller, lun, c)
+	mount, existed, err := mm.trackMount(controller, lun, path, c)
+	if err != nil {
+		return "", err
+	}
 	if existed {
 		select {
 		case <-ctx.Done():
@@ -108,7 +111,7 @@ func (mm *mountManager) unmount(ctx context.Context, path string) (bool, error)
 	return true, nil
 }
 
-func (mm *mountManager) trackMount(controller, lun uint, c *mountConfig) (*mount, bool) {
+func (mm *mountManager) trackMount(controller, lun uint, path string, c *mountConfig) (*mount, bool, error) {
 	mm.m.Lock()
 	defer mm.m.Unlock()
 
@@ -120,15 +123,19 @@ func (mm *mountManager) trackMount(controller, lun uint, c *mountConfig) (*mount
 			}
 		} else if controller == mount.controller &&
 			lun == mount.lun &&
+			(path == "" || path == mount.path) &&
 			reflect.DeepEqual(c, mount.config) {
 
 			mount.refCount++
-			return mount, true
+			return mount, true, nil
+		} else if path != "" && path == mount.path {
+			return nil, false, fmt.Errorf("cannot mount over an existing mountpoint: %s", path)
 		}
 	}
 
 	// New mount.
 	mount := &mount{
+		path:       path, // If path is empty, this will be replaced with a generated path below.
 		controller: controller,
 		lun:        lun,
 		config:     c,
@@ -142,9 +149,11 @@ func (mm *mountManager) trackMount(controller, lun uint, c *mountConfig) (*mount
 		mount.index = freeIndex
 		mm.mounts[freeIndex] = mount
 	}
-	// Use the mount index to produce a unique guest path.
-	mount.path = fmt.Sprintf(mm.mountFmt, mount.index)
-	return mount, false
+	if mount.path == "" {
+		// Use the mount index to produce a unique guest path.
+		mount.path = fmt.Sprintf(mm.mountFmt, mount.index)
+	}
+	return mount, false, nil
 }
 
 // Caller must be holding mm.m.