Merge pull request #296 from devimc/cpu/fixMemFootprint

virtcontainers/qemu: reduce memory footprint

Makefile

@@ -92,6 +92,8 @@ PROXYPATH := $(PKGLIBEXECDIR)/$(PROXYCMD)
 
 # Default number of vCPUs
 DEFVCPUS := 1
+# Default maximum number of vCPUs
+DEFMAXVCPUS := 0
 # Default memory size in MiB
 DEFMEMSZ := 2048
 #Default number of bridges
@@ -169,6 +171,7 @@ USER_VARS += SHAREDIR
 USER_VARS += SHIMPATH
 USER_VARS += SYSCONFDIR
 USER_VARS += DEFVCPUS
+USER_VARS += DEFMAXVCPUS
 USER_VARS += DEFMEMSZ
 USER_VARS += DEFBRIDGES
 USER_VARS += DEFNETWORKMODEL
@@ -262,6 +265,7 @@ const defaultMachineType = "$(MACHINETYPE)"
 const defaultRootDirectory = "$(PKGRUNDIR)"
 
 const defaultVCPUCount uint32 = $(DEFVCPUS)
+const defaultMaxVCPUCount uint32 = $(DEFMAXVCPUS)
 const defaultMemSize uint32 = $(DEFMEMSZ) // MiB
 const defaultBridgesCount uint32 = $(DEFBRIDGES)
 const defaultInterNetworkingModel = "$(DEFNETWORKMODEL)"
@@ -347,6 +351,7 @@ $(GENERATED_FILES): %: %.in Makefile VERSION
 		-e "s|@MACHINETYPE@|$(MACHINETYPE)|g" \
 		-e "s|@SHIMPATH@|$(SHIMPATH)|g" \
 		-e "s|@DEFVCPUS@|$(DEFVCPUS)|g" \
+		-e "s|@DEFMAXVCPUS@|$(DEFMAXVCPUS)|g" \
 		-e "s|@DEFMEMSZ@|$(DEFMEMSZ)|g" \
 		-e "s|@DEFBRIDGES@|$(DEFBRIDGES)|g" \
 		-e "s|@DEFNETWORKMODEL@|$(DEFNETWORKMODEL)|g" \

cli/config.go

