Skip to content

fix(lcow/v2): report physically backed VM stats#2834

Merged
rawahars merged 2 commits into
microsoft:mainfrom
shreyanshjain7174:user/shsancheti/fix-lcowv2-physically-backed-stats
Jul 23, 2026
Merged

fix(lcow/v2): report physically backed VM stats#2834
rawahars merged 2 commits into
microsoft:mainfrom
shreyanshjain7174:user/shsancheti/fix-lcowv2-physically-backed-stats

Conversation

@shreyanshjain7174

@shreyanshjain7174 shreyanshjain7174 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • derive LCOW V2 SandboxOptions.FullyPhysicallyBacked from the final VM memory configuration (!Memory.AllowOvercommit)
  • avoid looking up a vmmem process for physically backed UVMs; their working set comes from HCS AssignedMemory
  • add focused builder and parity coverage for the memory-backing decision

This includes Harsh's 47c5b143 patch, cherry-picked as e3e4cfb6, together with the complementary vm.go lookup gating.

Problem

LCOW V2 physically backed sandbox stats had two related failure modes:

  1. setting AllowOvercommit=false without the stronger FullyPhysicallyBacked annotation left SandboxOptions.FullyPhysicallyBacked false, so the controller selected the VA-backed stats path for physically backed VM memory;
  2. Stats() looked up vmmem before checking the backing mode, even though physically backed UVMs use HCS AssignedMemory. A failed lookup therefore prevented metrics that do not require vmmem.

Implementation

  • parse the FullyPhysicallyBacked annotation separately for device-backing behavior and its existing overcommit override
  • set SandboxOptions.FullyPhysicallyBacked from the finalized memory configuration
  • move LookupVMMEM and process working-set collection inside the VA-backed branch
  • update parity expectations and cover AllowOvercommit=false without the annotation

Validation

  • go test -tags lcow ./internal/builder/vm/lcow/... ./internal/controller/vm/... -count=1
  • go vet -tags lcow ./internal/builder/vm/lcow/... ./internal/controller/vm/...
  • rebuilt containerd-shim-lcow-v2.exe with both changes and ran Test_SandboxStats_WorkingSet_PhysicallyBacked/LCOW against a local build 26200 host: PASS
  • restored the original deployed shim after the functional run and verified its SHA-256 and the running containerd service

@shreyanshjain7174
shreyanshjain7174 requested a review from a team as a code owner July 21, 2026 08:52
Comment thread internal/controller/vm/save_lcow.go
@shreyanshjain7174
shreyanshjain7174 force-pushed the user/shsancheti/fix-lcowv2-physically-backed-stats branch from b1474f8 to 1fc6a13 Compare July 21, 2026 11:00
@rawahars

Copy link
Copy Markdown
Contributor

This PR can be superseded by 47c5b14 which correctly sets the param in SandboxOptions which in turn calculates the stats properly.

@shreyanshjain7174

shreyanshjain7174 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, Harsh. Does 47c5b143 also address the unconditional LookupVMMEM call in Stats()? If not, should this PR keep only that gating change and let your commit own the flag derivation?

@rawahars

Copy link
Copy Markdown
Contributor

@shreyanshjain7174 Yes let's keep your changes in vm.go which move vmmem lookup inside the if check.

Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>
@shreyanshjain7174
shreyanshjain7174 force-pushed the user/shsancheti/fix-lcowv2-physically-backed-stats branch from 1fc6a13 to e3303f0 Compare July 23, 2026 10:00
In the v2 LCOW builder, SandboxOptions.FullyPhysicallyBacked was taken
straight from the FullyPhysicallyBacked annotation. It feeds
isPhysicallyBacked, which only drives VM memory/working-set stats, so
setting AllowOvercommit=false alone left it false and a physically backed
VM had its stats computed as if VA-backed.

Set it to !AllowOvercommit so it reflects the VM's actual memory backing;
the annotation still forces overcommit off. v2 has no VSMB/VPMem, so there
is no separate devices-physically-backed notion.

Also update the parity check to compare against !AllowOvercommit and add a
regression test for overcommit-off alone.

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
@rawahars
rawahars merged commit 6d9c12a into microsoft:main Jul 23, 2026
20 checks passed
@shreyanshjain7174
shreyanshjain7174 deleted the user/shsancheti/fix-lcowv2-physically-backed-stats branch July 23, 2026 11:22
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.

2 participants