DaanHoogland commented on code in PR #12349:
URL: https://github.com/apache/cloudstack/pull/12349#discussion_r2652484329


##########
api/src/main/java/com/cloud/network/NetworkModel.java:
##########
@@ -125,6 +125,10 @@ public interface NetworkModel {
      */
     String getNextAvailableMacAddressInNetwork(long networkConfigurationId) 
throws InsufficientAddressCapacityException;
 
+    default boolean isMACUnique(String mac, long networkId) {
+        return true;
+    }

Review Comment:
   why this default implementation?



##########
server/src/main/java/com/cloud/network/guru/PodBasedNetworkGuru.java:
##########
@@ -131,7 +133,11 @@ public void reserve(NicProfile nic, Network config, 
VirtualMachineProfile vm, De
         Integer vlan = result.getVlan();
 
         nic.setIPv4Address(result.getIpAddress());
-        
nic.setMacAddress(NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(result.getMacAddress(),
 NetworkModel.MACIdentifier.value())));
+        String macAddress = 
NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(result.getMacAddress(),
 _networkModel.getMacIdentifier(dest.getDataCenter().getId())));
+        if (!_networkModel.isMACUnique(macAddress, config.getId())) {
+            macAddress = 
_networkModel.getNextAvailableMacAddressInNetwork(config.getId());
+        }
+        nic.setMacAddress(macAddress);

Review Comment:
   This pattern seems to be recurring (also in `PrivateNetworkGuru`, 
`StorageNetworkGuru` and `NetProfileHelperImpl`). Is it worth codifying it in a 
method?



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