GabrielBrascher commented on a change in pull request #4255: URL: https://github.com/apache/cloudstack/pull/4255#discussion_r469397649
########## File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java ########## @@ -1985,6 +1985,10 @@ private boolean hasSuitablePoolsForVolume(final VolumeVO volume, final Host host vlanSearch.and("vlanType", vlanSearch.entity().getVlanType(), SearchCriteria.Op.EQ); sb.join("vlanSearch", vlanSearch, sb.entity().getVlanId(), vlanSearch.entity().getId(), JoinBuilder.JoinType.INNER); + SearchBuilder<DataCenterVO> zoneSearchBuilder = _dcDao.createSearchBuilder(); Review comment: I think that it would be better to implement this via the VLAN Search Builder instead of adding the proposed search builder. Here are a few points of why I think it would be better: 1. All removed zones should have its networks removed anyway, therefore VLANs of a removed zone are marked as removed as well; 2. this could also prevent issues of selecting IP addresses of removed VLANs on an existing Zone; 3. and, at the end, it reduces costs of one extra join. Currently lines 1984 - 1986: ``` final SearchBuilder<VlanVO> vlanSearch = _vlanDao.createSearchBuilder(); vlanSearch.and("vlanType", vlanSearch.entity().getVlanType(), SearchCriteria.Op.EQ); sb.join("vlanSearch", vlanSearch, sb.entity().getVlanId(), vlanSearch.entity().getId(), JoinBuilder.JoinType.INNER); ``` Changing current VLAN search builder adding a verification "_WHERE removed IS NULL_": ``` final SearchBuilder<VlanVO> vlanSearch = _vlanDao.createSearchBuilder(); vlanSearch.and("vlanType", vlanSearch.entity().getVlanType(), SearchCriteria.Op.EQ); vlanSearch.and("removed", vlanSearch.entity().getRemoved(), SearchCriteria.Op.NULL); sb.join("vlanSearch", vlanSearch, sb.entity().getVlanId(), vlanSearch.entity().getId(), JoinBuilder.JoinType.INNER);SearchCriteria.Op.NULL); ``` ---------------------------------------------------------------- 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: us...@infra.apache.org