neonvm: apply code review fixes

Co-authored-by: Em Sharnoff <[email protected]>
Signed-off-by: Misha Sakhnov <[email protected]>

Author: mikhail-sakhnov

neonvm-daemon/cmd/main.go

@@ -51,13 +51,15 @@ type cpuServer struct {
 func (s *cpuServer) handleGetCPUStatus(w http.ResponseWriter) {
 	s.cpuOperationsMutex.Lock()
 	defer s.cpuOperationsMutex.Unlock()
+
 	activeCPUs, err := s.cpuScaler.ActiveCPUsCount()
 	if err != nil {
 		s.logger.Error("could not get active CPUs count", zap.Error(err))
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
 	w.WriteHeader(http.StatusOK)
+
 	if _, err := w.Write([]byte(fmt.Sprintf("%d", activeCPUs*1000))); err != nil {
 		s.logger.Error("could not write response", zap.Error(err))
 	}
@@ -66,20 +68,24 @@ func (s *cpuServer) handleGetCPUStatus(w http.ResponseWriter) {
 func (s *cpuServer) handleSetCPUStatus(w http.ResponseWriter, r *http.Request) {
 	s.cpuOperationsMutex.Lock()
 	defer s.cpuOperationsMutex.Unlock()
+
 	body, err := io.ReadAll(r.Body)
 	if err != nil {
 		s.logger.Error("could not read request body", zap.Error(err))
 		w.WriteHeader(http.StatusBadRequest)
 		return
 	}
+	defer r.Body.Close()
+
 	updateInt, err := strconv.Atoi(string(body))
-	update := vmv1.MilliCPU(updateInt)
 	if err != nil {
 		s.logger.Error("could not unmarshal request body", zap.Error(err))
 		w.WriteHeader(http.StatusBadRequest)
 		return
 	}
-	s.logger.Info("Setting CPU status", zap.String("body", string(body)))
+
+	s.logger.Debug("Setting CPU status", zap.String("body", string(body)))
+	update := vmv1.MilliCPU(updateInt)
 	if err := s.cpuScaler.ReconcileOnlineCPU(int(update.RoundedUp())); err != nil {
 		s.logger.Error("could not ensure online CPUs", zap.Error(err))
 		w.WriteHeader(http.StatusInternalServerError)

pkg/neonvm/controllers/vm_controller.go

@@ -78,7 +78,8 @@ type VMReconciler struct {
 	Scheme   *runtime.Scheme
 	Recorder record.EventRecorder
 	Config   *ReconcilerConfig
-	Metrics  ReconcilerMetrics `exhaustruct:"optional"`
+
+	Metrics ReconcilerMetrics `exhaustruct:"optional"`
 }
 
 // The following markers are used to generate the rules permissions (RBAC) on config/rbac using controller-gen
@@ -162,9 +163,9 @@ func (r *VMReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
 	}
 
 	// examine cpuScalingMode and set it to the default value if it is not set
-	if vm.Spec.CpuScalingMode == nil || *vm.Spec.CpuScalingMode == "" {
+	if vm.Spec.CpuScalingMode == nil {
 		log.Info("Setting default CPU scaling mode", "default", r.Config.DefaultCPUScalingMode)
-		vm.Spec.CpuScalingMode = lo.ToPtr(vmv1.CpuScalingMode(r.Config.DefaultCPUScalingMode))
+		vm.Spec.CpuScalingMode = lo.ToPtr(r.Config.DefaultCPUScalingMode)
 		if err := r.tryUpdateVM(ctx, &vm); err != nil {
 			log.Error(err, "Failed to set default CPU scaling mode")
 			return ctrl.Result{}, err
@@ -576,10 +577,8 @@ func (r *VMReconciler) doReconcile(ctx context.Context, vm *vmv1.VirtualMachine)
 
 			switch *vm.Spec.CpuScalingMode {
 			case vmv1.CpuScalingModeSysfs:
-				log.Info("CPU usage check based on cgroups", "CpuScalingMode", *vm.Spec.CpuScalingMode)
 				pluggedCPU = cgroupUsage.VCPUs.RoundedUp()
 			case vmv1.CpuScalingModeQMP:
-				log.Info("CPU usage check based on QMP", "CpuScalingMode", *vm.Spec.CpuScalingMode)
 				cpuSlotsPlugged, _, err := QmpGetCpus(QmpAddr(vm))
 				if err != nil {
 					log.Error(err, "Failed to get CPU details from VirtualMachine", "VirtualMachine", vm.Name)

pkg/neonvm/controllers/vm_controller_cpu_scaling.go

@@ -86,7 +86,6 @@ func (r *VMReconciler) handleCPUScalingQMP(ctx context.Context, vm *vmv1.Virtual
 			return false, err
 		}
 	} else {
-		log.Info("No need to plug or unplug CPU")
 		hotPlugCPUScaled = true
 	}
 

pkg/neonvm/cpuscaling/sysfs.go

@@ -32,21 +32,29 @@ func (cs *cpuSysfsState) SetState(cpuNum int, cpuState cpuState) error {
 }
 
 func (cs *cpuSysfsState) GetState(cpuNum int) (cpuState, error) {
-	data, err := os.ReadFile(filepath.Join(cpuPath, "online"))
+	onlineCPUs, err := cs.getAllOnlineCPUs()
 	if err != nil {
 		return cpuOffline, err
 	}
+	if slices.Contains(onlineCPUs, cpuNum) {
+		return cpuOnline, nil
+	}
 
-	onlineCPUs, err := cs.parseMultipleCPURange(string(data))
+	return cpuOffline, nil
+}
+
+func (cs *cpuSysfsState) getAllOnlineCPUs() ([]int, error) {
+	data, err := os.ReadFile(filepath.Join(cpuPath, "online"))
 	if err != nil {
-		return cpuOffline, err
+		return nil, err
 	}
 
-	if slices.Contains(onlineCPUs, cpuNum) {
-		return cpuOnline, nil
+	onlineCPUs, err := cs.parseMultipleCPURange(string(data))
+	if err != nil {
+		return onlineCPUs, err
 	}
 
-	return cpuOffline, nil
+	return onlineCPUs, nil
 }
 
 // PossibleCPUs returns the start and end indexes of all possible CPUs.
@@ -59,7 +67,7 @@ func (cs *cpuSysfsState) PossibleCPUs() (int, int, error) {
 	return cs.parseCPURange(string(data))
 }
 
-// parseCPURange parses the CPU range string (e.g., "0-3") and returns a list of CPUs.
+// parseCPURange parses the CPU range string (e.g., "0-3") and returns start and end indexes.
 func (cs *cpuSysfsState) parseCPURange(cpuRange string) (int, int, error) {
 	cpuRange = strings.TrimSpace(cpuRange)
 	parts := strings.Split(cpuRange, "-")
@@ -86,9 +94,9 @@ func (cs *cpuSysfsState) parseCPURange(cpuRange string) (int, int, error) {
 }
 
 // parseMultipleCPURange parses the multiple CPU range string (e.g., "0-3,5-7") and returns a list of CPUs.
-func (cs *cpuSysfsState) parseMultipleCPURange(cpuRange string) ([]int, error) {
-	cpuRange = strings.TrimSpace(cpuRange)
-	parts := strings.Split(cpuRange, ",")
+func (cs *cpuSysfsState) parseMultipleCPURange(cpuRanges string) ([]int, error) {
+	cpuRanges = strings.TrimSpace(cpuRanges)
+	parts := strings.Split(cpuRanges, ",")
 
 	var cpus []int
 	for _, part := range parts {