sandbox/virtcontainers: memory resource hotplug when create container.

When create sandbox, we setup a sandbox of 2048M base memory, and
then hotplug memory that is needed for every new container. And
we change the unit of c.config.Resources.Mem from MiB to Byte in
order to prevent the 4095B < memory < 1MiB from being lost.

Depends-on:github.com/kata-containers/tests#813

Fixes #400

Signed-off-by: Clare Chen <clare.chenhui@huawei.com>
Signed-off-by: Zichang Lin <linzichang@huawei.com>

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
 }
 

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()
 	}
 

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
 		}
 	}
 

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)