[shimV2] refactor scsi device controller to V2 template#2660
Open
rawahars wants to merge 1 commit intomicrosoft:mainfrom
Open
[shimV2] refactor scsi device controller to V2 template#2660rawahars wants to merge 1 commit intomicrosoft:mainfrom
rawahars wants to merge 1 commit intomicrosoft:mainfrom
Conversation
In this commit, we are just ensuring that all the files and format for this device/scsi package conforms with the other controllers for V2 shims. This includes- - `states` encapsulated in states file - `types` encapsulated in types file - package documentation in `doc.go` - Similar comment and error message style Note that in this commit, we have not made changes to the unit test files and hence we can be sure that the refactor has not broken any functional behavior. Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
15c8ba7 to
5228c78
Compare
jterry75
approved these changes
Apr 6, 2026
| @@ -0,0 +1,3 @@ | |||
| //go:build windows | |||
|
|
|||
| package disk | |||
Contributor
There was a problem hiding this comment.
Why do you have this and lcow when they have the same build tags?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is mostly a structural refactor + type renames + logging.
Summary of what was done (from code changes)
A) Reorganized the SCSI code into “template-style” modules
Moved definitions out of the main implementation files into dedicated files:
internal/controller/device/scsi/doc.go(package docs)types.go(interfaces +numLUNsPerController+reservationstruct)scsi.go(old minimal package comment file)internal/controller/device/scsi/disk/types.go(diskType, diskConfig, VM/guest interfaces)state.go(disk lifecycle state enum + String())doc.go(disk lifecycle documentation)disk_lcow.goanddisk_wcow.go(currently only//go:build windows+package disk; effectively placeholders)internal/controller/device/scsi/mount/types.go(mountConfig+ guest interfaces +Equals)state.go(mount lifecycle state enum + String())doc.go(mount lifecycle documentation)Net effect: same concepts, but split into consistent “types/state/doc” layout.
B) Renamed public types
disk.DiskConfig→disk.Configdisk.DiskType*→disk.Type*disk.DiskState*→disk.State*mount.MountConfig→mount.Configmount.MountState*→mount.State*Also Controller’s reservation ID type changed:
Controller.Reserve(...)returnsguid.GUIDinstead ofuuid.UUIDController.MapToGuest/UnmapFromGuest(...)now takeguid.GUIDinstead ofuuid.UUIDreservations map[guid.GUID]*reservationC) Added structured logging + new logfields
Changes are sprinkled through
Controller,disk.Disk, andmount.Mount:internal/logfields/fields.goadds:Controller,LUN,DiskType,PartitionController.Reserve,MapToGuest,UnmapFromGuestnow:disk.Disk.AttachToVM/DetachFromVM/ReservePartitionandmount.Mount.MountToGuest/UnmountFromGuestnow emit debug/trace logs and enrich context.Are there functional changes?
1) Yes: panic → error in
Mount.MountToGuestwhen both guests are nilOld behavior:
MountToGuest(ctx, nil, nil)panicked.New behavior:
both linuxGuest and windowsGuest cannot be nilTestMountToGuest_PanicsWhenBothGuestsNil→TestMountToGuest_ErrorWhenBothGuestsNil).2) Slight semantic tightening in
Disk.MountPartitionToGuestwhen partition wasn’t reservedOld behavior:
d.mounts, it returned:partition %d not found on diskNew behavior: