hypervisor: refactor VMM overhead into shared library and use in eve-k#5681
hypervisor: refactor VMM overhead into shared library and use in eve-k#5681zedi-pramodh wants to merge 2 commits intolf-edge:masterfrom
Conversation
zedi-pramodh
requested review from
rene,
rouming,
rucoder and
shjala
as code owners
|
Verified domain metrics to see overhead got added.. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5681 +/- ##
==========================================
+ Coverage 19.52% 29.45% +9.92%
==========================================
Files 19 18 -1
Lines 3021 2417 -604
==========================================
+ Hits 590 712 +122
+ Misses 2310 1554 -756
- Partials 121 151 +30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors KVM VMM memory overhead estimation into a shared hypervisor helper and applies the same overhead calculation to the KubeVirt (“eve-k”) backend so that AppInstance memory accounting can include hypervisor overhead.
Changes:
- Extracted VMM overhead calculation helpers from
kvm.gointo a new sharedvmm_overhead.go. - Updated KubeVirt backend to report/track VMM memory overhead (including adding it into AllocatedMB metrics output).
- Removed the duplicated overhead helpers from
kvm.go(KVM continues to call the samevmmOverheadwrapper).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| pkg/pillar/hypervisor/vmm_overhead.go | New shared VMM overhead estimation implementation (RAM/QEMU/MMIO/CPU/base components + global/per-app override priority). |
| pkg/pillar/hypervisor/kvm.go | Removes overhead helper implementations now moved into the shared file. |
| pkg/pillar/hypervisor/kubevirt.go | Adds CountMemOverhead for kubevirt and propagates overhead into domain/pod metrics accounting via stored metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
pkg/pillar/hypervisor/kubevirt.go
Outdated
| overhead, err := vmmOverhead(domainName, config.UUIDandVersion.UUID, | ||
| int64(config.Memory), int64(config.VMMMaxMem), | ||
| int64(config.MaxCpus), int64(config.VCpus), config.IoAdapterList, aa, nil) | ||
| if err != nil { | ||
| logrus.Warnf("CreateReplicaVMIConfig: vmmOverhead failed for %s: %v, using 0", domainName, err) | ||
| overhead = 0 | ||
| } |
pkg/pillar/hypervisor/kubevirt.go
Outdated
| // Add VMM overhead to AllocatedMB so it reflects total host memory | ||
| // consumption (guest RAM + hypervisor overhead), consistent with KVM. | ||
| if vmis, ok := ctx.vmiList[n]; ok && vmis.memOverhead > 0 { | ||
| r.AllocatedMB += uint32(vmis.memOverhead / (1024 * 1024)) |
There was a problem hiding this comment.
valid point, will fix this.
pkg/pillar/hypervisor/kubevirt.go
Outdated
| // overhead so it reflects total host memory consumption, consistent with KVM. | ||
| allocatedMB := uint32(memoryLimits.Value()) / BytesInMegabyte | ||
| if vmis != nil && vmis.memOverhead > 0 { | ||
| allocatedMB += uint32(vmis.memOverhead / uint64(BytesInMegabyte)) |
| } | ||
|
|
||
| // memory allocated by QEMU for its own purposes. | ||
| // statistical analysis did not revile any correlation between |
zedi-pramodh
force-pushed
the
vmm-overhead
branch
from
1103db0 to
438c972
Compare
eriknordmark
left a comment
eriknordmark
left a comment
There was a problem hiding this comment.
The copilot comments seems relevant. Please review them and see what might need to be changed.
Looks like Claude and copilot does not like each other :) let me review and get back. thanks |
|
@zedi-pramodh can you rebase this on master so we can get the eve-k build to work? |
…nto KubeVirt Extract VMM overhead calculation functions from kvm.go into a new shared file vmm_overhead.go (no build tag) so they are available to all hypervisor backends: - vmmOverhead: top-level function respecting global config, per-app override, and automatic estimation priority - estimatedVMMOverhead: combines all overhead components - ramVMMOverhead: 2.5% of domain RAM for page table overhead - qemuVMMOverhead: 20 MB for QEMU binaries/libraries - mmioVMMOverhead: 1% of total MMIO size for GPU/VGA passthrough devices - cpuVMMOverhead: 3 MB per vCPU - undefinedVMMOverhead: 350 MB base overhead for QEMU internal use kvm.go: remove the above functions (now in vmm_overhead.go); CountMemOverhead wrapper is unchanged. kubevirt.go: add CountMemOverhead method on kubevirtContext that delegates to the shared vmmOverhead function, replacing the inherited zero-overhead ctrdContext implementation. Add uuid import. include VMM overhead in DomainMetric.AllocatedMB Previously AllocatedMB in KubeVirt DomainMetric was set to the Kubernetes resource limit (guest RAM only), so reported metrics and device-level AllocatedAppsMB undercounted total host memory consumption by the full VMM overhead. Calculate and store the VMM overhead (via the shared vmmOverhead function) in vmiMetaData when the domain config is created — both for VMI replicasets (CreateReplicaVMIConfig) and pod replicasets (CreateReplicaPodConfig). In GetDomsCPUMem (VMI virt-handler metrics path), add the stored overhead to AllocatedMB in the post-processing loop after all metrics are filled. In getPodMetrics (pod replicaset path), add the stored overhead to AllocatedMB before constructing the DomainMetric. This makes KubeVirt consistent with KVM, where AllocatedMB is read from the cgroup HierarchicalMemoryLimit which naturally includes QEMU overhead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>
NOHYPER runs workloads directly in containers without a VMM/QEMU process, so there is no hypervisor memory overhead to account for. - updatestatus.go: skip CountMemOverhead call when VirtualizationMode is NOHYPER - kubevirt.go: skip vmmOverhead in CreateReplicaVMIConfig and CreateReplicaPodConfig when VirtualizationMode is NOHYPER Store globalConfig on kubevirtContext in Setup() so that CreateReplicaVMIConfig and CreateReplicaPodConfig can pass it to vmmOverhead instead of nil. This ensures the global VmmMemoryLimitInMiB override is respected when calculating VMM overhead during domain config creation, consistent with the CountMemOverhead path. Replace truncating integer division (memOverhead / (1024 * 1024)) with roundFromBytesToMbytes() when adding VMM overhead to AllocatedMB. This avoids under-reporting overhead by up to ~1 MB and is consistent with how other memory conversions are done in the codebase. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>
zedi-pramodh
force-pushed
the
vmm-overhead
branch
from
075f601 to
45af414
Compare
|
Rebased and pushed the commits |
This commit adds VMM Overhead support to eve-k
Extract VMM overhead calculation functions from kvm.go into a new shared file vmm_overhead.go (no build tag) so they are available to all hypervisor backends:
kvm.go: remove the above functions (now in vmm_overhead.go); CountMemOverhead wrapper is unchanged.
kubevirt.go: add CountMemOverhead method on kubevirtContext that delegates to the shared vmmOverhead function, replacing the inherited zero-overhead ctrdContext implementation. Add uuid import.
How to test and validate this PR
Changelog notes
Users can now use VMM overhead value in AppInstance config for eve-k
Checklist
And the last but not least:
check them.
Please, check the boxes above after submitting the PR in interactive mode.