Skip to content

Commit

Permalink
pillar: Refactor CountMemOverhead to use explicit parameters instead …
Browse files Browse the repository at this point in the history
…of DomainConfig.

In this commit, the CountMemOverhead function across various hypervisors
(containerd, kvm, null) is refactored. The change involves passing individual
parameters (like domain UUID, memory size, CPU counts, etc.) directly to the
function, rather than encapsulating them within a DomainConfig object. This
approach eliminates the need for creating a mock DomainConfig just to calculate
memory overhead, leading to cleaner and more explicit code. The changes are
applied consistently across the relevant files in the zedmanager and hypervisor
packages.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
  • Loading branch information
OhmSpectator committed Nov 17, 2023
1 parent 5f5a124 commit a41e382
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 24 deletions.
16 changes: 4 additions & 12 deletions pkg/pillar/cmd/zedmanager/updatestatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,18 +625,10 @@ func doActivate(ctx *zedmanagerContext, uuidStr string,
log.Fatalf("Cannot get hypervisor: %s", err)
}

// Create a mock domain config to calculate memory overhead
mockDomainConfig := types.DomainConfig{
VmConfig: types.VmConfig{
Memory: config.FixedResources.Memory,
MaxCpus: config.FixedResources.MaxCpus,
VCpus: config.FixedResources.VCpus,
},
IoAdapterList: config.IoAdapterList,
UUIDandVersion: config.UUIDandVersion,
}

status.MemOverhead, err = hyp.CountMemOverhead(status.DomainName, &mockDomainConfig, ctx.globalConfig, ctx.assignableAdapters)
status.MemOverhead, err = hyp.CountMemOverhead(status.DomainName, config.UUIDandVersion.UUID,
int64(config.FixedResources.Memory), int64(config.FixedResources.VMMMaxMem),
int64(config.FixedResources.MaxCpus), int64(config.FixedResources.VCpus), config.IoAdapterList,
ctx.assignableAdapters, ctx.globalConfig)
// We have to publish the status here, because we need to save the memory overhead value, it's used in getRemainingMemory
publishAppInstanceStatus(ctx, status)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/pillar/hypervisor/containerd.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"path/filepath"
"time"

