Skip to content

Commit

Permalink
Remove dependency on GetScsiUvmPath from WCOW-isolated mounts
Browse files Browse the repository at this point in the history
The WCOW-isolated SCSI mount process currently works as follows:
- In resources_wcow.go, go through each mount on the OCI spec, and if it
  is a SCSI mount, add a mount to the UVM for it.
- in hcsdoc_wcow.go, go through each mount on the OCI spec, use
  GetScsiUvmPath to determine the guest path it was mounted to, and add
  an entry to the container doc for it.

This is quite hacky, as it relies on a 1:1 mapping between host VHDs and
mounts in the guest, and also because it requires us to re-query
information we've already been given. The SCSIMount object returned when
we mounted to the guest can already tell us the guest path.

This change resolves this problem by instead determing the set of guest
mounts that should be added to the container doc at the time when the
SCSI mounts are done, and saving it in the creation options. Then, when
we construct the actual container doc, we just grab those mounts and add
them in.

Signed-off-by: Kevin Parsons <kevpar@microsoft.com>
  • Loading branch information
kevpar committed Apr 26, 2023
1 parent 3fe129a commit 2b0a504
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 52 deletions.
6 changes: 4 additions & 2 deletions internal/hcsoci/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)))
Expand Down
17 changes: 4 additions & 13 deletions internal/hcsoci/hcsdoc_wcow.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}

Expand Down
66 changes: 42 additions & 24 deletions internal/hcsoci/resources_wcow.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
//
Expand Down
13 changes: 0 additions & 13 deletions internal/uvm/scsi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 2b0a504

Please sign in to comment.