weizhouapache commented on a change in pull request #5428:
URL: https://github.com/apache/cloudstack/pull/5428#discussion_r707294037
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -2575,6 +2575,53 @@ protected void runInContext() {
}
}
+ private void verifyVmLimits(UserVmVO vmInstance, Map<String,String>
details) {
+ Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER)
!= null ? details.get(VmDetailConstants.CPU_NUMBER) : "0");
+ Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY) !=
null ? details.get(VmDetailConstants.MEMORY) : "0");
+ long currentCpu = 0L;
+ long currentMemory = 0L;
+ List<UserVmDetailVO> vmDetails =
userVmDetailsDao.listDetails(vmInstance.getId());
+ for (UserVmDetailVO detailVO : vmDetails) {
+ if (VmDetailConstants.CPU_NUMBER.equals(detailVO.getName())) {
+ currentCpu = Long.parseLong(detailVO.getValue());
+ }
+ if (VmDetailConstants.MEMORY.equals(detailVO.getName())) {
+ currentMemory = Long.parseLong(detailVO.getValue());
+ }
+ }
+ Account owner = _accountDao.findById(vmInstance.getAccountId());
+ if (owner == null) {
+ throw new InvalidParameterValueException("The owner of " +
vmInstance + " does not exist: " + vmInstance.getAccountId());
+ }
+
+ if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
Review comment:
@Pearl1594
you can move this to the first check of 'verifyVmLimits', so no need to get
current cpu/memory, etc
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -2582,8 +2576,8 @@ protected void runInContext() {
}
private void verifyVmLimits(UserVmVO vmInstance, Map<String,String>
details) {
- Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER));
- Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY));
+ Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER)
!= null ? details.get(VmDetailConstants.CPU_NUMBER) : "0");
Review comment:
@Pearl1594
use NumberUtils.toLong ?
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -2575,6 +2575,53 @@ protected void runInContext() {
}
}
+ private void verifyVmLimits(UserVmVO vmInstance, Map<String,String>
details) {
+ Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER)
!= null ? details.get(VmDetailConstants.CPU_NUMBER) : "0");
+ Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY) !=
null ? details.get(VmDetailConstants.MEMORY) : "0");
+ long currentCpu = 0L;
+ long currentMemory = 0L;
+ List<UserVmDetailVO> vmDetails =
userVmDetailsDao.listDetails(vmInstance.getId());
Review comment:
you can use this instead.
ServiceOfferingVO currentServiceOffering =
_offeringDao.findByIdIncludingRemoved(vmInstance.getId(),
vmInstance.getServiceOfferingId());
int currentCpu = currentServiceOffering.getCpu();
int currentMemory = currentServiceOffering.getRamSize();
##########
File path:
engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java
##########
@@ -197,7 +197,7 @@ public void delTag() {
}
@Override
- public String reserve(DeploymentPlanner plannerToUse, DeploymentPlan plan,
ExcludeList exclude, String caller) throws InsufficientCapacityException,
+ public String reserve(DeploymentPlanner plannerToUse, DeploymentPlan
plan, ExcludeList exclude, String caller) throws InsufficientCapacityException,
Review comment:
@Pearl1594
revert this line ?
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -2575,6 +2575,53 @@ protected void runInContext() {
}
}
+ private void verifyVmLimits(UserVmVO vmInstance, Map<String,String>
details) {
+ Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER)
!= null ? details.get(VmDetailConstants.CPU_NUMBER) : "0");
+ Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY) !=
null ? details.get(VmDetailConstants.MEMORY) : "0");
+ long currentCpu = 0L;
+ long currentMemory = 0L;
+ List<UserVmDetailVO> vmDetails =
userVmDetailsDao.listDetails(vmInstance.getId());
+ for (UserVmDetailVO detailVO : vmDetails) {
+ if (VmDetailConstants.CPU_NUMBER.equals(detailVO.getName())) {
+ currentCpu = Long.parseLong(detailVO.getValue());
+ }
+ if (VmDetailConstants.MEMORY.equals(detailVO.getName())) {
+ currentMemory = Long.parseLong(detailVO.getValue());
+ }
+ }
+ Account owner = _accountDao.findById(vmInstance.getAccountId());
+ if (owner == null) {
+ throw new InvalidParameterValueException("The owner of " +
vmInstance + " does not exist: " + vmInstance.getAccountId());
+ }
+
+ if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
+ return;
+ }
+
+ try {
+ if (newCpu > currentCpu) {
+ _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu,
newCpu - currentCpu);
+ }
+ if (newMemory > currentMemory) {
+ _resourceLimitMgr.checkResourceLimit(owner,
ResourceType.memory, newMemory - currentMemory);
+ }
+ } catch (ResourceAllocationException e) {
+ throw new CloudRuntimeException("Failed to update VM settings", e);
+ }
+
+ if (newCpu > currentCpu) {
+ _resourceLimitMgr.incrementResourceCount(owner.getAccountId(),
ResourceType.cpu, newCpu - currentCpu);
+ } else {
Review comment:
is it better to use ?
```
} else if (newCpu > 0 && newCpu < currentCpu) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
