[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4190: Broadcast URI not set to vxlan, but vlan (Fix #3040)

[GitBox <gi...@apache.org>]

GabrielBrascher commented on a change in pull request #4190:
URL: https://github.com/apache/cloudstack/pull/4190#discussion_r448341456



##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -2475,6 +2475,21 @@ public Network doInTransaction(final TransactionStatus status) {
         return network;
     }
 
+    /**
+     * Encodes VLAN/VXLAN ID into a Broadcast URI according to the isolation method from the Physical Network.
+     * @return Broadcast URI, e.g. 'vlan://vlan_ID' or 'vxlan://vlxan_ID'
+     */
+    protected URI encodeVlanIdIntoBroadcastUri(String vlanId, PhysicalNetwork pNtwk) {
+        if(StringUtils.isNotBlank(pNtwk.getIsolationMethods().get(0))) {

Review comment:
       I got wondering if there could be more than one isolation method on a network. Considering that this is a List of isolation methods. I assume that to get into a VLAN conditional (the ones that this method is called) there will be only one isolation method, either VLAN or VXLAN.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org