Skip to content

[shimV2] Refactor V2 shim for platform consistency and remove vPMem support in v2 LCOW#2701

Merged
rawahars merged 6 commits intomicrosoft:mainfrom
rawahars:fix_v2_shim
Apr 23, 2026
Merged

[shimV2] Refactor V2 shim for platform consistency and remove vPMem support in v2 LCOW#2701
rawahars merged 6 commits intomicrosoft:mainfrom
rawahars:fix_v2_shim

Conversation

@rawahars
Copy link
Copy Markdown
Contributor

Overview

This PR focuses on standardizing the guest manager and controller APIs across LCOW and WCOW platforms, making internal interfaces cleaner, more idiomatic, and platform-neutral. Additionally, it officially removes support for Virtual PMem (vPMem) devices within the v2 LCOW shim.

Key Themes

  • API Unification & Code Cleanup
    • Removal of Redundant OS Prefixes: Guest manager methods (e.g., adding/removing virtual disks, combined layers, network interfaces, and mapped directories) have been renamed to their unprefixed forms. Since platform separation is already strictly managed via build tags, dropping the LCOW and WCOW prefixes provides a consistent, platform-agnostic API for controller interfaces. This is a pure refactor with no behavioral changes.
    • vPCI Controller Abstraction: The vPCI controller has been refactored to make its guest-side interface build-specific rather than LCOW-exclusive. It now cleanly delegates to OS-specific implementations under the hood, streamlining the shared architecture.
  • Deprecation of Virtual PMem (vPMem) Support
    • Virtual PMem devices are no longer supported in the v2 LCOW shim. The logic has been updated to explicitly reject any requests for a non-zero vPMem count. All associated device parsing, kernel argument construction, and emissions to the compute system document have been safely removed.
  • Strict Build Tag Enforcement
    • Build constraints have been thoroughly updated and applied across the V2 shim components (including guest managers, VM managers, and shared controller logic) to guarantee precise, platform-specific compilation for Windows, LCOW, and WCOW targets. Test files and mocks have been appropriately renamed and scoped to match these build constraints.

@rawahars rawahars requested a review from a team as a code owner April 23, 2026 08:25
@rawahars rawahars linked an issue Apr 23, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@shreyanshjain7174 shreyanshjain7174 left a comment

Choose a reason for hiding this comment

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

LGTM — clean refactor. The method renames, vPMem removal logic, and build tag enforcement all look correct. Verified no stale callers of old names remain in production code.

Two minor nits left as inline comments (a copy-paste doc comment and a stale error message string). Neither blocks merge.

Comment thread internal/controller/device/vpci/vpci_wcow.go Outdated
Comment thread internal/controller/device/scsi/mount/mount_wcow_test.go
Copy link
Copy Markdown
Contributor

@shreyanshjain7174 shreyanshjain7174 left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread internal/builder/vm/lcow/devices.go Outdated
Comment thread internal/builder/vm/lcow/devices.go Outdated
Refactor the vpci controller to make the guest-side interface
build-specific instead of LCOW-only on the shared type:

- Move the guest vPCI interface out of types.go and define a
  per-build guestVPCI interface in vpci_lcow.go (with AddVPCIDevice)
  and vpci_wcow.go (empty, no-op).
- Rename Controller field linuxGuestVPCI -> guestVPCI and update
  New() accordingly.
- Update build tags on shared files to `windows && (lcow || wcow)`.
- Rename LCOW-specific test/mock files to reflect their scope:
    mocks/mock_vpci.go  -> mocks/mock_lcow_vpci.go
    vpci_test.go        -> vpci_lcow_test.go
- Remove vpci_unsupported.go; coverage is now provided by the
  per-OS build files.

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
The guestmanager.Guest methods and the controller-side interfaces that
consume them are already split into LCOW- and WCOW-specific files via
build tags, so the OS prefix on method names (AddLCOWMappedVirtualDisk,
AddWCOWMappedVirtualDisk, AddLCOWCombinedLayers, AddLCOWNetworkInterface,
AddWCOWBlockCIMs, AddLCOWMappedDirectory, etc.) is redundant and made
the controller interfaces diverge needlessly between platforms.

Rename the methods to their unprefixed forms (AddMappedVirtualDisk,
AddMappedVirtualDiskForContainerScratch, RemoveMappedVirtualDisk,
AddCombinedLayers / RemoveCombinedLayers, AddNetworkInterface /
RemoveNetworkInterface, AddBlockCIMs / RemoveBlockCIMs, AddMappedDirectory
/ RemoveMappedDirectory) across:

  - internal/vm/guestmanager (scsi, combinedlayers, network, block_cims,
    mapped_directory, doc)
  - internal/controller/device/scsi (mount + tests, controller tests,
    disk tests)
  - internal/controller/device/plan9 (mount + types + tests, share tests,
    controller tests, generated mocks)
  - internal/controller/linuxcontainer (container, layers, types and
    generated mocks + tests)
  - internal/controller/network (network_lcow guestNetwork interface)

No behavioral change; pure rename to give the LCOW and WCOW guest
surfaces a consistent, platform-neutral API.

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
@rawahars rawahars force-pushed the fix_v2_shim branch 2 times, most recently from 4cfdf85 to 502d831 Compare April 23, 2026 17:59
Comment thread internal/controller/device/scsi/disk/disk_lcow_test.go
Comment thread test/pkg/uvm/wcow.go
//
// Deprecated: use [CreateWCOW] and [layers.WCOWScratchDir].
//
//nolint:staticcheck // SA5011: staticcheck thinks `opts` may be nil, even though we fail if it is
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Youll have to leave this. While correct, it doesnt know that t.Fatal panics here so you couldnt hit the 2nd case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed this now.

…nt tests

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
- Gate lcow parity tests behind the `lcow` build tag and run them in a
  separate CI step so the default non-functional test job no longer
  pulls them in.
- Run non-functional tests from the `test` working directory to match
  the new parity job layout.
- Fix malformed build constraint in internal/controller/device/vpci/utils.go
  (`windows && (lcow  wcow)`) by simplifying it to `windows` as its used by old shim.

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
@rawahars rawahars merged commit a6af825 into microsoft:main Apr 23, 2026
19 checks passed
@rawahars rawahars deleted the fix_v2_shim branch April 23, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[shimV2] Minor refactoring changes

5 participants