Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
hypervisor: don't enforce a minimum memory setting
Browse files Browse the repository at this point in the history
Currently, we enforce a lower limit of 256MB for the defaultMemorySize.

In general case with QEMU, this isn't a major problem, since we'll just
eat the extra page table overhead, and only consume pages when needed.
The memcgroup in the guest should make sure it only utilizes requested
amount, not what is actually available.

However, this becomes very problematic when you use preallocated memory.
In the k8s case, the VMM will get OOM killed very quickly since the host's
memory cgroup (created by kubelet) will limit the entire sandbox to the
requests + pod overhead (on the order of 140 MB). We should allow the administrator
of kata to set a better default value, which should be much closer aligned
with what's used for PodOverhead (in the kube case).

Let's remove the artifical limit in kata, and leave it up to the end
user to pick an appropriate non-default value, if desired.

Fixes: #2987

Signed-off-by: Eric Ernst <[email protected]>
  • Loading branch information
egernst committed Oct 12, 2020
1 parent 4af5ead commit ab7f18d
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 12 deletions.
6 changes: 3 additions & 3 deletions pkg/katautils/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,11 @@ func (h hypervisor) defaultMaxVCPUs() uint32 {
}

func (h hypervisor) defaultMemSz() uint32 {
if h.MemorySize < vc.MinHypervisorMemory {
return defaultMemSize // MiB
if h.MemorySize > 0 {
return h.MemorySize
}

return h.MemorySize
return defaultMemSize
}

func (h hypervisor) defaultMemSlots() uint32 {
Expand Down
3 changes: 0 additions & 3 deletions virtcontainers/hypervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ const (
// Port where the agent will send the logs. Logs are sent through the vsock in cases
// where the hypervisor has no console.sock, i.e firecracker
vSockLogsPort = 1025

// MinHypervisorMemory is the minimum memory required for a VM.
MinHypervisorMemory = 256
)

// In some architectures the maximum number of vCPUs depends on the number of physical cores.
Expand Down
6 changes: 1 addition & 5 deletions virtcontainers/pkg/oci/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,11 +477,7 @@ func addHypervisorMemoryOverrides(ocispec specs.Spec, sbConfig *vc.SandboxConfig
if value, ok := ocispec.Annotations[vcAnnotations.DefaultMemory]; ok {
memorySz, err := strconv.ParseUint(value, 10, 32)
if err != nil {
return fmt.Errorf("Error encountered parsing annotation for default_memory: %v, please specify positive numeric value greater than 8", err)
}

if memorySz < vc.MinHypervisorMemory {
return fmt.Errorf("Memory specified in annotation %s is less than minimum required %d, please specify a larger value", vcAnnotations.DefaultMemory, vc.MinHypervisorMemory)
return fmt.Errorf("Error encountered parsing annotation for default_memory: %v", err)
}

sbConfig.HypervisorConfig.MemorySize = uint32(memorySz)
Expand Down
1 change: 0 additions & 1 deletion virtcontainers/pkg/oci/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,6 @@ func TestAddHypervisorAnnotations(t *testing.T) {
assert.Error(err)

ocispec.Annotations[vcAnnotations.DefaultMaxVCPUs] = "1"
ocispec.Annotations[vcAnnotations.DefaultMemory] = fmt.Sprintf("%d", vc.MinHypervisorMemory+1)
assert.Error(err)
}

Expand Down

0 comments on commit ab7f18d

Please sign in to comment.