Address comments

* fs cgroup drivers should handle the cases when CPU burst is set to 0
  (i.e., to disable).
* require period to be set when setting burst.

Signed-off-by: Kailun Qin <kailun.qin@intel.com>

libcontainer/cgroups/fs/cpu.go

@@ -91,21 +91,19 @@ func (s *CpuGroup) Set(path string, r *configs.Resources) error {
 	}
 
 	var burst string
-	if r.CpuBurst != 0 {
-		burst = strconv.FormatUint(r.CpuBurst, 10)
-		if err := cgroups.WriteFile(path, "cpu.cfs_burst_us", burst); err != nil {
-			// Sometimes when the burst to be set is larger
-			// than the current one, it is rejected by the kernel
-			// (EINVAL) as old_quota/new_burst exceeds the parent
-			// cgroup quota limit. If this happens and the quota is
-			// going to be set, ignore the error for now and retry
-			// after setting the quota.
-			if !errors.Is(err, unix.EINVAL) || r.CpuQuota == 0 {
-				return err
-			}
-		} else {
-			burst = ""
+	burst = strconv.FormatUint(r.CpuBurst, 10)
+	if err := cgroups.WriteFile(path, "cpu.cfs_burst_us", burst); err != nil {
+		// Sometimes when the burst to be set is larger
+		// than the current one, it is rejected by the kernel
+		// (EINVAL) as old_quota/new_burst exceeds the parent
+		// cgroup quota limit. If this happens and the quota is
+		// going to be set, ignore the error for now and retry
+		// after setting the quota.
+		if !errors.Is(err, unix.EINVAL) || r.CpuQuota == 0 {
+			return err
 		}
+	} else {
+		burst = ""
 	}
 
 	if r.CpuQuota != 0 {

libcontainer/cgroups/fs2/cpu.go

@@ -29,21 +29,19 @@ func setCpu(dirPath string, r *configs.Resources) error {
 	}
 
 	var burst string
-	if r.CpuBurst != 0 {
-		burst = strconv.FormatUint(r.CpuBurst, 10)
-		if err := cgroups.WriteFile(dirPath, "cpu.max.burst", burst); err != nil {
-			// Sometimes when the burst to be set is larger
-			// than the current one, it is rejected by the kernel
-			// (EINVAL) as old_quota/new_burst exceeds the parent
-			// cgroup quota limit. If this happens and the quota is
-			// going to be set, ignore the error for now and retry
-			// after setting the quota.
-			if !errors.Is(err, unix.EINVAL) || r.CpuQuota == 0 {
-				return err
-			}
-		} else {
-			burst = ""
+	burst = strconv.FormatUint(r.CpuBurst, 10)
+	if err := cgroups.WriteFile(dirPath, "cpu.max.burst", burst); err != nil {
+		// Sometimes when the burst to be set is larger
+		// than the current one, it is rejected by the kernel
+		// (EINVAL) as old_quota/new_burst exceeds the parent
+		// cgroup quota limit. If this happens and the quota is
+		// going to be set, ignore the error for now and retry
+		// after setting the quota.
+		if !errors.Is(err, unix.EINVAL) || r.CpuQuota == 0 {
+			return err
 		}
+	} else {
+		burst = ""
 	}
 
 	if r.CpuQuota != 0 || r.CpuPeriod != 0 {

update.go

@@ -264,37 +264,39 @@ other options are ignored.
 		// Update the values
 		config.Cgroups.Resources.BlkioWeight = *r.BlockIO.Weight
 
-		// Setting CPU quota, period and burst independently does not make much
-		// sense, but historically runc allowed it and this needs to be
-		// supported to not break compatibility.
+		// Setting CPU quota and period independently does not make much sense,
+		// but historically runc allowed it and this needs to be supported
+		// to not break compatibility.
 		//
 		// For systemd cgroup drivers to set CPU quota/period correctly,
 		// it needs to know both values. For fs2 cgroup driver to be compatible
 		// with the fs driver, it also needs to know both values.
 		//
 		// Here in update, previously set values are available from config.
-		// If only one of {quota,period,burst} is set and the others are not,
-		// leave the unset parameter at the old value (don't overwrite config).
-		p, q, b := *r.CPU.Period, *r.CPU.Quota, *r.CPU.Burst
-		if (p == 0 && q == 0 && b == 0) || (p != 0 && q != 0 && b != 0) {
-			// all values are either set or unset (0)
+		// If only one of {quota,period} is set and the other is not, leave
+		// the unset parameter at the old value (don't overwrite config).
+		p, q := *r.CPU.Period, *r.CPU.Quota
+		if (p == 0 && q == 0) || (p != 0 && q != 0) {
+			// both values are either set or unset (0)
 			config.Cgroups.Resources.CpuPeriod = p
 			config.Cgroups.Resources.CpuQuota = q
-			config.Cgroups.Resources.CpuBurst = b
 		} else {
-			// one is set and the others are not
+			// one is set and the other is not
 			if p != 0 {
-				// set new period, leave quota and burst at old values
+				// set new period, leave quota at old value
 				config.Cgroups.Resources.CpuPeriod = p
 			} else if q != 0 {
-				// set new quota, leave period and burst at old values
+				// set new quota, leave period at old value
 				config.Cgroups.Resources.CpuQuota = q
-			} else if b != 0 {
-				// set new burst, leave quota and period at old values
-				config.Cgroups.Resources.CpuBurst = b
 			}
 		}
 
+		b := *r.CPU.Burst
+		if config.Cgroups.Resources.CpuPeriod == 0 && b != 0 {
+			return errors.New("period is not set when setting burst")
+		}
+		config.Cgroups.Resources.CpuBurst = b
+
 		config.Cgroups.Resources.CpuShares = *r.CPU.Shares
 		// CpuWeight is used for cgroupv2 and should be converted
 		config.Cgroups.Resources.CpuWeight = cgroups.ConvertCPUSharesToCgroupV2Value(*r.CPU.Shares)