DaanHoogland commented on a change in pull request #4422:
URL: https://github.com/apache/cloudstack/pull/4422#discussion_r608427716
##########
File path:
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
##########
@@ -996,7 +1052,51 @@ public long calculateMemoryForAccount(long accountId) {
for (UserVmJoinVO vm : userVms) {
ramtotal += Long.valueOf(vm.getRamSize());
}
- return ramtotal;
+
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
Review comment:
no reason to grow this method for this bit.
```suggestion
ServiceOffering defaultRouterOffering =
getDefaultServiceOfferingOrSomeOtherNewName();
```
##########
File path:
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
##########
@@ -972,11 +980,59 @@ public long countCpusForAccount(long accountId) {
for (UserVmJoinVO vm : userVms) {
cputotal += Long.valueOf(vm.getCpu());
}
- return cputotal;
+
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
+ GenericSearchBuilder<ServiceOfferingVO, SumCount> cpuSearch =
_serviceOfferingDao.createSearchBuilder(SumCount.class);
+ cpuSearch.select("sum", Func.SUM, cpuSearch.entity().getCpu());
+ cpuSearch.select("count", Func.COUNT, (Object[])null);
+ SearchBuilder<VMInstanceVO> join1 = _vmDao.createSearchBuilder();
+ join1.and("accountId", join1.entity().getAccountId(), Op.EQ);
+ join1.and("type", join1.entity().getType(), Op.IN);
+ join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN);
+ join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ);
+ cpuSearch.join("offerings", join1, cpuSearch.entity().getId(),
join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER);
+ cpuSearch.done();
+
+ SearchCriteria<SumCount> sc = cpuSearch.create();
+ sc.setJoinParameters("offerings", "accountId", accountId);
+ sc.setJoinParameters("offerings", "type",
VirtualMachine.Type.DomainRouter); // domain routers
+
+ if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
+ sc.setJoinParameters("offerings", "state", new Object[]
{State.Destroyed, State.Error, State.Expunging, State.Stopped});
+ } else {
+ sc.setJoinParameters("offerings", "state", new Object[]
{State.Destroyed, State.Error, State.Expunging});
+ }
+ sc.setJoinParameters("offerings", "displayVm", 1);
+ List<SumCount> cpus = _serviceOfferingDao.customSearch(sc, null);
Review comment:
should be in `ServiceOfferingDao`, no reason to grow this method for
this code.
##########
File path:
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
##########
@@ -996,7 +1052,51 @@ public long calculateMemoryForAccount(long accountId) {
for (UserVmJoinVO vm : userVms) {
ramtotal += Long.valueOf(vm.getRamSize());
}
- return ramtotal;
+
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
+ GenericSearchBuilder<ServiceOfferingVO, SumCount> memorySearch =
_serviceOfferingDao.createSearchBuilder(SumCount.class);
+ memorySearch.select("sum", Func.SUM,
memorySearch.entity().getRamSize());
+ memorySearch.select("count", Func.COUNT, (Object[])null);
+ SearchBuilder<VMInstanceVO> join1 = _vmDao.createSearchBuilder();
+ join1.and("accountId", join1.entity().getAccountId(), Op.EQ);
+ join1.and("type", join1.entity().getType(), Op.IN);
+ join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN);
+ join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ);
+ memorySearch.join("offerings", join1, memorySearch.entity().getId(),
join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER);
+ memorySearch.done();
+
+ SearchCriteria<SumCount> sc = memorySearch.create();
+ sc.setJoinParameters("offerings", "accountId", accountId);
+ sc.setJoinParameters("offerings", "type",
VirtualMachine.Type.DomainRouter); // domain routers
+ sc.setJoinParameters("offerings", "displayVm", 1);
+
+ if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
+ sc.setJoinParameters("offerings", "state", new Object[]
{State.Destroyed, State.Error, State.Expunging, State.Stopped});
+ } else {
+ sc.setJoinParameters("offerings", "state", new Object[]
{State.Destroyed, State.Error, State.Expunging});
+ }
+ sc.setJoinParameters("offerings", "displayVm", 1);
+ List<SumCount> memory = _serviceOfferingDao.customSearch(sc, null);
Review comment:
this bit seems like it should be a member method of
`ServiceOfferringDao`, or activated by flag in an existing method in
`ServiceOfferringDao`
##########
File path:
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
##########
@@ -972,11 +980,59 @@ public long countCpusForAccount(long accountId) {
for (UserVmJoinVO vm : userVms) {
cputotal += Long.valueOf(vm.getCpu());
}
- return cputotal;
+
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
Review comment:
```suggestion
ServiceOffering defaultRouterOffering =
getDefaultServiceOfferingOrSomeOtherNewName();
```
this happens below as well, for memory.
##########
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -1368,6 +1382,131 @@ private void
logBootModeParameters(Map<VirtualMachineProfile.Param, Object> para
}
}
+ /**
Review comment:
`incrementVrResourceCount` (60 lines) and `decrementVrResourceCount` (38
lines) are big to start with and a lot of comment. Each comment could be the
javadoc for a new method instead of having just these two.
##########
File path:
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
##########
@@ -972,11 +980,59 @@ public long countCpusForAccount(long accountId) {
for (UserVmJoinVO vm : userVms) {
cputotal += Long.valueOf(vm.getCpu());
}
- return cputotal;
+
+ ServiceOffering defaultRouterOffering = null;
+ String globalRouterOffering =
VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+ if (globalRouterOffering != null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByUuid(globalRouterOffering);
+ }
+ if (defaultRouterOffering == null) {
+ defaultRouterOffering =
_serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName);
+ }
+ GenericSearchBuilder<ServiceOfferingVO, SumCount> cpuSearch =
_serviceOfferingDao.createSearchBuilder(SumCount.class);
+ cpuSearch.select("sum", Func.SUM, cpuSearch.entity().getCpu());
+ cpuSearch.select("count", Func.COUNT, (Object[])null);
+ SearchBuilder<VMInstanceVO> join1 = _vmDao.createSearchBuilder();
+ join1.and("accountId", join1.entity().getAccountId(), Op.EQ);
+ join1.and("type", join1.entity().getType(), Op.IN);
+ join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN);
+ join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ);
+ cpuSearch.join("offerings", join1, cpuSearch.entity().getId(),
join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER);
+ cpuSearch.done();
+
+ SearchCriteria<SumCount> sc = cpuSearch.create();
+ sc.setJoinParameters("offerings", "accountId", accountId);
+ sc.setJoinParameters("offerings", "type",
VirtualMachine.Type.DomainRouter); // domain routers
+
+ if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
+ sc.setJoinParameters("offerings", "state", new Object[]
{State.Destroyed, State.Error, State.Expunging, State.Stopped});
+ } else {
+ sc.setJoinParameters("offerings", "state", new Object[]
{State.Destroyed, State.Error, State.Expunging});
+ }
+ sc.setJoinParameters("offerings", "displayVm", 1);
+ List<SumCount> cpus = _serviceOfferingDao.customSearch(sc, null);
+ if (cpus != null) {
+ // Calculate the VR CPU resource count depending on the global
setting
+ if
(VirtualMachineManager.ResourceCountRouters.valueIn(domain.getId())) {
+ cputotal += cpus.get(0).sum;
+
+ // Get the diff of current router offering with default
offering
+ if
(VirtualMachineManager.ResourceCountRoutersType.valueIn(domain.getId())
+
.equalsIgnoreCase(VirtualMachineManager.COUNT_DELTA_VR_RESOURCES)) {
+ cputotal = cputotal - cpus.get(0).count *
defaultRouterOffering.getCpu();
+ }
+ }
+ return cputotal;
+ } else {
+ return cputotal;
+ }
Review comment:
this also is the same as below with the exception of the naming
maybe create a
private long totaliseResourceCount(List<SumCount> resourceCounts){...}
--
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:
[email protected]
