fix NPE in volumes statistics

If volume is migrated between storage pools and "volume.stats.interval"
is set to more frequent interval for exmaple 30 sec.
NullPointerException is thrown, because the hypervisor is looking for
volume on storage pool from which the volume was migrated.
The statistics did not work right - before this fix only one cluster is
lists if we have more clusters from the same type (eg. KVM), and only
one host will return volume stats. Also all volumes are sent to that
host, but they could be on another host. And the logic does not work
correctly.
Also the response was not working correctly for QCOW2 images, because
it's looking for volume's uuid, but in the statistics CS keeps the path
(because of the migrated volumes which UUIDs are changed)

Author: slavkap

engine/schema/src/main/java/com/cloud/dc/dao/ClusterDaoImpl.java

@@ -81,7 +81,6 @@ public ClusterDaoImpl() {
 
         ZoneSearch = createSearchBuilder();
         ZoneSearch.and("dataCenterId", ZoneSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ);
-        ZoneSearch.groupBy(ZoneSearch.entity().getHypervisorType());
         ZoneSearch.done();
 
         AvailHyperSearch = createSearchBuilder();

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

@@ -287,10 +287,7 @@ 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){

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

@@ -16,6 +16,8 @@
 // under the License.
 package com.cloud.vm;
 
+import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
+
 import java.io.IOException;
 import java.io.StringReader;
 import java.io.UnsupportedEncodingException;
@@ -29,6 +31,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;
@@ -47,8 +50,6 @@
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 
-import com.cloud.exception.UnsupportedServiceException;
-import com.cloud.hypervisor.Hypervisor;
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.affinity.AffinityGroupService;
@@ -188,13 +189,15 @@
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.exception.StorageUnavailableException;
+import com.cloud.exception.UnsupportedServiceException;
 import com.cloud.exception.VirtualMachineMigrationException;
 import com.cloud.gpu.GPU;
 import com.cloud.ha.HighAvailabilityManager;
 import com.cloud.host.Host;
 import com.cloud.host.HostVO;
 import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorCapabilitiesVO;
 import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
@@ -328,8 +331,6 @@
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 
-import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
-
 public class UserVmManagerImpl extends ManagerBase implements UserVmManager, VirtualMachineGuru, UserVmService, Configurable {
     private static final Logger s_logger = Logger.getLogger(UserVmManagerImpl.class);
 
@@ -1930,18 +1931,13 @@ public HashMap<Long, VmStatsEntry> getVirtualMachineStatistics(long hostId, Stri
     public HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, String poolUuid, StoragePoolType poolType, List<String> volumeLocators, 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) {
+            volumeLocators = getVolumesByHost(neighbor, storagePool);
 
             // - 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()) || (volumeLocators == null || volumeLocators.size() == 0)) {
                 // skip this neighbour if their hypervisor type is not the same as that of the store
                 continue;
             }
@@ -1956,16 +1952,16 @@ public HashMap<String, VolumeStatsEntry> getVolumeStatistics(long clusterId, Str
 
             if (answer instanceof GetVolumeStatsAnswer){
                 GetVolumeStatsAnswer volstats = (GetVolumeStatsAnswer)answer;
-                return volstats.getVolumeStats();
+                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());
         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.getPath() : null).filter(Objects::nonNull))
                 .collect(Collectors.toList());
     }