wido commented on code in PR #7580:
URL: https://github.com/apache/cloudstack/pull/7580#discussion_r1241921463


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java:
##########
@@ -44,15 +44,15 @@ public void configure(Map<String, Object> params) throws 
ConfigurationException
         _pifs = (Map<String, String>)params.get("libvirt.host.pifs");
     }
 
-    protected String getControllCidr(String defaultValue, Map<String, Object> 
params) {
-        String controllCidr = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.CONTROLL_CIDR);
-        if (StringUtils.isBlank(controllCidr)) {
-            controllCidr = (String) params.get("control.cidr");
+    protected String getControlCidr(String defaultValue, Map<String, Object> 
params) {
+        String controlCidr = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.CONTROL_CIDR);
+        if (StringUtils.isBlank(controlCidr)) {
+            controlCidr = (String) params.get("control.cidr");

Review Comment:
   I think this is not needed. The AgentPropertiesFileHandler class should 
provide us what's in the agent.properties file or return the default.
   
   Do we maybe want to add some debug logging here as well?



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java:
##########
@@ -44,15 +44,15 @@ public void configure(Map<String, Object> params) throws 
ConfigurationException
         _pifs = (Map<String, String>)params.get("libvirt.host.pifs");
     }
 
-    protected String getControllCidr(String defaultValue, Map<String, Object> 
params) {
-        String controllCidr = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.CONTROLL_CIDR);
-        if (StringUtils.isBlank(controllCidr)) {
-            controllCidr = (String) params.get("control.cidr");
+    protected String getControlCidr(String defaultValue, Map<String, Object> 
params) {
+        String controlCidr = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.CONTROL_CIDR);
+        if (StringUtils.isBlank(controlCidr)) {
+            controlCidr = (String) params.get("control.cidr");
         }
-        if (StringUtils.isBlank(controllCidr)) {
-            controllCidr = defaultValue;
+        if (StringUtils.isBlank(controlCidr)) {
+            controlCidr = defaultValue;

Review Comment:
   I think this defaultValue can go as you already define this in 
[AgentProperties.java](https://github.com/apache/cloudstack/pull/7580/files/44752f1d208d03c6ee3e9ac8bbef0dad2cef8803..d4da6fa9e067d3c7a900ff459f1332a8a021f0c6#diff-967488af285b7f13f46833e85a5bee82912ec7af150055f14a0b7e70a8efc3fe)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to