nvazquez commented on a change in pull request #5181:
URL: https://github.com/apache/cloudstack/pull/5181#discussion_r683465834
##########
File path:
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
##########
@@ -956,6 +952,11 @@ public static boolean isSpecMatch(DVPortgroupConfigInfo
currentDvPortgroupInfo,
specMatches = false;
}
}
+ if (currentPortSetting.getMacManagementPolicy() != null &&
newPortSetting.getMacManagementPolicy() != null &&
+
currentPortSetting.getMacManagementPolicy().getMacLearningPolicy() != null &&
newPortSetting.getMacManagementPolicy().getMacLearningPolicy() != null &&
+
currentPortSetting.getMacManagementPolicy().getMacLearningPolicy().isEnabled()
!= newPortSetting.getMacManagementPolicy().getMacLearningPolicy().isEnabled()) {
Review comment:
Can you extract this big `if` clause into a method to improve
readability? Then this could be like:
````
if (hasMacLearningPolicyChanged(currentPortSetting, newPortSetting) {
specMatches = false;
}
````
##########
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##########
@@ -2144,6 +2152,38 @@ private boolean checkForNonStoppedVmInNetwork(long
networkId) {
return vms.isEmpty();
}
+ private void replugNicsForUpdatedNetwork(NetworkVO network) throws
ResourceUnavailableException, InsufficientCapacityException {
+ List<NicVO> nics = _nicDao.listByNetworkId(network.getId());
+ Network updatedNetwork = getNetwork(network.getId());
+ for (NicVO nic : nics) {
+ long vmId = nic.getInstanceId();
+ VMInstanceVO vm = _vmDao.findById(vmId);
+ if (vm == null) {
+ s_logger.error("VM for nic " + nic.getId() + " not found with
Vm Id:" + vmId);
+ continue;
+ }
+ if
(!Hypervisor.HypervisorType.VMware.equals(vm.getHypervisorType())) {
+ s_logger.debug("VM is not on VMware");
Review comment:
Maybe add more information to the following log lines? Something like:
`Cannot replug nic <ID> as: VM ...`
##########
File path:
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
##########
@@ -1215,6 +1218,35 @@ public static DVSSecurityPolicy
createDVSSecurityPolicy(Map<NetworkOffering.Deta
return secPolicy;
}
+ public static DVSMacManagementPolicy
createDVSMacManagementPolicy(Map<NetworkOffering.Detail, String> nicDetails) {
+ DVSMacManagementPolicy macManagementPolicy = new
DVSMacManagementPolicy();
+ macManagementPolicy.setAllowPromiscuous(false);
Review comment:
Minor improvement to reduce lines of code:
````
macManagementPolicy.setAllowPromiscuous(
nicDetails.containsKey(NetworkOffering.Detail.PromiscuousMode) ?
Boolean.valueOf(nicDetails.get(NetworkOffering.Detail.PromiscuousMode)) :
false);
````
Same for the other properties. Then you won't need the if blocks below.
--
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]
