slavkap commented on a change in pull request #4388:
URL: https://github.com/apache/cloudstack/pull/4388#discussion_r502203876
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -1956,16 +1952,16 @@ private boolean upgradeRunningVirtualMachine(Long vmId,
Long newServiceOfferingI
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());
+ 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.getPath() :
null).filter(Objects::nonNull))
Review comment:
@sureshanaparti I was just follow the original logic and just added to
get only the volumes with status "Ready". I will check for the rest of the
formats, but I will be not able to do tests for them
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -1930,18 +1931,13 @@ private boolean upgradeRunningVirtualMachine(Long vmId,
Long newServiceOfferingI
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()) {
Review comment:
The logic was correct for managed/local storages, but for unmanaged I've
descried in the description what is happening. I have tested with NFS unmanaged
storage and StorPool as well, and it should get the volumes only on the current
host
##########
File path: 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());
Review comment:
@sureshanaparti I have tested it. My setup is 2 KVM cluster with 2 hosts
on each. Before the change when listing hypervisors:
> list hypervisors zoneid=307c2b69-86eb-451e-8517-6113c59648c4
> { "count": 1, "hypervisor": [ { "name": "KVM" } ] }
with the change when listing hypervisors:
> list hypervisors zoneid=307c2b69-86eb-451e-8517-6113c59648c4
> { "count": 2, "hypervisor": [ { "name": "KVM" }, { "name": "KVM" }
] }
##########
File path: 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());
Review comment:
ok than, I wasn't sure that it should return only 1 KVM if we have for
example 3 clusters of them. I will implement another method, which gets all
clusters in the zone. It's needed for StatsCollector to list all clusters in
the zone, otherwise we get only one cluster of a hypervisor type.
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -1956,16 +1952,16 @@ private boolean upgradeRunningVirtualMachine(Long vmId,
Long newServiceOfferingI
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());
+ 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.getPath() :
null).filter(Objects::nonNull))
Review comment:
yes, I will add to check if it's OVA
----------------------------------------------------------------
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]
