GabrielBrascher commented on a change in pull request #4955:
URL: https://github.com/apache/cloudstack/pull/4955#discussion_r623927903
##########
File path:
services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
##########
@@ -816,37 +773,33 @@ public boolean isZoneReady(Map<Long, ZoneHostInfo>
zoneHostInfoMap, long dataCen
VMTemplateVO template =
_templateDao.findSystemVMReadyTemplate(dataCenterId, HypervisorType.Any);
if (template == null) {
if (s_logger.isDebugEnabled()) {
- s_logger.debug("System vm template is not ready at data
center " + dataCenterId + ", wait until it is ready to launch secondary storage
vm");
+ s_logger.debug(String.format("System VM template is not
ready at zone [%s], wait until it is ready to launch secondary storage VM.",
dataCenterId));
}
return false;
}
List<DataStore> stores =
_dataStoreMgr.getImageStoresByScopeExcludingReadOnly(new
ZoneScope(dataCenterId));
- if (stores.size() < 1) {
- s_logger.debug("No image store added in zone " + dataCenterId
+ ", wait until it is ready to launch secondary storage vm");
+ if (CollectionUtils.isEmpty(stores)) {
+ s_logger.debug(String.format("No image store added in zone
[%s], wait until it is ready to launch secondary storage VM.", dataCenterId));
return false;
}
if (!template.isDirectDownload() &&
templateMgr.getImageStore(dataCenterId, template.getId()) == null) {
if (s_logger.isDebugEnabled()) {
- s_logger.debug("No secondary storage available in zone " +
dataCenterId + ", wait until it is ready to launch secondary storage vm");
+ s_logger.debug(String.format("No secondary storage
available in zone [%s], wait until it is ready to launch secondary storage
VM.", dataCenterId));
}
return false;
}
- boolean useLocalStorage = false;
- Boolean useLocal =
ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dataCenterId);
- if (useLocal != null) {
- useLocalStorage = useLocal.booleanValue();
- }
+ boolean useLocalStorage =
BooleanUtils.toBoolean(ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dataCenterId));
List<Pair<Long, Integer>> l =
_storagePoolHostDao.getDatacenterStoragePoolHostInfo(dataCenterId,
!useLocalStorage);
- if (l != null && l.size() > 0 && l.get(0).second().intValue() > 0)
{
+ if (CollectionUtils.isNotEmpty(l) && l.get(0).second() > 0) {
Review comment:
The changes are really nice, good work. I would also suggest renaming
these lists from `l` to something that actually means something.
Example:
1. From
```
List<Pair<Long, Integer>> l =
_storagePoolHostDao.getDatacenterStoragePoolHostInfo(dataCenterId,
!useLocalStorage);
if (CollectionUtils.isNotEmpty(l) && l.get(0).second() > 0) {
...
}
```
2. To
```
List<Pair<Long, Integer>> storagePoolHostInfos =
_storagePoolHostDao.getDatacenterStoragePoolHostInfo(dataCenterId,
!useLocalStorage);
if (CollectionUtils.isNotEmpty(storagePoolHostInfos) &&
storagePoolHostInfos.get(0).second() > 0) {
...
}
```
3. Or
```
List<Pair<Long, Integer>> zoneStoragePoolsInfo =
_storagePoolHostDao.getDatacenterStoragePoolHostInfo(dataCenterId,
!useLocalStorage);
if (CollectionUtils.isNotEmpty(zoneStoragePoolsInfo) &&
zoneStoragePoolsInfo.get(0).second() > 0) {
...
}
```
--
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]
