Skip to content

CWCOW: Unmount CIM volume #2588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MahatiC wants to merge 1 commit into
base: main
Choose a base branch
from
+110 −103
Open

CWCOW: Unmount CIM volume #2588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
212 changes: 110 additions & 102 deletions internal/gcs-sidecar/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,71 +569,80 @@ func (b *Bridge) modifySettings(req *request) (err error) {
return errors.New("the request settings are not of type SecurityPolicyFragment")
}
return b.hostState.securityOptions.InjectFragment(ctx, r)

case guestresource.ResourceTypeWCOWBlockCims:
// This is request to mount the merged cim at given volumeGUID
if modifyGuestSettingsRequest.RequestType == guestrequest.RequestTypeRemove {
return fmt.Errorf("not implemented")
}

wcowBlockCimMounts := modifyGuestSettingsRequest.Settings.(*guestresource.CWCOWBlockCIMMounts)
containerID := wcowBlockCimMounts.ContainerID
log.G(ctx).Tracef("WCOWBlockCIMMounts { %v}", wcowBlockCimMounts)

// The block device takes some time to show up. Wait for a few seconds.
time.Sleep(2 * time.Second)
switch modifyGuestSettingsRequest.RequestType {
case guestrequest.RequestTypeAdd:
wcowBlockCimMounts := modifyGuestSettingsRequest.Settings.(*guestresource.CWCOWBlockCIMMounts)
containerID := wcowBlockCimMounts.ContainerID
log.G(ctx).Tracef("WCOWBlockCIMMounts Add { %v}", wcowBlockCimMounts)

// The block device takes some time to show up. Wait for a few seconds.
time.Sleep(2 * time.Second)
Copy link
Collaborator

@KenGordon KenGordon Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed (but not for today) this can't be a sleep before check but needs to be more like the LCOW scheme where it only sleeps if it fails. See

hcsshim/internal/guest/storage/scsi/scsi.go

Line 205 in cbc0126

// todo (maksiman): add better retry logic, similar to how SCSI device mounts are
which is a bit nasty but illustrates the point.


var layerCIMs []*cimfs.BlockCIM
layerHashes := make([]string, len(wcowBlockCimMounts.BlockCIMs))
layerDigests := make([][]byte, len(wcowBlockCimMounts.BlockCIMs))
for i, blockCimDevice := range wcowBlockCimMounts.BlockCIMs {
// Get the scsi device path for the blockCim lun
devNumber, err := windevice.GetDeviceNumberFromControllerLUN(
req.ctx,
0, /* controller is always 0 for wcow */
uint8(blockCimDevice.Lun))
if err != nil {
return fmt.Errorf("err getting scsiDevPath: %w", err)
}
physicalDevPath := fmt.Sprintf(devPathFormat, devNumber)
layerCim := cimfs.BlockCIM{
Type: cimfs.BlockCIMTypeDevice,
BlockPath: physicalDevPath,
CimName: blockCimDevice.CimName,
}
cimRootDigestBytes, err := cimfs.GetVerificationInfo(physicalDevPath)
if err != nil {
return fmt.Errorf("failed to get CIM verification info: %w", err)
}
layerDigests[i] = cimRootDigestBytes
layerHashes[i] = base64.URLEncoding.EncodeToString(cimRootDigestBytes)
Copy link
Collaborator

@KenGordon KenGordon Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed (also not for today), the layer hash strings ought to be hex encoded as per the linux ones.

layerCIMs = append(layerCIMs, &layerCim)

var layerCIMs []*cimfs.BlockCIM
layerHashes := make([]string, len(wcowBlockCimMounts.BlockCIMs))
layerDigests := make([][]byte, len(wcowBlockCimMounts.BlockCIMs))
for i, blockCimDevice := range wcowBlockCimMounts.BlockCIMs {
// Get the scsi device path for the blockCim lun
devNumber, err := windevice.GetDeviceNumberFromControllerLUN(
req.ctx,
0, /* controller is always 0 for wcow */
uint8(blockCimDevice.Lun))
if err != nil {
return fmt.Errorf("err getting scsiDevPath: %w", err)
log.G(ctx).Debugf("block CIM layer digest %s, path: %s\n", layerHashes[i], physicalDevPath)
}
physicalDevPath := fmt.Sprintf(devPathFormat, devNumber)
layerCim := cimfs.BlockCIM{
Type: cimfs.BlockCIMTypeDevice,
BlockPath: physicalDevPath,
CimName: blockCimDevice.CimName,

// skip the merged cim and verify individual layer hashes
Copy link
Collaborator

@KenGordon KenGordon Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a written explanation anywhere of why we don't need to check the merged CIM root hash? I think it needs to be explained here.

hashesToVerify := layerHashes
if len(layerHashes) > 1 {
hashesToVerify = layerHashes[1:]
}
cimRootDigestBytes, err := cimfs.GetVerificationInfo(physicalDevPath)

err := b.hostState.securityOptions.PolicyEnforcer.EnforceVerifiedCIMsPolicy(req.ctx, containerID, hashesToVerify)
if err != nil {
return fmt.Errorf("failed to get CIM verification info: %w", err)
return errors.Wrap(err, "CIM mount is denied by policy")
}
layerDigests[i] = cimRootDigestBytes
layerHashes[i] = base64.URLEncoding.EncodeToString(cimRootDigestBytes)
layerCIMs = append(layerCIMs, &layerCim)

log.G(ctx).Debugf("block CIM layer digest %s, path: %s\n", layerHashes[i], physicalDevPath)
}

// skip the merged cim and verify individual layer hashes
hashesToVerify := layerHashes
if len(layerHashes) > 1 {
hashesToVerify = layerHashes[1:]
}
if len(layerCIMs) > 1 {
_, err = cimfs.MountMergedVerifiedBlockCIMs(layerCIMs[0], layerCIMs[1:], wcowBlockCimMounts.MountFlags, wcowBlockCimMounts.VolumeGUID, layerDigests[0])
if err != nil {
return fmt.Errorf("error mounting multilayer block cims: %w", err)
}
} else {
_, err = cimfs.MountVerifiedBlockCIM(layerCIMs[0], wcowBlockCimMounts.MountFlags, wcowBlockCimMounts.VolumeGUID, layerDigests[0])
if err != nil {
return fmt.Errorf("error mounting verified block cim: %w", err)
}
}

err := b.hostState.securityOptions.PolicyEnforcer.EnforceVerifiedCIMsPolicy(req.ctx, containerID, hashesToVerify)
if err != nil {
return errors.Wrap(err, "CIM mount is denied by policy")
}
case guestrequest.RequestTypeRemove:
log.G(ctx).Tracef("WCOWBlockCIMMounts: Remove")
wcowBlockCimMounts := modifyGuestSettingsRequest.Settings.(*guestresource.CWCOWBlockCIMMounts)
volumePath := fmt.Sprintf(cimfs.VolumePathFormat, wcowBlockCimMounts.VolumeGUID.String())
err := cimfs.Unmount(volumePath)

if len(layerCIMs) > 1 {
_, err = cimfs.MountMergedVerifiedBlockCIMs(layerCIMs[0], layerCIMs[1:], wcowBlockCimMounts.MountFlags, wcowBlockCimMounts.VolumeGUID, layerDigests[0])
if err != nil {
return fmt.Errorf("error mounting multilayer block cims: %w", err)
}
} else {
_, err = cimfs.MountVerifiedBlockCIM(layerCIMs[0], wcowBlockCimMounts.MountFlags, wcowBlockCimMounts.VolumeGUID, layerDigests[0])
if err != nil {
return fmt.Errorf("error mounting verified block cim: %w", err)
return fmt.Errorf("error unmounting block cim: %w", err)
}
}

// Send response back to shim
resp := &prot.ResponseBase{
Result: 0, // 0 means success
Expand All @@ -645,56 +654,6 @@ func (b *Bridge) modifySettings(req *request) (err error) {
}
return nil

case guestresource.ResourceTypeCWCOWCombinedLayers:
settings := modifyGuestSettingsRequest.Settings.(*guestresource.CWCOWCombinedLayers)
switch modifyGuestSettingsRequest.RequestType {
case guestrequest.RequestTypeAdd:
containerID := settings.ContainerID
log.G(ctx).Tracef("CWCOWCombinedLayers:: ContainerID: %v, ContainerRootPath: %v, Layers: %v, ScratchPath: %v",
containerID, settings.CombinedLayers.ContainerRootPath, settings.CombinedLayers.Layers, settings.CombinedLayers.ScratchPath)

//Since unencrypted scratch is not an option, always pass true
if err := b.hostState.securityOptions.PolicyEnforcer.EnforceScratchMountPolicy(ctx, settings.CombinedLayers.ContainerRootPath, true); err != nil {
return fmt.Errorf("scratch mounting denied by policy: %w", err)
}
// The following two folders are expected to be present in the scratch.
// But since we have just formatted the scratch we would need to
// create them manually.
sandboxStateDirectory := filepath.Join(settings.CombinedLayers.ContainerRootPath, sandboxStateDirName)
err = os.Mkdir(sandboxStateDirectory, 0777)
if err != nil {
return fmt.Errorf("failed to create sandboxStateDirectory: %w", err)
}

hivesDirectory := filepath.Join(settings.CombinedLayers.ContainerRootPath, hivesDirName)
err = os.Mkdir(hivesDirectory, 0777)
if err != nil {
return fmt.Errorf("failed to create hivesDirectory: %w", err)
}

case guestrequest.RequestTypeRemove:
log.G(ctx).Tracef("CWCOWCombinedLayers: Remove")
if err := b.hostState.securityOptions.PolicyEnforcer.EnforceScratchUnmountPolicy(ctx, settings.CombinedLayers.ContainerRootPath); err != nil {
return fmt.Errorf("scratch unmounting denied by policy: %w", err)
}
}

// Reconstruct WCOWCombinedLayers{} req before forwarding to GCS
// as GCS does not understand ResourceTypeCWCOWCombinedLayers
modifyGuestSettingsRequest.ResourceType = guestresource.ResourceTypeCombinedLayers
modifyGuestSettingsRequest.Settings = settings.CombinedLayers
modifyRequest.Request = modifyGuestSettingsRequest
buf, err := json.Marshal(modifyRequest)
if err != nil {
return fmt.Errorf("failed to marshal rpcModifySettings: %w", err)
}
var newRequest request
newRequest.ctx = req.ctx
newRequest.header = req.header
newRequest.header.Size = uint32(len(buf)) + prot.HdrSize
newRequest.message = buf
req = &newRequest

case guestresource.ResourceTypeMappedVirtualDiskForContainerScratch:
wcowMappedVirtualDisk := modifyGuestSettingsRequest.Settings.(*guestresource.WCOWMappedVirtualDisk)
log.G(ctx).Tracef("ResourceTypeMappedVirtualDiskForContainerScratch: { %v }", wcowMappedVirtualDisk)
Expand Down Expand Up @@ -747,6 +706,55 @@ func (b *Bridge) modifySettings(req *request) (err error) {
newRequest.header.Size = uint32(len(buf)) + prot.HdrSize
newRequest.message = buf
req = &newRequest
case guestresource.ResourceTypeCWCOWCombinedLayers:
settings := modifyGuestSettingsRequest.Settings.(*guestresource.CWCOWCombinedLayers)
switch modifyGuestSettingsRequest.RequestType {
case guestrequest.RequestTypeAdd:
containerID := settings.ContainerID
log.G(ctx).Tracef("CWCOWCombinedLayers:: ContainerID: %v, ContainerRootPath: %v, Layers: %v, ScratchPath: %v",
containerID, settings.CombinedLayers.ContainerRootPath, settings.CombinedLayers.Layers, settings.CombinedLayers.ScratchPath)

//Since unencrypted scratch is not an option, always pass true
if err := b.hostState.securityOptions.PolicyEnforcer.EnforceScratchMountPolicy(ctx, settings.CombinedLayers.ContainerRootPath, true); err != nil {
return fmt.Errorf("scratch mounting denied by policy: %w", err)
}
// The following two folders are expected to be present in the scratch.
// But since we have just formatted the scratch we would need to
// create them manually.
sandboxStateDirectory := filepath.Join(settings.CombinedLayers.ContainerRootPath, sandboxStateDirName)
err = os.Mkdir(sandboxStateDirectory, 0777)
if err != nil {
return fmt.Errorf("failed to create sandboxStateDirectory: %w", err)
}

hivesDirectory := filepath.Join(settings.CombinedLayers.ContainerRootPath, hivesDirName)
err = os.Mkdir(hivesDirectory, 0777)
if err != nil {
return fmt.Errorf("failed to create hivesDirectory: %w", err)
}

case guestrequest.RequestTypeRemove:
log.G(ctx).Tracef("CWCOWCombinedLayers: Remove")
if err := b.hostState.securityOptions.PolicyEnforcer.EnforceScratchUnmountPolicy(ctx, settings.CombinedLayers.ContainerRootPath); err != nil {
return fmt.Errorf("scratch unmounting denied by policy: %w", err)
}
}

// Reconstruct WCOWCombinedLayers{} req before forwarding to GCS
// as GCS does not understand ResourceTypeCWCOWCombinedLayers
modifyGuestSettingsRequest.ResourceType = guestresource.ResourceTypeCombinedLayers
modifyGuestSettingsRequest.Settings = settings.CombinedLayers
modifyRequest.Request = modifyGuestSettingsRequest
buf, err := json.Marshal(modifyRequest)
if err != nil {
return fmt.Errorf("failed to marshal rpcModifySettings: %w", err)
}
var newRequest request
newRequest.ctx = req.ctx
newRequest.header = req.header
newRequest.header.Size = uint32(len(buf)) + prot.HdrSize
newRequest.message = buf
req = &newRequest

default:
// Invalid request
Expand Down
1 change: 0 additions & 1 deletion internal/layers/wcow_mount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,6 @@ func mountHypervIsolatedBlockCIMLayers(ctx context.Context, l *wcowBlockCIMLayer
return nil, nil, fmt.Errorf("failed to add SCSI scratch VHD: %w", err)
}
containerScratchPathInUVM := scsiMount.GuestPath()
rcl.Add(scsiMount)

log.G(ctx).WithFields(logrus.Fields{
"hostPath": hostPath,
Expand Down