Skip to content

fix NPE in volumes statistics #4388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 8 commits into from
Oct 30, 2020
Merged

fix NPE in volumes statistics #4388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cff272a
fix NPE in volumes statistics
slavkap Oct 7, 2020
2f445df
list all virtual machines by host id
slavkap Oct 7, 2020
03a7837
Return groupBy hypervisor type when listing clusters by zone id
slavkap Oct 9, 2020
a82f4e3
Replace listByZoneId in VolumeStatsTask with listClustersByDcId
slavkap Oct 9, 2020
5c5f5f4
Removed unused arg and condition if format is OVA when listing volumes
slavkap Oct 9, 2020
fcb9979
Formating the brackets
slavkap Oct 12, 2020
5008846
Won't get volumes if the hypervisor type is not the same of the store
slavkap Oct 29, 2020
93214ff
Add import for CollectionUtils
slavkap Oct 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,10 @@ public static List<VolumeResponse> createVolumeResponse(ResponseView view, Volum
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());
}
Expand Down
14 changes: 4 additions & 10 deletions server/src/main/java/com/cloud/server/StatsCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,13 +928,8 @@ protected void runInContext() {

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;
}
Expand All @@ -943,15 +938,14 @@ protected void runInContext() {
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()) {
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/vm/UserVmManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
45 changes: 24 additions & 21 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -96,6 +97,7 @@
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;
Expand Down Expand Up @@ -1882,45 +1884,46 @@ public HashMap<Long, VmStatsEntry> getVirtualMachineStatistics(long hostId, Stri
}

@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());
}

Expand Down