Merge pull request #1742 from kevpar/getscsiuvmpath

Remove dependence on GetScsiUvmPath function

cmd/gcstools/installdrivers.go

@@ -6,6 +6,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -27,6 +28,14 @@ func install(ctx context.Context) error {
 	targetOverlayPath := args[0]
 	driver := args[1]
 
+	if _, err := os.Lstat(targetOverlayPath); err == nil {
+		// We assume the overlay path to be unique per set of drivers. Thus, if the path
+		// exists already, we have already installed these drivers, and can quit early.
+		return nil
+	} else if !errors.Is(err, fs.ErrNotExist) {
+		return fmt.Errorf("failed to stat overlay dir: %s: %w", targetOverlayPath, err)
+	}
+
 	// create an overlay mount from the driver's UVM path so we can write to the
 	// mount path in the UVM despite having mounted in the driver originally as
 	// readonly

internal/devices/drivers.go

@@ -53,14 +53,9 @@ func InstallDrivers(ctx context.Context, vm *uvm.UtilityVM, share string, gpuDri
 		return closer, execPnPInstallDriver(ctx, vm, uvmPath)
 	}
 
-	// no need to reinstall if the driver has already been added in LCOW, return early
-	if _, err := vm.GetScsiUvmPath(ctx, share); err != uvm.ErrNotAttached {
-		return nil, err
-	}
-
 	// first mount driver as scsi in standard mount location
 	uvmPathForShare := fmt.Sprintf(guestpath.LCOWGlobalMountPrefixFmt, vm.UVMMountCounter())
-	closer, err = vm.AddSCSI(ctx,
+	mount, err := vm.AddSCSI(ctx,
 		share,
 		uvmPathForShare,
 		true,
@@ -70,9 +65,14 @@ func InstallDrivers(ctx context.Context, vm *uvm.UtilityVM, share string, gpuDri
 	if err != nil {
 		return closer, fmt.Errorf("failed to add SCSI disk to utility VM for path %+v: %s", share, err)
 	}
+	closer = mount
+	uvmPathForShare = mount.UVMPath
 
 	// construct path that the drivers will be remounted as read/write in the UVM
-	driverGUID, err := guid.NewV4()
+
+	// 914aadc8-f700-4365-8016-ddad0a9d406d. Random GUID chosen for namespace.
+	ns := guid.GUID{Data1: 0x914aadc8, Data2: 0xf700, Data3: 0x4365, Data4: [8]byte{0x80, 0x16, 0xdd, 0xad, 0x0a, 0x9d, 0x40, 0x6d}}
+	driverGUID, err := guid.NewV5(ns, []byte(share))
 	if err != nil {
 		return closer, fmt.Errorf("failed to create a guid path for driver %+v: %s", share, err)
 	}

internal/hcsoci/create.go

@@ -66,6 +66,8 @@ type createOptionsInternal struct {
 	actualOwner            string             // Owner for the container
 	actualNetworkNamespace string
 	ccgState               *hcsschema.ContainerCredentialGuardState // Container Credential Guard information to be attached to HCS container document
+
+	windowsAdditionalMounts []hcsschema.MappedDirectory // Holds additional mounts based on added devices (such as SCSI). Only used for Windows v2 schema containers.
 }
 
 func validateContainerConfig(ctx context.Context, coi *createOptionsInternal) error {
@@ -173,7 +175,7 @@ func CreateContainer(ctx context.Context, createOptions *CreateOptions) (_ cow.C
 		return nil, nil, fmt.Errorf("container config validation failed: %s", err)
 	}
 
-	r := resources.NewContainerResources(createOptions.ID)
+	r := resources.NewContainerResources(coi.ID)
 	defer func() {
 		if err != nil {
 			if !coi.DoNotReleaseResourcesOnFailure {
@@ -184,7 +186,7 @@ func CreateContainer(ctx context.Context, createOptions *CreateOptions) (_ cow.C
 
 	if coi.HostingSystem != nil {
 		if coi.Spec.Linux != nil {
-			r.SetContainerRootInUVM(fmt.Sprintf(lcowRootInUVM, createOptions.ID))
+			r.SetContainerRootInUVM(fmt.Sprintf(lcowRootInUVM, coi.ID))
 		} else {
 			n := coi.HostingSystem.ContainerCounter()
 			r.SetContainerRootInUVM(fmt.Sprintf(wcowRootInUVM, strconv.FormatUint(n, 16)))

internal/hcsoci/hcsdoc_wcow.go

@@ -69,19 +69,9 @@ func createMountsConfig(ctx context.Context, coi *createOptionsInternal) (*mount
 				}
 				mdv2.HostPath = src
 			} else if mount.Type == "virtual-disk" || mount.Type == "physical-disk" || mount.Type == "extensible-virtual-disk" {
-				mountPath := mount.Source
-				var err error
-				if mount.Type == "extensible-virtual-disk" {
-					_, mountPath, err = uvm.ParseExtensibleVirtualDiskPath(mount.Source)
-					if err != nil {
-						return nil, err
-					}
-				}
-				uvmPath, err := coi.HostingSystem.GetScsiUvmPath(ctx, mountPath)
-				if err != nil {
-					return nil, err
-				}
-				mdv2.HostPath = uvmPath
+				// For v2 schema containers, any disk mounts will be part of coi.additionalMounts.
+				// For v1 schema containers, we don't even get here, since there is no HostingSystem.
+				continue
 			} else if strings.HasPrefix(mount.Source, guestpath.SandboxMountPrefix) {
 				// Convert to the path in the guest that was asked for.
 				mdv2.HostPath = convertToWCOWSandboxMountPath(mount.Source)
@@ -97,6 +87,7 @@ func createMountsConfig(ctx context.Context, coi *createOptionsInternal) (*mount
 			config.mdsv2 = append(config.mdsv2, mdv2)
 		}
 	}
+	config.mdsv2 = append(config.mdsv2, coi.windowsAdditionalMounts...)
 	return &config, nil
 }
 

internal/hcsoci/resources_wcow.go

@@ -19,6 +19,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/cmd"
 	"github.com/Microsoft/hcsshim/internal/credentials"
 	"github.com/Microsoft/hcsshim/internal/guestpath"
+	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/layers"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/resources"
@@ -151,35 +152,52 @@ func setupMounts(ctx context.Context, coi *createOptionsInternal, r *resources.R
 				}
 			}
 			l := log.G(ctx).WithField("mount", fmt.Sprintf("%+v", mount))
-			if mount.Type == "physical-disk" {
-				l.Debug("hcsshim::allocateWindowsResources Hot-adding SCSI physical disk for OCI mount")
-				scsiMount, err := coi.HostingSystem.AddSCSIPhysicalDisk(ctx, mount.Source, uvmPath, readOnly, mount.Options)
-				if err != nil {
-					return errors.Wrapf(err, "adding SCSI physical disk mount %+v", mount)
-				}
-				r.Add(scsiMount)
-			} else if mount.Type == "virtual-disk" {
-				l.Debug("hcsshim::allocateWindowsResources Hot-adding SCSI virtual disk for OCI mount")
-				scsiMount, err := coi.HostingSystem.AddSCSI(
-					ctx,
-					mount.Source,
-					uvmPath,
-					readOnly,
-					false,
-					mount.Options,
-					uvm.VMAccessTypeIndividual,
+			if mount.Type == "physical-disk" || mount.Type == "virtual-disk" || mount.Type == "extensible-virtual-disk" {
+				var (
+					scsiMount *uvm.SCSIMount
+					err       error
 				)
-				if err != nil {
-					return errors.Wrapf(err, "adding SCSI virtual disk mount %+v", mount)
+				switch mount.Type {
+				case "physical-disk":
+					l.Debug("hcsshim::allocateWindowsResources Hot-adding SCSI physical disk for OCI mount")
+					scsiMount, err = coi.HostingSystem.AddSCSIPhysicalDisk(
+						ctx,
+						mount.Source,
+						uvmPath,
+						readOnly,
+						mount.Options,
+					)
+				case "virtual-disk":
+					l.Debug("hcsshim::allocateWindowsResources Hot-adding SCSI virtual disk for OCI mount")
+					scsiMount, err = coi.HostingSystem.AddSCSI(
+						ctx,
+						mount.Source,
+						uvmPath,
+						readOnly,
+						false,
+						mount.Options,
+						uvm.VMAccessTypeIndividual,
+					)
+				case "extensible-virtual-disk":
+					l.Debug("hcsshim::allocateWindowsResource Hot-adding ExtensibleVirtualDisk")
+					scsiMount, err = coi.HostingSystem.AddSCSIExtensibleVirtualDisk(
+						ctx,
+						mount.Source,
+						uvmPath,
+						readOnly,
+					)
 				}
-				r.Add(scsiMount)
-			} else if mount.Type == "extensible-virtual-disk" {
-				l.Debug("hcsshim::allocateWindowsResource Hot-adding ExtensibleVirtualDisk")
-				scsiMount, err := coi.HostingSystem.AddSCSIExtensibleVirtualDisk(ctx, mount.Source, uvmPath, readOnly)
 				if err != nil {
-					return errors.Wrapf(err, "adding SCSI EVD mount failed %+v", mount)
+					return fmt.Errorf("adding SCSI mount %+v: %w", mount, err)
 				}
 				r.Add(scsiMount)
+				// Compute guest mounts now, and store them, so they can be added to the container doc later.
+				// We do this now because we have access to the guest path through the returned mount object.
+				coi.windowsAdditionalMounts = append(coi.windowsAdditionalMounts, hcsschema.MappedDirectory{
+					HostPath:      scsiMount.UVMPath,
+					ContainerPath: mount.Destination,
+					ReadOnly:      readOnly,
+				})
 			} else if strings.HasPrefix(mount.Source, guestpath.SandboxMountPrefix) {
 				// Mounts that map to a path in the UVM are specified with a 'sandbox://' prefix.
 				//

internal/uvm/scsi.go

@@ -519,19 +519,6 @@ func (uvm *UtilityVM) allocateSCSIMount(
 	return uvm.scsiLocations[controller][lun], false, nil
 }
 
-// GetScsiUvmPath returns the guest mounted path of a SCSI drive.
-//
-// If `hostPath` is not mounted returns `ErrNotAttached`.
-func (uvm *UtilityVM) GetScsiUvmPath(ctx context.Context, hostPath string) (string, error) {
-	uvm.m.Lock()
-	defer uvm.m.Unlock()
-	sm, err := uvm.findSCSIAttachment(ctx, hostPath)
-	if err != nil {
-		return "", err
-	}
-	return sm.UVMPath, err
-}
-
 // ScratchEncryptionEnabled is a getter for `uvm.encryptScratch`.
 //
 // Returns true if the scratch disks should be encrypted, false otherwise.