Skip to content

Commit 8afb451

Browse files
slavkapslavkap
authored
fix NPE in volumes statistics (#4388)
1 parent 963d603 commit 8afb451

4 files changed

Lines changed: 31 additions & 37 deletions

File tree

‎server/src/main/java/com/cloud/api/query/ViewResponseHelper.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,10 @@ public static List<VolumeResponse> createVolumeResponse(ResponseView view, Volum
282282
vrDataList.put(vr.getId(), vrData);
283283

284284
VolumeStats vs = null;
285-
if (vr.getFormat() == ImageFormat.QCOW2) {
286-
vs = ApiDBUtils.getVolumeStatistics(vrData.getId());
287-
}
288-
else if (vr.getFormat() == ImageFormat.VHD){
285+
if (vr.getFormat() == ImageFormat.VHD || vr.getFormat() == ImageFormat.QCOW2) {
289286
vs = ApiDBUtils.getVolumeStatistics(vrData.getPath());
290287
}
291-
else if (vr.getFormat() == ImageFormat.OVA){
288+
else if (vr.getFormat() == ImageFormat.OVA) {
292289
if (vrData.getChainInfo() != null) {
293290
vs = ApiDBUtils.getVolumeStatistics(vrData.getChainInfo());
294291
}

‎server/src/main/java/com/cloud/server/StatsCollector.java‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -928,13 +928,8 @@ protected void runInContext() {
928928

929929
for (StoragePoolVO pool : pools) {
930930
List<VolumeVO> volumes = _volsDao.findByPoolId(pool.getId(), null);
931-
List<String> volumeLocators = new ArrayList<String>();
932931
for (VolumeVO volume : volumes) {
933-
if (volume.getFormat() == ImageFormat.QCOW2 || volume.getFormat() == ImageFormat.VHD) {
934-
volumeLocators.add(volume.getPath());
935-
} else if (volume.getFormat() == ImageFormat.OVA) {
936-
volumeLocators.add(volume.getChainInfo());
937-
} else {
932+
if (volume.getFormat() != ImageFormat.QCOW2 && volume.getFormat() != ImageFormat.VHD && volume.getFormat() != ImageFormat.OVA) {
938933
s_logger.warn("Volume stats not implemented for this format type " + volume.getFormat());
939934
break;
940935
}
@@ -943,15 +938,14 @@ protected void runInContext() {
943938
Map<String, VolumeStatsEntry> volumeStatsByUuid;
944939
if (pool.getScope() == ScopeType.ZONE) {
945940
volumeStatsByUuid = new HashMap<>();
946-
for (final Cluster cluster : _clusterDao.listByZoneId(pool.getDataCenterId())) {
947-
final Map<String, VolumeStatsEntry> volumeStatsForCluster = _userVmMgr.getVolumeStatistics(cluster.getId(), pool.getUuid(), pool.getPoolType(),
948-
volumeLocators, StatsTimeout.value());
941+
for (final Cluster cluster : _clusterDao.listClustersByDcId(pool.getDataCenterId())) {
942+
final Map<String, VolumeStatsEntry> volumeStatsForCluster = _userVmMgr.getVolumeStatistics(cluster.getId(), pool.getUuid(), pool.getPoolType(), StatsTimeout.value());
949943
if (volumeStatsForCluster != null) {
950944
volumeStatsByUuid.putAll(volumeStatsForCluster);
951945
}
952946
}
953947
} else {
954-
volumeStatsByUuid = _userVmMgr.getVolumeStatistics(pool.getClusterId(), pool.getUuid(), pool.getPoolType(), volumeLocators, StatsTimeout.value());
948+
volumeStatsByUuid = _userVmMgr.getVolumeStatistics(pool.getClusterId(), pool.getUuid(), pool.getPoolType(), StatsTimeout.value());
955949
}
956950
if (volumeStatsByUuid != null) {
957951
for (final Map.Entry<String, VolumeStatsEntry> entry : volumeStatsByUuid.entrySet()) {

‎server/src/main/java/com/cloud/vm/UserVmManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public interface UserVmManager extends UserVmService {
8585

8686
HashMap<Long, List<VmDiskStatsEntry>> getVmDiskStatistics(long hostId, String hostName, List<Long> vmIds);
8787

88-
HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, List<String> volumeLocator, int timout);
88+
HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, int timout);
8989

9090
boolean deleteVmGroup(long groupId);
9191

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.List;
2828
import java.util.Map;
2929
import java.util.Map.Entry;
30+
import java.util.Objects;
3031
import java.util.Set;
3132
import java.util.UUID;
3233
import java.util.concurrent.ConcurrentHashMap;
@@ -96,6 +97,7 @@
9697
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
9798
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
9899
import org.apache.commons.codec.binary.Base64;
100+
import org.apache.commons.collections.CollectionUtils;
99101
import org.apache.commons.collections.MapUtils;
100102
import org.apache.commons.lang3.StringUtils;
101103
import org.apache.log4j.Logger;
@@ -1882,45 +1884,46 @@ public HashMap<Long, VmStatsEntry> getVirtualMachineStatistics(long hostId, Stri
18821884
}
18831885

18841886
@Override
1885-
public HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, List<String> volumeLocators, int timeout) {
1887+
public HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, int timeout) {
18861888
List<HostVO> neighbors = _resourceMgr.listHostsInClusterByStatus(clusterId, Status.Up);
18871889
StoragePoolVO storagePool = _storagePoolDao.findPoolByUUID(poolUuid);
1888-
for (HostVO neighbor : neighbors) {
1889-
// apply filters:
1890-
// - managed storage
1891-
// - local storage
1892-
if (storagePool.isManaged() || storagePool.isLocal()) {
1893-
1894-
volumeLocators = getVolumesByHost(neighbor, storagePool);
1890+
HashMap<String, VolumeStatsEntry> volumeStatsByUuid = new HashMap<>();
18951891

1896-
}
1892+
for (HostVO neighbor : neighbors) {
18971893

18981894
// - zone wide storage for specific hypervisortypes
1899-
if (ScopeType.ZONE.equals(storagePool.getScope()) && storagePool.getHypervisor() != neighbor.getHypervisorType()) {
1895+
if ((ScopeType.ZONE.equals(storagePool.getScope()) && storagePool.getHypervisor() != neighbor.getHypervisorType())) {
19001896
// skip this neighbour if their hypervisor type is not the same as that of the store
19011897
continue;
19021898
}
19031899

1904-
GetVolumeStatsCommand cmd = new GetVolumeStatsCommand(poolType, poolUuid, volumeLocators);
1900+
List<String> volumeLocators = getVolumesByHost(neighbor, storagePool);
1901+
if (!CollectionUtils.isEmpty(volumeLocators)) {
19051902

1906-
if (timeout > 0) {
1907-
cmd.setWait(timeout/1000);
1908-
}
1903+
GetVolumeStatsCommand cmd = new GetVolumeStatsCommand(poolType, poolUuid, volumeLocators);
19091904

1910-
Answer answer = _agentMgr.easySend(neighbor.getId(), cmd);
1905+
if (timeout > 0) {
1906+
cmd.setWait(timeout/1000);
1907+
}
1908+
1909+
Answer answer = _agentMgr.easySend(neighbor.getId(), cmd);
19111910

1912-
if (answer instanceof GetVolumeStatsAnswer){
1913-
GetVolumeStatsAnswer volstats = (GetVolumeStatsAnswer)answer;
1914-
return volstats.getVolumeStats();
1911+
if (answer instanceof GetVolumeStatsAnswer){
1912+
GetVolumeStatsAnswer volstats = (GetVolumeStatsAnswer)answer;
1913+
if (volstats.getVolumeStats() != null) {
1914+
volumeStatsByUuid.putAll(volstats.getVolumeStats());
1915+
}
1916+
}
19151917
}
19161918
}
1917-
return null;
1919+
return volumeStatsByUuid.size() > 0 ? volumeStatsByUuid : null;
19181920
}
19191921

19201922
private List<String> getVolumesByHost(HostVO host, StoragePool pool){
1921-
List<UserVmVO> vmsPerHost = _vmDao.listByHostId(host.getId());
1923+
List<VMInstanceVO> vmsPerHost = _vmInstanceDao.listByHostId(host.getId());
19221924
return vmsPerHost.stream()
1923-
.flatMap(vm -> _volsDao.findByInstanceIdAndPoolId(vm.getId(),pool.getId()).stream().map(vol -> vol.getPath()))
1925+
.flatMap(vm -> _volsDao.findByInstanceIdAndPoolId(vm.getId(),pool.getId()).stream().map(vol ->
1926+
vol.getState() == Volume.State.Ready ? (vol.getFormat() == ImageFormat.OVA ? vol.getChainInfo() : vol.getPath()) : null).filter(Objects::nonNull))
19241927
.collect(Collectors.toList());
19251928
}
19261929

0 commit comments

Comments
 (0)