[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6655: server: fix error when dedicatingguestvlanrange for physical nw without vlan range

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

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


##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -4068,16 +4068,15 @@ public GuestVlanRange dedicateGuestVlanRange(DedicateGuestVlanRangeCmd cmd) {
             }
         }
 
-        // Verify guest vlans in the range don't belong to a network of a different account
         for (int i = startVlan; i <= endVlan; i++) {
-            List<DataCenterVnetVO> allocatedVlans = _dcVnetDao.listAllocatedVnetsInRange(physicalNetwork.getDataCenterId(), physicalNetwork.getId(), startVlan, endVlan);
-            if (allocatedVlans != null && !allocatedVlans.isEmpty()) {
-                for (DataCenterVnetVO allocatedVlan : allocatedVlans) {
-                    if (allocatedVlan.getAccountId() != vlanOwner.getAccountId()) {
-                        throw new InvalidParameterValueException("Guest vlan from this range " + allocatedVlan.getVnet() + " is allocated to a different account."
-                                + " Can only dedicate a range which has no allocated vlans or has vlans allocated to the same account ");
-                    }
-                }
+            List<DataCenterVnetVO> dataCenterVnet = _dcVnetDao.findVnet(physicalNetwork.getDataCenterId(), physicalNetworkId, Integer.toString(i));
+            if (CollectionUtils.isEmpty(dataCenterVnet)) {
+                throw new InvalidParameterValueException(String.format("Guest vlan %d from this range %s is not present in the system for physical network ID: %s", i, vlan, physicalNetwork.getUuid()));
+            }
+            // Verify guest vlans in the range don't belong to a network of a different account
+            if (dataCenterVnet.get(0).getAccountId() != null && dataCenterVnet.get(0).getAccountId() != vlanOwner.getAccountId()) {
+                throw new InvalidParameterValueException("Guest vlan from this range " + dataCenterVnet.get(0).getVnet() + " is allocated to a different account."
+                        + " Can only dedicate a range which has no allocated vlans or has vlans allocated to the same account ");

Review Comment:
   can you extract to a `verifyGuestVlan()` (or `verifyGuestVlans()` when including the loop in the extraction)?



-- 
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: commits-unsubscribe@cloudstack.apache.org

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