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
