Copilot commented on code in PR #12767:
URL: https://github.com/apache/cloudstack/pull/12767#discussion_r2904765807
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -1017,6 +1017,10 @@ public PrimaryDataStoreInfo
createPool(CreateStoragePoolCmd cmd) throws Resource
if (Grouping.AllocationState.Disabled == zone.getAllocationState() &&
!_accountMgr.isRootAdmin(account.getId())) {
throw new PermissionDeniedException(String.format("Cannot perform
this operation, Zone is currently disabled: %s", zone));
}
+ // Check if it's local storage and if it's enabled on the zone
+ if (isFileScheme && !zone.isLocalStorageEnabled()) {
+ throw new InvalidParameterValueException("Local storage is not
enabled for zone: " + zone);
+ }
Review Comment:
The condition here only checks `zone.isLocalStorageEnabled()`, but the
underlying `createLocalStorage(Host, StoragePoolInfo)` method at line 818
allows local storage creation when **either** `isLocalStorageEnabled()` is true
**or** `SystemVMUseLocalStorage` is true for the zone.
This means that if an admin has configured
`system.vm.use.local.storage=true` for a zone (but hasn't enabled the
zone-level "local storage enabled" flag), this new check will incorrectly
reject the request with an `InvalidParameterValueException`, whereas previously
the pool creation would have succeeded.
The fix should replicate the same condition from `createLocalStorage(Host,
StoragePoolInfo)` (line 818) by also checking
`ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(zone.getId())`. For
example, the condition should be: `if (isFileScheme &&
!zone.isLocalStorageEnabled() &&
!BooleanUtils.toBoolean(ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(zone.getId())))`.
--
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]