uuid "github.com/satori/go.uuid"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -51,7 +52,9 @@ func newContainerd() Hypervisor {

// CountMemOverhead - returns the memory overhead for a domain.
// This implementation is used for Xen as well
func (ctx ctrdContext) CountMemOverhead(domainName string, config *types.DomainConfig, globalConfig *types.ConfigItemValueMap, aa *types.AssignableAdapters) (uint64, error) {
func (ctx ctrdContext) CountMemOverhead(domainName string, domainUUID uuid.UUID, domainRAMMemory int64, vmmMaxMem int64,
domainMaxCpus int64, domainVCpus int64, domainIoAdapterList []types.IoAdapter, aa *types.AssignableAdapters,
globalConfig *types.ConfigItemValueMap) (uint64, error) {
// Does containerd have any overhead?
return 0, nil
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/pillar/hypervisor/hypervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package hypervisor
import (
"fmt"
"github.com/lf-edge/eve/pkg/pillar/types"
uuid "github.com/satori/go.uuid"
"github.com/shirou/gopsutil/cpu"
"github.com/shirou/gopsutil/mem"
"github.com/sirupsen/logrus"
Expand All @@ -26,7 +27,9 @@ type Hypervisor interface {

GetCapabilities() (*types.Capabilities, error)

CountMemOverhead(domainName string, config *types.DomainConfig, globalConfig *types.ConfigItemValueMap, aa *types.AssignableAdapters) (uint64, error)
CountMemOverhead(domainName string, domainUUID uuid.UUID, domainRAMSize int64, vmmMaxMem int64,
domainMaxCpus int64, domainVCpus int64, domainIoAdapterList []types.IoAdapter, aa *types.AssignableAdapters,
globalConfig *types.ConfigItemValueMap) (uint64, error)
}

type hypervisorDesc struct {
Expand Down
21 changes: 12 additions & 9 deletions pkg/pillar/hypervisor/kvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/lf-edge/eve/pkg/pillar/agentlog"
"github.com/lf-edge/eve/pkg/pillar/containerd"
"github.com/lf-edge/eve/pkg/pillar/types"
uuid "github.com/satori/go.uuid"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -436,8 +437,11 @@ func (ctx kvmContext) GetCapabilities() (*types.Capabilities, error) {
return ctx.capabilities, nil
}

func (ctx kvmContext) CountMemOverhead(domainName string, config *types.DomainConfig, globalConfig *types.ConfigItemValueMap, aa *types.AssignableAdapters) (uint64, error) {
result, err := vmmOverhead(*config, globalConfig)
// CountMemOverhead - returns the memory overhead estimation for a domain.
func (ctx kvmContext) CountMemOverhead(domainName string, domainUUID uuid.UUID, domainRAMSize int64, vmmMaxMem int64,
domainMaxCpus int64, domainVCpus int64, domainIoAdapterList []types.IoAdapter, aa *types.AssignableAdapters,
globalConfig *types.ConfigItemValueMap) (uint64, error) {
result, err := vmmOverhead(domainName, domainUUID, domainRAMSize, vmmMaxMem, domainMaxCpus, domainVCpus, domainIoAdapterList, aa, globalConfig)
return uint64(result), err
}

Expand Down Expand Up @@ -469,16 +473,15 @@ func (ctx kvmContext) Task(status *types.DomainStatus) types.Task {

// TBD: Have a better way to calculate this number.
// For now it is based on some trial-and-error experiments.
func minVMMOverhead(config types.DomainConfig) int64 {
func minVMMOverhead() int64 {
return 600 << 20 // Mb in bytes
}

func vmmOverhead(config types.DomainConfig,
globalConfig *types.ConfigItemValueMap) (int64, error) {
func vmmOverhead(domainName string, domainUUID uuid.UUID, domainRAMSize int64, vmmMaxMem int64, domainMaxCpus int64, domainVCpus int64, domainIoAdapterList []types.IoAdapter, aa *types.AssignableAdapters, globalConfig *types.ConfigItemValueMap) (int64, error) {
var overhead int64

// Fetch VMM max memory setting (aka vmm overhead)
overhead = int64(config.VMMMaxMem) << 10
overhead = vmmMaxMem << 10

// Global node setting has a higher priority
if globalConfig != nil {
Expand All @@ -493,8 +496,8 @@ func vmmOverhead(config types.DomainConfig,
if overhead == 0 {
// XXX: This looks ugly and probably incorrect, please FIXME
// Default formula - 2.5 % of total memory
overhead = int64(config.Memory) * 1024 * 25 / 1000
minOverhead := minVMMOverhead(config)
overhead = domainRAMSize * 1024 * 25 / 1000
minOverhead := minVMMOverhead()
if overhead < minOverhead {
overhead = minOverhead
}
Expand Down Expand Up @@ -542,7 +545,7 @@ func (ctx kvmContext) Setup(status types.DomainStatus, config types.DomainConfig
if err = spec.AddLoader("/containers/services/xen-tools"); err != nil {
return logError("failed to add kvm hypervisor loader to domain %s: %v", status.DomainName, err)
}
overhead, err := vmmOverhead(config, globalConfig)
overhead, err := vmmOverhead(domainName, domainUUID, int64(config.Memory), int64(config.VMMMaxMem), int64(config.MaxCpus), int64(config.VCpus), config.IoAdapterList, aa, globalConfig)
if err != nil {
return logError("vmmOverhead() failed for domain %s: %v",
status.DomainName, err)
Expand Down
5 changes: 4 additions & 1 deletion pkg/pillar/hypervisor/null.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/lf-edge/eve/pkg/pillar/types"
"os"

uuid "github.com/satori/go.uuid"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -39,7 +40,9 @@ func (ctx nullContext) GetCapabilities() (*types.Capabilities, error) {

// CountMemOverhead - returns the memory overhead for a domain.
// Null-implementation that returns 0 is used now for Acrn hypervisor
func (ctx nullContext) CountMemOverhead(domainName string, config *types.DomainConfig, globalConfig *types.ConfigItemValueMap, aa *types.AssignableAdapters) (uint64, error) {
func (ctx nullContext) CountMemOverhead(domainName string, domainUUID uuid.UUID, domainRAMSize int64, vmmMaxMem int64,
domainMaxCpus int64, domainVCpus int64, domainIoAdapterList []types.IoAdapter, aa *types.AssignableAdapters,
globalConfig *types.ConfigItemValueMap) (uint64, error) {
return 0, nil
}

Expand Down

0 comments on commit a41e382

Please sign in to comment.