hypervisor: refactor VMM overhead into shared library and use in eve-k#5681
Open
zedi-pramodh wants to merge 2 commits intolf-edge:masterfrom
Open
hypervisor: refactor VMM overhead into shared library and use in eve-k#5681zedi-pramodh wants to merge 2 commits intolf-edge:masterfrom
zedi-pramodh wants to merge 2 commits intolf-edge:masterfrom
Conversation
…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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>
zedi-pramodh
requested review from
rene,
rouming,
rucoder and
shjala
as code owners
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>
Author
|
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.49% +9.96%
==========================================
Files 19 18 -1
Lines 3021 2417 -604
==========================================
+ Hits 590 713 +123
+ Misses 2310 1552 -758
- Partials 121 152 +31 ☔ 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.
Comment on lines
+199
to
+200
| result, err := vmmOverhead(domainName, domainUUID, domainRAMSize, vmmMaxMem, domainMaxCpus, domainVCpus, domainIoAdapterList, aa, globalConfig) | ||
| return uint64(result), err |
Comment on lines
+515
to
+521
| 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 | ||
| } |
Comment on lines
+1439
to
+1445
| 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("CreateReplicaPodConfig: vmmOverhead failed for %s: %v, using 0", domainName, err) | ||
| overhead = 0 | ||
| } |
| // 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)) |
| // 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)) |
| overhead = vmmMaxMem << 10 | ||
|
|
||
| // Global node setting has a higher priority. | ||
| // Note: globalConfig can be nil only in unit tests. |
| } | ||
|
|
||
| // memory allocated by QEMU for its own purposes. | ||
| // statistical analysis did not revile any correlation between |
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 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.