diff --git a/virtcontainers/api.go b/virtcontainers/api.go index 3573b3707..78d527f76 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -112,6 +112,10 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f } }() + if err := s.getAndStoreGuestDetails(); err != nil { + return nil, err + } + // Create Containers if err = s.createContainers(); err != nil { return nil, err @@ -122,11 +126,6 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f return nil, err } - // get and store guest details - if err := s.getAndStoreGuestDetails(); err != nil { - return nil, err - } - return s, nil } diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 96e853eb5..4073503c2 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -178,7 +178,7 @@ type ContainerResources struct { VCPUs uint32 // Mem is the memory that is being used by the container - MemMB uint32 + MemByte int64 } // ContainerConfig describes one container runtime configuration. @@ -984,7 +984,9 @@ func (c *Container) update(resources specs.LinuxResources) error { newResources := ContainerResources{ VCPUs: uint32(utils.ConstraintsToVCPUs(*resources.CPU.Quota, *resources.CPU.Period)), - MemMB: uint32(*resources.Memory.Limit >> 20), + // do page align to memory, as cgroup memory.limit_in_bytes will be aligned to page when effect. + // TODO use GetGuestDetails to get the guest OS page size. + MemByte: (*resources.Memory.Limit >> 12) << 12, } if err := c.updateResources(currentConfig.Resources, newResources); err != nil { @@ -1181,7 +1183,6 @@ func (c *Container) detachDevices() error { } func (c *Container) addResources() error { - //TODO add support for memory, Issue: https://github.com/containers/virtcontainers/issues/578 if c.config == nil { return nil } @@ -1189,7 +1190,7 @@ func (c *Container) addResources() error { // Container is being created, try to add the number of vCPUs specified vCPUs := c.config.Resources.VCPUs if vCPUs != 0 { - virtLog.Debugf("hot adding %d vCPUs", vCPUs) + virtLog.Debugf("create container: hot adding %d vCPUs", vCPUs) data, err := c.sandbox.hypervisor.hotplugAddDevice(vCPUs, cpuDev) if err != nil { return err @@ -1210,14 +1211,32 @@ func (c *Container) addResources() error { } } - return c.sandbox.agent.onlineCPUMem(vcpusAdded, true) + if err := c.sandbox.agent.onlineCPUMem(vcpusAdded, true); err != nil { + return err + } + } + + // try to add the number of Mem specified + addMemByte := c.config.Resources.MemByte + if addMemByte != 0 { + memHotplugMB, err := c.calcHotplugMemMiBSize(addMemByte) + if err != nil { + return err + } + virtLog.Debugf("create container: hotplug %dMB mem", memHotplugMB) + _, err = c.sandbox.hypervisor.hotplugAddDevice(&memoryDevice{sizeMB: int(memHotplugMB)}, memoryDev) + if err != nil { + return err + } + if err := c.sandbox.agent.onlineCPUMem(0, false); err != nil { + return err + } } return nil } func (c *Container) removeResources() error { - //TODO add support for memory, Issue: https://github.com/containers/virtcontainers/issues/578 if c.config == nil { return nil } @@ -1237,6 +1256,7 @@ func (c *Container) removeResources() error { return err } } + // hot remove memory unsupported return nil } @@ -1259,7 +1279,7 @@ func (c *Container) updateVCPUResources(oldResources, newResources ContainerReso if oldVCPUs < newVCPUs { // hot add vCPUs vCPUs = newVCPUs - oldVCPUs - virtLog.Debugf("hot adding %d vCPUs", vCPUs) + virtLog.Debugf("update container: hot adding %d vCPUs", vCPUs) data, err := c.sandbox.hypervisor.hotplugAddDevice(vCPUs, cpuDev) if err != nil { return err @@ -1291,36 +1311,37 @@ func (c *Container) updateVCPUResources(oldResources, newResources ContainerReso return nil } -func (c *Container) memHotplugValid(mem uint32) (uint32, error) { - memorySectionSizeMB := c.sandbox.state.GuestMemoryBlockSizeMB - if memorySectionSizeMB == 0 { - return mem, nil +// calculate hotplug memory size with memory block size of guestos +func (c *Container) calcHotplugMemMiBSize(memByte int64) (uint32, error) { + memoryBlockSize := int64(c.sandbox.state.GuestMemoryBlockSizeMB) + if memoryBlockSize == 0 { + return uint32(memByte >> 20), nil } // TODO: hot add memory aligned to memory section should be more properly. See https://github.com/kata-containers/runtime/pull/624#issuecomment-419656853 - return uint32(math.Ceil(float64(mem)/float64(memorySectionSizeMB))) * memorySectionSizeMB, nil + return uint32(int64(math.Ceil(float64(memByte)/float64(memoryBlockSize<<20))) * memoryBlockSize), nil } func (c *Container) updateMemoryResources(oldResources ContainerResources, newResources *ContainerResources) error { - oldMemMB := oldResources.MemMB - newMemMB := newResources.MemMB + oldMemByte := oldResources.MemByte + newMemByte := newResources.MemByte c.Logger().WithFields(logrus.Fields{ - "old-mem": fmt.Sprintf("%dMB", oldMemMB), - "new-mem": fmt.Sprintf("%dMB", newMemMB), + "old-mem": fmt.Sprintf("%dByte", oldMemByte), + "new-mem": fmt.Sprintf("%dByte", newMemByte), }).Debug("Request update memory") - if oldMemMB == newMemMB { + if oldMemByte == newMemByte { c.Logger().WithFields(logrus.Fields{ - "old-mem": fmt.Sprintf("%dMB", oldMemMB), - "new-mem": fmt.Sprintf("%dMB", newMemMB), + "old-mem": fmt.Sprintf("%dByte", oldMemByte), + "new-mem": fmt.Sprintf("%dByte", newMemByte), }).Debug("the actual number of Mem will not be modified") return nil } - if oldMemMB < newMemMB { + if oldMemByte < newMemByte { // hot add memory - addMemMB := newMemMB - oldMemMB - memHotplugMB, err := c.memHotplugValid(addMemMB) + addMemByte := newMemByte - oldMemByte + memHotplugMB, err := c.calcHotplugMemMiBSize(addMemByte) if err != nil { return err } @@ -1337,16 +1358,16 @@ func (c *Container) updateMemoryResources(oldResources ContainerResources, newRe if !ok { return fmt.Errorf("Could not get the memory added, got %+v", data) } - newResources.MemMB = oldMemMB + uint32(memoryAdded) + newResources.MemByte = oldMemByte + int64(memoryAdded)<<20 if err := c.sandbox.agent.onlineCPUMem(0, false); err != nil { return err } } - if oldMemMB > newMemMB { + if oldMemByte > newMemByte { // Try to remove a memory device with the difference // from new memory and old memory removeMem := &memoryDevice{ - sizeMB: int(oldMemMB - newMemMB), + sizeMB: int((oldMemByte - newMemByte) >> 20), } data, err := c.sandbox.hypervisor.hotplugRemoveDevice(removeMem, memoryDev) @@ -1357,8 +1378,7 @@ func (c *Container) updateMemoryResources(oldResources ContainerResources, newRe if !ok { return fmt.Errorf("Could not get the memory added, got %+v", data) } - newResources.MemMB = oldMemMB - uint32(memoryRemoved) - newResources.MemMB = oldResources.MemMB + newResources.MemByte = oldMemByte - int64(memoryRemoved)<<20 } return nil } @@ -1366,7 +1386,7 @@ func (c *Container) updateMemoryResources(oldResources ContainerResources, newRe func (c *Container) updateResources(oldResources, newResources ContainerResources) error { // initialize with oldResources c.config.Resources.VCPUs = oldResources.VCPUs - c.config.Resources.MemMB = oldResources.MemMB + c.config.Resources.MemByte = oldResources.MemByte // Cpu is not updated if period and/or quota not set if newResources.VCPUs != 0 { @@ -1382,13 +1402,13 @@ func (c *Container) updateResources(oldResources, newResources ContainerResource } // Memory is not updated if memory limit not set - if newResources.MemMB != 0 { + if newResources.MemByte != 0 { if err := c.updateMemoryResources(oldResources, &newResources); err != nil { return err } - // Set and save container's config MemMB field only - c.config.Resources.MemMB = newResources.MemMB + // Set and save container's config Mem field only + c.config.Resources.MemByte = newResources.MemByte return c.storeContainer() } diff --git a/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index 7978be9cd..e7c37a617 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/virtcontainers/pkg/oci/utils.go @@ -412,25 +412,6 @@ func (spec *CompatOCISpec) SandboxID() (string, error) { return "", fmt.Errorf("Could not find sandbox ID") } -func updateVMConfig(ocispec CompatOCISpec, config *RuntimeConfig) error { - if ocispec.Linux == nil || ocispec.Linux.Resources == nil { - return nil - } - - if ocispec.Linux.Resources.Memory != nil && - ocispec.Linux.Resources.Memory.Limit != nil { - memBytes := *ocispec.Linux.Resources.Memory.Limit - if memBytes <= 0 { - return fmt.Errorf("Invalid OCI memory limit %d", memBytes) - } - // Use some math magic to round up to the nearest Mb. - // This has the side effect that we can never have <1Mb assigned. - config.HypervisorConfig.MemorySize = uint32((memBytes + (1024*1024 - 1)) / (1024 * 1024)) - } - - return nil -} - func addAssetAnnotations(ocispec CompatOCISpec, config *vc.SandboxConfig) { assetAnnotations := []string{ vcAnnotations.KernelPath, @@ -469,11 +450,6 @@ func SandboxConfig(ocispec CompatOCISpec, runtime RuntimeConfig, bundlePath, cid return vc.SandboxConfig{}, err } - err = updateVMConfig(ocispec, &runtime) - if err != nil { - return vc.SandboxConfig{}, err - } - ociSpecJSON, err := json.Marshal(ocispec) if err != nil { return vc.SandboxConfig{}, err @@ -570,7 +546,8 @@ func ContainerConfig(ocispec CompatOCISpec, bundlePath, cid, console string, det } if ocispec.Linux.Resources.Memory != nil { if ocispec.Linux.Resources.Memory.Limit != nil { - resources.MemMB = uint32(*ocispec.Linux.Resources.Memory.Limit >> 20) + // do page align to memory, as cgroup memory.limit_in_bytes will be aligned to page when effect + resources.MemByte = (*ocispec.Linux.Resources.Memory.Limit >> 12) << 12 } } diff --git a/virtcontainers/pkg/oci/utils_test.go b/virtcontainers/pkg/oci/utils_test.go index 042a02f49..721d827c0 100644 --- a/virtcontainers/pkg/oci/utils_test.go +++ b/virtcontainers/pkg/oci/utils_test.go @@ -254,54 +254,6 @@ func TestMinimalSandboxConfig(t *testing.T) { } } -func TestUpdateVmConfig(t *testing.T) { - var limitBytes int64 = 128 * 1024 * 1024 - assert := assert.New(t) - - config := RuntimeConfig{ - HypervisorConfig: vc.HypervisorConfig{ - MemorySize: 2048, - }, - } - - expectedMem := uint32(128) - - ocispec := CompatOCISpec{ - Spec: specs.Spec{ - Linux: &specs.Linux{ - Resources: &specs.LinuxResources{ - Memory: &specs.LinuxMemory{ - Limit: &limitBytes, - }, - }, - }, - }, - } - - err := updateVMConfig(ocispec, &config) - assert.Nil(err) - assert.Equal(config.HypervisorConfig.MemorySize, expectedMem) - - limitBytes = -128 * 1024 * 1024 - ocispec.Linux.Resources.Memory.Limit = &limitBytes - - err = updateVMConfig(ocispec, &config) - assert.NotNil(err) - - // Test case when Memory is nil - ocispec.Spec.Linux.Resources.Memory = nil - err = updateVMConfig(ocispec, &config) - assert.Nil(err) - - // Test case when CPU is nil - ocispec.Spec.Linux.Resources.CPU = nil - limitBytes = 20 - ocispec.Linux.Resources.Memory = &specs.LinuxMemory{Limit: &limitBytes} - err = updateVMConfig(ocispec, &config) - assert.Nil(err) - assert.NotEqual(config.HypervisorConfig.MemorySize, expectedMem) -} - func testStatusToOCIStateSuccessful(t *testing.T, cStatus vc.ContainerStatus, expected specs.State) { ociState := StatusToOCIState(cStatus)