You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2020/10/30 15:53:33 UTC
[cloudstack] branch 4.13 updated: fix NPE in volumes statistics
(#4388)
This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.13
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.13 by this push:
new 8afb451 fix NPE in volumes statistics (#4388)
8afb451 is described below
commit 8afb451c1c60ec106aeecedafdfb491e3ae16cc8
Author: slavkap <51...@users.noreply.github.com>
AuthorDate: Fri Oct 30 17:53:05 2020 +0200
fix NPE in volumes statistics (#4388)
---
.../com/cloud/api/query/ViewResponseHelper.java | 7 +---
.../main/java/com/cloud/server/StatsCollector.java | 14 ++-----
.../src/main/java/com/cloud/vm/UserVmManager.java | 2 +-
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 45 ++++++++++++----------
4 files changed, 31 insertions(+), 37 deletions(-)
diff --git a/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java b/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
index ced81a6..d168168 100644
--- a/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
+++ b/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
@@ -282,13 +282,10 @@ public class ViewResponseHelper {
vrDataList.put(vr.getId(), vrData);
VolumeStats vs = null;
- if (vr.getFormat() == ImageFormat.QCOW2) {
- vs = ApiDBUtils.getVolumeStatistics(vrData.getId());
- }
- else if (vr.getFormat() == ImageFormat.VHD){
+ if (vr.getFormat() == ImageFormat.VHD || vr.getFormat() == ImageFormat.QCOW2) {
vs = ApiDBUtils.getVolumeStatistics(vrData.getPath());
}
- else if (vr.getFormat() == ImageFormat.OVA){
+ else if (vr.getFormat() == ImageFormat.OVA) {
if (vrData.getChainInfo() != null) {
vs = ApiDBUtils.getVolumeStatistics(vrData.getChainInfo());
}
diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java
index 3937bd9..43aecde 100644
--- a/server/src/main/java/com/cloud/server/StatsCollector.java
+++ b/server/src/main/java/com/cloud/server/StatsCollector.java
@@ -928,13 +928,8 @@ public class StatsCollector extends ManagerBase implements ComponentMethodInterc
for (StoragePoolVO pool : pools) {
List<VolumeVO> volumes = _volsDao.findByPoolId(pool.getId(), null);
- List<String> volumeLocators = new ArrayList<String>();
for (VolumeVO volume : volumes) {
- if (volume.getFormat() == ImageFormat.QCOW2 || volume.getFormat() == ImageFormat.VHD) {
- volumeLocators.add(volume.getPath());
- } else if (volume.getFormat() == ImageFormat.OVA) {
- volumeLocators.add(volume.getChainInfo());
- } else {
+ if (volume.getFormat() != ImageFormat.QCOW2 && volume.getFormat() != ImageFormat.VHD && volume.getFormat() != ImageFormat.OVA) {
s_logger.warn("Volume stats not implemented for this format type " + volume.getFormat());
break;
}
@@ -943,15 +938,14 @@ public class StatsCollector extends ManagerBase implements ComponentMethodInterc
Map<String, VolumeStatsEntry> volumeStatsByUuid;
if (pool.getScope() == ScopeType.ZONE) {
volumeStatsByUuid = new HashMap<>();
- for (final Cluster cluster : _clusterDao.listByZoneId(pool.getDataCenterId())) {
- final Map<String, VolumeStatsEntry> volumeStatsForCluster = _userVmMgr.getVolumeStatistics(cluster.getId(), pool.getUuid(), pool.getPoolType(),
- volumeLocators, StatsTimeout.value());
+ for (final Cluster cluster : _clusterDao.listClustersByDcId(pool.getDataCenterId())) {
+ final Map<String, VolumeStatsEntry> volumeStatsForCluster = _userVmMgr.getVolumeStatistics(cluster.getId(), pool.getUuid(), pool.getPoolType(), StatsTimeout.value());
if (volumeStatsForCluster != null) {
volumeStatsByUuid.putAll(volumeStatsForCluster);
}
}
} else {
- volumeStatsByUuid = _userVmMgr.getVolumeStatistics(pool.getClusterId(), pool.getUuid(), pool.getPoolType(), volumeLocators, StatsTimeout.value());
+ volumeStatsByUuid = _userVmMgr.getVolumeStatistics(pool.getClusterId(), pool.getUuid(), pool.getPoolType(), StatsTimeout.value());
}
if (volumeStatsByUuid != null) {
for (final Map.Entry<String, VolumeStatsEntry> entry : volumeStatsByUuid.entrySet()) {
diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java b/server/src/main/java/com/cloud/vm/UserVmManager.java
index 1fe3f4d..95e1a41 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManager.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManager.java
@@ -85,7 +85,7 @@ public interface UserVmManager extends UserVmService {
HashMap<Long, List<VmDiskStatsEntry>> getVmDiskStatistics(long hostId, String hostName, List<Long> vmIds);
- HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, List<String> volumeLocator, int timout);
+ HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, int timout);
boolean deleteVmGroup(long groupId);
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 558b980..2a9bb2e 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -27,6 +27,7 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
@@ -96,6 +97,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
@@ -1882,45 +1884,46 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
}
@Override
- public HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, List<String> volumeLocators, int timeout) {
+ public HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, int timeout) {
List<HostVO> neighbors = _resourceMgr.listHostsInClusterByStatus(clusterId, Status.Up);
StoragePoolVO storagePool = _storagePoolDao.findPoolByUUID(poolUuid);
- for (HostVO neighbor : neighbors) {
- // apply filters:
- // - managed storage
- // - local storage
- if (storagePool.isManaged() || storagePool.isLocal()) {
-
- volumeLocators = getVolumesByHost(neighbor, storagePool);
+ HashMap<String, VolumeStatsEntry> volumeStatsByUuid = new HashMap<>();
- }
+ for (HostVO neighbor : neighbors) {
// - zone wide storage for specific hypervisortypes
- if (ScopeType.ZONE.equals(storagePool.getScope()) && storagePool.getHypervisor() != neighbor.getHypervisorType()) {
+ if ((ScopeType.ZONE.equals(storagePool.getScope()) && storagePool.getHypervisor() != neighbor.getHypervisorType())) {
// skip this neighbour if their hypervisor type is not the same as that of the store
continue;
}
- GetVolumeStatsCommand cmd = new GetVolumeStatsCommand(poolType, poolUuid, volumeLocators);
+ List<String> volumeLocators = getVolumesByHost(neighbor, storagePool);
+ if (!CollectionUtils.isEmpty(volumeLocators)) {
- if (timeout > 0) {
- cmd.setWait(timeout/1000);
- }
+ GetVolumeStatsCommand cmd = new GetVolumeStatsCommand(poolType, poolUuid, volumeLocators);
- Answer answer = _agentMgr.easySend(neighbor.getId(), cmd);
+ if (timeout > 0) {
+ cmd.setWait(timeout/1000);
+ }
+
+ Answer answer = _agentMgr.easySend(neighbor.getId(), cmd);
- if (answer instanceof GetVolumeStatsAnswer){
- GetVolumeStatsAnswer volstats = (GetVolumeStatsAnswer)answer;
- return volstats.getVolumeStats();
+ if (answer instanceof GetVolumeStatsAnswer){
+ GetVolumeStatsAnswer volstats = (GetVolumeStatsAnswer)answer;
+ if (volstats.getVolumeStats() != null) {
+ volumeStatsByUuid.putAll(volstats.getVolumeStats());
+ }
+ }
}
}
- return null;
+ return volumeStatsByUuid.size() > 0 ? volumeStatsByUuid : null;
}
private List<String> getVolumesByHost(HostVO host, StoragePool pool){
- List<UserVmVO> vmsPerHost = _vmDao.listByHostId(host.getId());
+ List<VMInstanceVO> vmsPerHost = _vmInstanceDao.listByHostId(host.getId());
return vmsPerHost.stream()
- .flatMap(vm -> _volsDao.findByInstanceIdAndPoolId(vm.getId(),pool.getId()).stream().map(vol -> vol.getPath()))
+ .flatMap(vm -> _volsDao.findByInstanceIdAndPoolId(vm.getId(),pool.getId()).stream().map(vol ->
+ vol.getState() == Volume.State.Ready ? (vol.getFormat() == ImageFormat.OVA ? vol.getChainInfo() : vol.getPath()) : null).filter(Objects::nonNull))
.collect(Collectors.toList());
}