@@ -74,6 +74,7 @@ type hypervisor struct {
 	KernelParams          string `toml:"kernel_params"`
 	MachineType           string `toml:"machine_type"`
 	DefaultVCPUs          int32  `toml:"default_vcpus"`
+	DefaultMaxVCPUs       uint32 `toml:"default_maxvcpus"`
 	DefaultMemSz          uint32 `toml:"default_memory"`
 	DefaultBridges        uint32 `toml:"default_bridges"`
 	Msize9p               uint32 `toml:"msize_9p"`
@@ -202,6 +203,25 @@ func (h hypervisor) defaultVCPUs() uint32 {
 	return uint32(h.DefaultVCPUs)
 }
 
+func (h hypervisor) defaultMaxVCPUs() uint32 {
+	numcpus := uint32(goruntime.NumCPU())
+	maxvcpus := vc.MaxQemuVCPUs()
+	reqVCPUs := h.DefaultMaxVCPUs
+
+	//don't exceed the number of physical CPUs. If a default is not provided, use the
+	// numbers of physical CPUs
+	if reqVCPUs >= numcpus || reqVCPUs == 0 {
+		reqVCPUs = numcpus
+	}
+
+	// Don't exceed the maximum number of vCPUs supported by hypervisor
+	if reqVCPUs > maxvcpus {
+		return maxvcpus
+	}
+
+	return reqVCPUs
+}
+
 func (h hypervisor) defaultMemSz() uint32 {
 	if h.DefaultMemSz < 8 {
 		return defaultMemSize // MiB
@@ -313,6 +333,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		KernelParams:          vc.DeserializeParams(strings.Fields(kernelParams)),
 		HypervisorMachineType: machineType,
 		DefaultVCPUs:          h.defaultVCPUs(),
+		DefaultMaxVCPUs:       h.defaultMaxVCPUs(),
 		DefaultMemSz:          h.defaultMemSz(),
 		DefaultBridges:        h.defaultBridges(),
 		DisableBlockDeviceUse: h.DisableBlockDeviceUse,
@@ -418,6 +439,7 @@ func loadConfiguration(configPath string, ignoreLogging bool) (resolvedConfigPat
 		MachineAccelerators:   defaultMachineAccelerators,
 		HypervisorMachineType: defaultMachineType,
 		DefaultVCPUs:          defaultVCPUCount,
+		DefaultMaxVCPUs:       defaultMaxVCPUCount,
 		DefaultMemSz:          defaultMemSize,
 		DefaultBridges:        defaultBridgesCount,
 		MemPrealloc:           defaultEnableMemPrealloc,

cli/config/configuration.toml.in

@@ -45,6 +45,21 @@ machine_accelerators="@MACHINEACCELERATORS@"
 # > number of physical cores      --> will be set to the actual number of physical cores
 default_vcpus = 1
 
+# Default maximum number of vCPUs per SB/VM:
+# unspecified or == 0             --> will be set to the actual number of physical cores or to the maximum number
+#                                     of vCPUs supported by KVM if that number is exceeded
+# > 0 <= number of physical cores --> will be set to the specified number
+# > number of physical cores      --> will be set to the actual number of physical cores or to the maximum number
+#                                     of vCPUs supported by KVM if that number is exceeded
+# WARNING: Depending of the architecture, the maximum number of vCPUs supported by KVM is used when
+# the actual number of physical cores is greater than it.
+# WARNING: Be aware that this value impacts the virtual machine's memory footprint and CPU
+# the hotplug functionality. For example, `default_maxvcpus = 240` specifies that until 240 vCPUs
+# can be added to a SB/VM, but the memory footprint will be big. Another example, with
+# `default_maxvcpus = 8` the memory footprint will be small, but 8 will be the maximum number of
+# vCPUs supported by the SB/VM. In general, we recommend that you do not edit this variable,
+# unless you know what are you doing.
+default_maxvcpus = @DEFMAXVCPUS@
 
 # Bridges can be used to hot plug devices.
 # Limitations:

cli/config_test.go

@@ -44,6 +44,7 @@ func makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath
 	image = "` + imagePath + `"
 	machine_type = "` + machineType + `"
 	default_vcpus = ` + strconv.FormatUint(uint64(defaultVCPUCount), 10) + `
+	default_maxvcpus = ` + strconv.FormatUint(uint64(defaultMaxVCPUCount), 10) + `
 	default_memory = ` + strconv.FormatUint(uint64(defaultMemSize), 10) + `
 	disable_block_device_use =  ` + strconv.FormatBool(disableBlock) + `
 	enable_iothreads =  ` + strconv.FormatBool(enableIOThreads) + `
@@ -129,6 +130,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf
 		KernelParams:          vc.DeserializeParams(strings.Fields(kernelParams)),
 		HypervisorMachineType: machineType,
 		DefaultVCPUs:          defaultVCPUCount,
+		DefaultMaxVCPUs:       uint32(goruntime.NumCPU()),
 		DefaultMemSz:          defaultMemSize,
 		DisableBlockDeviceUse: disableBlockDevice,
 		BlockDeviceDriver:     defaultBlockDeviceDriver,
@@ -513,6 +515,7 @@ func TestMinimalRuntimeConfig(t *testing.T) {
 		InitrdPath:            defaultInitrdPath,
 		HypervisorMachineType: defaultMachineType,
 		DefaultVCPUs:          defaultVCPUCount,
+		DefaultMaxVCPUs:       defaultMaxVCPUCount,
 		DefaultMemSz:          defaultMemSize,
 		DisableBlockDeviceUse: defaultDisableBlockDeviceUse,
 		DefaultBridges:        defaultBridgesCount,
@@ -658,10 +661,13 @@ func TestNewShimConfig(t *testing.T) {
 func TestHypervisorDefaults(t *testing.T) {
 	assert := assert.New(t)
 
+	numCPUs := goruntime.NumCPU()
+
 	h := hypervisor{}
 
 	assert.Equal(h.machineType(), defaultMachineType, "default hypervisor machine type wrong")
 	assert.Equal(h.defaultVCPUs(), defaultVCPUCount, "default vCPU number is wrong")
+	assert.Equal(h.defaultMaxVCPUs(), uint32(numCPUs), "default max vCPU number is wrong")
 	assert.Equal(h.defaultMemSz(), defaultMemSize, "default memory size is wrong")
 
 	machineType := "foo"
@@ -670,15 +676,24 @@ func TestHypervisorDefaults(t *testing.T) {
 
 	// auto inferring
 	h.DefaultVCPUs = -1
-	assert.Equal(h.defaultVCPUs(), uint32(goruntime.NumCPU()), "default vCPU number is wrong")
+	assert.Equal(h.defaultVCPUs(), uint32(numCPUs), "default vCPU number is wrong")
 
 	h.DefaultVCPUs = 2
 	assert.Equal(h.defaultVCPUs(), uint32(2), "default vCPU number is wrong")
 
-	numCPUs := goruntime.NumCPU()
 	h.DefaultVCPUs = int32(numCPUs) + 1
 	assert.Equal(h.defaultVCPUs(), uint32(numCPUs), "default vCPU number is wrong")
 
+	h.DefaultMaxVCPUs = 2
+	assert.Equal(h.defaultMaxVCPUs(), uint32(h.DefaultMaxVCPUs), "default max vCPU number is wrong")
+
+	h.DefaultMaxVCPUs = uint32(numCPUs) + 1
+	assert.Equal(h.defaultMaxVCPUs(), uint32(numCPUs), "default max vCPU number is wrong")
+
+	maxvcpus := vc.MaxQemuVCPUs()
+	h.DefaultMaxVCPUs = uint32(maxvcpus) + 1
+	assert.Equal(h.defaultMaxVCPUs(), uint32(numCPUs), "default max vCPU number is wrong")
+
 	h.DefaultMemSz = 1024
 	assert.Equal(h.defaultMemSz(), uint32(1024), "default memory size is wrong")
 }

virtcontainers/container.go

@@ -57,15 +57,11 @@ type ContainerStatus struct {
 
 // ContainerResources describes container resources
 type ContainerResources struct {
-	// CPUQuota specifies the total amount of time in microseconds
-	// The number of microseconds per CPUPeriod that the container is guaranteed CPU access
-	CPUQuota int64
+	// VCPUs are the number of vCPUs that are being used by the container
+	VCPUs uint32
 
-	// CPUPeriod specifies the CPU CFS scheduler period of time in microseconds
-	CPUPeriod uint64
-
-	// CPUShares specifies container's weight vs. other containers
-	CPUShares uint64
+	// Mem is the memory that is being used by the container
+	Mem uint32
 }
 
 // ContainerConfig describes one container runtime configuration.
@@ -804,8 +800,7 @@ func (c *Container) update(resources specs.LinuxResources) error {
 	}
 
 	newResources := ContainerResources{
-		CPUPeriod: *resources.CPU.Period,
-		CPUQuota:  *resources.CPU.Quota,
+		VCPUs: uint32(utils.ConstraintsToVCPUs(*resources.CPU.Quota, *resources.CPU.Period)),
 	}
 
 	if err := c.updateResources(currentConfig.Resources, newResources); err != nil {
@@ -866,7 +861,7 @@ func (c *Container) hotplugDrive() error {
 		Index:  driveIndex,
 	}
 
-	if err := c.sandbox.hypervisor.hotplugAddDevice(&drive, blockDev); err != nil {
+	if _, err := c.sandbox.hypervisor.hotplugAddDevice(&drive, blockDev); err != nil {
 		return err
 	}
 
@@ -903,7 +898,7 @@ func (c *Container) removeDrive() (err error) {
 		l := c.Logger().WithField("device-id", devID)
 		l.Info("Unplugging block device")
 
-		if err := c.sandbox.hypervisor.hotplugRemoveDevice(drive, blockDev); err != nil {
+		if _, err := c.sandbox.hypervisor.hotplugRemoveDevice(drive, blockDev); err != nil {
 			l.WithError(err).Info("Failed to unplug block device")
 			return err
 		}
@@ -938,14 +933,31 @@ func (c *Container) addResources() error {
 		return nil
 	}
 
-	vCPUs := utils.ConstraintsToVCPUs(c.config.Resources.CPUQuota, c.config.Resources.CPUPeriod)
+	// 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)
-		if err := c.sandbox.hypervisor.hotplugAddDevice(uint32(vCPUs), cpuDev); err != nil {
+		data, err := c.sandbox.hypervisor.hotplugAddDevice(vCPUs, cpuDev)
+		if err != nil {
 			return err
 		}
 
-		return c.sandbox.agent.onlineCPUMem(uint32(vCPUs))
+		vcpusAdded, ok := data.(uint32)
+		if !ok {
+			return fmt.Errorf("Could not get the number of vCPUs added, got %+v", data)
+		}
+
+		// A different number of vCPUs was added, we have to update
+		// the resources in order to don't remove vCPUs used by other containers.
+		if vcpusAdded != vCPUs {
+			// Set and save container's config
+			c.config.Resources.VCPUs = vcpusAdded
+			if err := c.storeContainer(); err != nil {
+				return err
+			}
+		}
+
+		return c.sandbox.agent.onlineCPUMem(vcpusAdded)
 	}
 
 	return nil
@@ -957,10 +969,18 @@ func (c *Container) removeResources() error {
 		return nil
 	}
 
-	vCPUs := utils.ConstraintsToVCPUs(c.config.Resources.CPUQuota, c.config.Resources.CPUPeriod)
+	// In order to don't remove vCPUs used by other containers, we have to remove
+	// only the vCPUs assigned to the container
+	config, err := c.sandbox.storage.fetchContainerConfig(c.sandbox.id, c.id)
+	if err != nil {
+		// don't fail, let's use the default configuration
+		config = *c.config
+	}
+
+	vCPUs := config.Resources.VCPUs
 	if vCPUs != 0 {
 		virtLog.Debugf("hot removing %d vCPUs", vCPUs)
-		if err := c.sandbox.hypervisor.hotplugRemoveDevice(uint32(vCPUs), cpuDev); err != nil {
+		if _, err := c.sandbox.hypervisor.hotplugRemoveDevice(vCPUs, cpuDev); err != nil {
 			return err
 		}
 	}
@@ -970,9 +990,9 @@ func (c *Container) removeResources() error {
 
 func (c *Container) updateResources(oldResources, newResources ContainerResources) error {
 	//TODO add support for memory, Issue: https://github.com/containers/virtcontainers/issues/578
-	var vCPUs uint
-	oldVCPUs := utils.ConstraintsToVCPUs(oldResources.CPUQuota, oldResources.CPUPeriod)
-	newVCPUs := utils.ConstraintsToVCPUs(newResources.CPUQuota, newResources.CPUPeriod)
+	var vCPUs uint32
+	oldVCPUs := oldResources.VCPUs
+	newVCPUs := newResources.VCPUs
 
 	// Update vCPUs is not possible if period and/or quota are not set or
 	// oldVCPUs and newVCPUs are equal.
@@ -989,23 +1009,36 @@ func (c *Container) updateResources(oldResources, newResources ContainerResource
 		// hot add vCPUs
 		vCPUs = newVCPUs - oldVCPUs
 		virtLog.Debugf("hot adding %d vCPUs", vCPUs)
-		if err := c.sandbox.hypervisor.hotplugAddDevice(uint32(vCPUs), cpuDev); err != nil {
+		data, err := c.sandbox.hypervisor.hotplugAddDevice(vCPUs, cpuDev)
+		if err != nil {
+			return err
+		}
+		vcpusAdded, ok := data.(uint32)
+		if !ok {
+			return fmt.Errorf("Could not get the number of vCPUs added, got %+v", data)
+		}
+		// recalculate the actual number of vCPUs if a different number of vCPUs was added
+		newResources.VCPUs = oldVCPUs + vcpusAdded
+		if err := c.sandbox.agent.onlineCPUMem(vcpusAdded); err != nil {
 			return err
 		}
 	} else {
 		// hot remove vCPUs
 		vCPUs = oldVCPUs - newVCPUs
 		virtLog.Debugf("hot removing %d vCPUs", vCPUs)
-		if err := c.sandbox.hypervisor.hotplugRemoveDevice(uint32(vCPUs), cpuDev); err != nil {
+		data, err := c.sandbox.hypervisor.hotplugRemoveDevice(vCPUs, cpuDev)
+		if err != nil {
 			return err
 		}
+		vcpusRemoved, ok := data.(uint32)
+		if !ok {
+			return fmt.Errorf("Could not get the number of vCPUs removed, got %+v", data)
+		}
+		// recalculate the actual number of vCPUs if a different number of vCPUs was removed
+		newResources.VCPUs = oldVCPUs - vcpusRemoved
 	}
 
 	// Set and save container's config
 	c.config.Resources = newResources
-	if err := c.storeContainer(); err != nil {
-		return err
-	}
-
-	return c.sandbox.agent.onlineCPUMem(uint32(vCPUs))
+	return c.storeContainer()
 }