Damans227 commented on code in PR #14275:
URL: https://github.com/apache/cloudstack/pull/14275#discussion_r4283334740
##########
server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java:
##########
@@ -1761,21 +1761,10 @@ protected Pair<Map<Volume, List<StoragePool>>,
List<Volume>> findSuitablePoolsFo
for (VolumeVO toBeCreated : volumesTobeCreated) {
logger.debug("Checking suitable pools for volume [{}, {}] of VM
[{}].", toBeCreated, toBeCreated.getVolumeType().name(), vmProfile);
-
- if (toBeCreated.getState() == Volume.State.Allocated &&
toBeCreated.getPoolId() != null) {
- toBeCreated.setPoolId(null);
- if (!_volsDao.update(toBeCreated.getId(), toBeCreated)) {
- throw new CloudRuntimeException(String.format("Error
updating volume [%s] to clear pool Id.", toBeCreated));
- }
- if (logger.isDebugEnabled()) {
- logger.debug("Setting pool_id to NULL for volume id={} as
it is in Allocated state", toBeCreated);
- }
- }
- // If the plan specifies a poolId, it means that this VM's ROOT
- // volume is ready and the pool should be reused.
- // In this case, also check if rest of the volumes are ready and
can
- // be reused.
- if ((plan.getPoolId() != null || (toBeCreated.getVolumeType() ==
Volume.Type.DATADISK && toBeCreated.getPoolId() != null &&
toBeCreated.getState() == Volume.State.Ready)) &&
+ // If the plan specifies a poolId, prefer the existing pool if it
is still suitable;
+ // otherwise fall through to the allocator so the VM can still
start.
+ if (plan.getPoolId() != null || (toBeCreated.getPoolId() != null &&
Review Comment:
when the plan already has a pool, does every volume skip here now without
getting a pool? the outer brackets from the old check are gone, so the reuse
check never runs in that case
--
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]
