weizhouapache commented on code in PR #8371:
URL: https://github.com/apache/cloudstack/pull/8371#discussion_r1433034778


##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java:
##########
@@ -36,82 +36,83 @@
  */
 public interface VirtualNetworkApplianceManager extends Manager, 
VirtualNetworkApplianceService {
 
-    static final String RouterTemplateXenCK = "router.template.xenserver";
-    static final String RouterTemplateKvmCK = "router.template.kvm";
-    static final String RouterTemplateVmwareCK = "router.template.vmware";
-    static final String RouterTemplateHyperVCK = "router.template.hyperv";
-    static final String RouterTemplateLxcCK = "router.template.lxc";
-    static final String RouterTemplateOvm3CK = "router.template.ovm3";
-    static final String SetServiceMonitorCK = 
"network.router.EnableServiceMonitoring";
-    static final String RouterAlertsCheckIntervalCK = 
"router.alerts.check.interval";
-    static final String VirtualRouterServiceOfferingCK = 
"router.service.offering";
-
-    static final String RouterHealthChecksConfigRefreshIntervalCK = 
"router.health.checks.config.refresh.interval";
-    static final String RouterHealthChecksResultFetchIntervalCK = 
"router.health.checks.results.fetch.interval";
-    static final String RouterHealthChecksFailuresToRecreateVrCK = 
"router.health.checks.failures.to.recreate.vr";
-
-    static final ConfigKey<String> RouterTemplateXen = new 
ConfigKey<String>(String.class, RouterTemplateXenCK, "Advanced", "SystemVM 
Template (XenServer)",
+    String RouterTemplateXenCK = "router.template.xenserver";
+    String RouterTemplateKvmCK = "router.template.kvm";
+    String RouterTemplateVmwareCK = "router.template.vmware";
+    String RouterTemplateHyperVCK = "router.template.hyperv";
+    String RouterTemplateLxcCK = "router.template.lxc";
+    String RouterTemplateOvm3CK = "router.template.ovm3";
+    String SetServiceMonitorCK = "network.router.EnableServiceMonitoring";
+    String RouterAlertsCheckIntervalCK = "router.alerts.check.interval";
+    String VirtualRouterServiceOfferingCK = "router.service.offering";
+
+    String RouterHealthChecksConfigRefreshIntervalCK = 
"router.health.checks.config.refresh.interval";
+    String RouterHealthChecksResultFetchIntervalCK = 
"router.health.checks.results.fetch.interval";
+    String RouterHealthChecksFailuresToRecreateVrCK = 
"router.health.checks.failures.to.recreate.vr";
+    String RemoveNicsOnStopCK = "ssvm.release.control.ip.on.stop";

Review Comment:
   SystemVmReleaseControlIpOnStop
   
   looks more accurate 



##########
server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java:
##########
@@ -166,18 +167,25 @@ public boolean release(NicProfile nic, 
VirtualMachineProfile vm, String reservat
         assert nic.getTrafficType() == TrafficType.Control;
         HypervisorType hType = vm.getHypervisorType();
         if ( ( (hType == HypervisorType.VMware) || (hType == 
HypervisorType.Hyperv) )&& isRouterVm(vm)) {
+            // for now place this in the vmware specific part, but it miught 
be more generic and move up two or three lines
+            if 
(!VirtualNetworkApplianceManager.RemoveNicsOnStop.valueIn(vm.getVirtualMachine().getDataCenterId()))
 {
+                if (s_logger.isDebugEnabled()) {
+                    s_logger.debug(String.format("not releasing %s\n\t from 
%s\n\t with reservationId %s.", nic, vm, reservationId));

Review Comment:
   why \n\t is used here (and other lines)? 



##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java:
##########
@@ -36,82 +36,83 @@
  */
 public interface VirtualNetworkApplianceManager extends Manager, 
VirtualNetworkApplianceService {
 
-    static final String RouterTemplateXenCK = "router.template.xenserver";
-    static final String RouterTemplateKvmCK = "router.template.kvm";
-    static final String RouterTemplateVmwareCK = "router.template.vmware";
-    static final String RouterTemplateHyperVCK = "router.template.hyperv";
-    static final String RouterTemplateLxcCK = "router.template.lxc";
-    static final String RouterTemplateOvm3CK = "router.template.ovm3";
-    static final String SetServiceMonitorCK = 
"network.router.EnableServiceMonitoring";
-    static final String RouterAlertsCheckIntervalCK = 
"router.alerts.check.interval";
-    static final String VirtualRouterServiceOfferingCK = 
"router.service.offering";
-
-    static final String RouterHealthChecksConfigRefreshIntervalCK = 
"router.health.checks.config.refresh.interval";
-    static final String RouterHealthChecksResultFetchIntervalCK = 
"router.health.checks.results.fetch.interval";
-    static final String RouterHealthChecksFailuresToRecreateVrCK = 
"router.health.checks.failures.to.recreate.vr";
-
-    static final ConfigKey<String> RouterTemplateXen = new 
ConfigKey<String>(String.class, RouterTemplateXenCK, "Advanced", "SystemVM 
Template (XenServer)",
+    String RouterTemplateXenCK = "router.template.xenserver";
+    String RouterTemplateKvmCK = "router.template.kvm";
+    String RouterTemplateVmwareCK = "router.template.vmware";
+    String RouterTemplateHyperVCK = "router.template.hyperv";
+    String RouterTemplateLxcCK = "router.template.lxc";
+    String RouterTemplateOvm3CK = "router.template.ovm3";
+    String SetServiceMonitorCK = "network.router.EnableServiceMonitoring";
+    String RouterAlertsCheckIntervalCK = "router.alerts.check.interval";
+    String VirtualRouterServiceOfferingCK = "router.service.offering";
+
+    String RouterHealthChecksConfigRefreshIntervalCK = 
"router.health.checks.config.refresh.interval";
+    String RouterHealthChecksResultFetchIntervalCK = 
"router.health.checks.results.fetch.interval";
+    String RouterHealthChecksFailuresToRecreateVrCK = 
"router.health.checks.failures.to.recreate.vr";
+    String RemoveNicsOnStopCK = "ssvm.release.control.ip.on.stop";

Review Comment:
   system.vm instead of ssvm? 



##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java:
##########
@@ -36,82 +36,83 @@
  */
 public interface VirtualNetworkApplianceManager extends Manager, 
VirtualNetworkApplianceService {
 
-    static final String RouterTemplateXenCK = "router.template.xenserver";
-    static final String RouterTemplateKvmCK = "router.template.kvm";
-    static final String RouterTemplateVmwareCK = "router.template.vmware";
-    static final String RouterTemplateHyperVCK = "router.template.hyperv";
-    static final String RouterTemplateLxcCK = "router.template.lxc";
-    static final String RouterTemplateOvm3CK = "router.template.ovm3";
-    static final String SetServiceMonitorCK = 
"network.router.EnableServiceMonitoring";
-    static final String RouterAlertsCheckIntervalCK = 
"router.alerts.check.interval";
-    static final String VirtualRouterServiceOfferingCK = 
"router.service.offering";
-
-    static final String RouterHealthChecksConfigRefreshIntervalCK = 
"router.health.checks.config.refresh.interval";
-    static final String RouterHealthChecksResultFetchIntervalCK = 
"router.health.checks.results.fetch.interval";
-    static final String RouterHealthChecksFailuresToRecreateVrCK = 
"router.health.checks.failures.to.recreate.vr";
-
-    static final ConfigKey<String> RouterTemplateXen = new 
ConfigKey<String>(String.class, RouterTemplateXenCK, "Advanced", "SystemVM 
Template (XenServer)",
+    String RouterTemplateXenCK = "router.template.xenserver";
+    String RouterTemplateKvmCK = "router.template.kvm";
+    String RouterTemplateVmwareCK = "router.template.vmware";
+    String RouterTemplateHyperVCK = "router.template.hyperv";
+    String RouterTemplateLxcCK = "router.template.lxc";
+    String RouterTemplateOvm3CK = "router.template.ovm3";
+    String SetServiceMonitorCK = "network.router.EnableServiceMonitoring";
+    String RouterAlertsCheckIntervalCK = "router.alerts.check.interval";
+    String VirtualRouterServiceOfferingCK = "router.service.offering";
+
+    String RouterHealthChecksConfigRefreshIntervalCK = 
"router.health.checks.config.refresh.interval";
+    String RouterHealthChecksResultFetchIntervalCK = 
"router.health.checks.results.fetch.interval";
+    String RouterHealthChecksFailuresToRecreateVrCK = 
"router.health.checks.failures.to.recreate.vr";
+    String RemoveNicsOnStopCK = "ssvm.release.control.ip.on.stop";

Review Comment:
   Since it is applicable for all system vms and virtual routers,  does it make 
sense move the definition to another class? 



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