skattoju4 commented on issue #3486: filter volumes by host when refreshing stats URL: https://github.com/apache/cloudstack/pull/3486#issuecomment-513763347 yes you are right. better to factor it out... On Mon., Jul. 22, 2019, 7:29 a.m. dahn, <[email protected]> wrote: > *@DaanHoogland* approved this pull request. > ------------------------------ > > In server/src/main/java/com/cloud/vm/UserVmManagerImpl.java > <https://github.com/apache/cloudstack/pull/3486#discussion_r305795840>: > > > List<HostVO> neighbors = _resourceMgr.listHostsInClusterByStatus(clusterId, Status.Up); > for (HostVO neighbor : neighbors) { > - GetVolumeStatsCommand cmd = new GetVolumeStatsCommand(poolType, poolUuid, volumeLocator); > + StoragePool storagePool = _storagePoolDao.findPoolByUUID(poolUuid); > + List<String> volumeLocatorsForHost; > + if (storagePool.isManaged()) { > + List<UserVmVO> vmsPerHost = _vmDao.listByHostId(neighbor.getId()); > > looks good but can you isolate the body of the if in a method like filter > hostsToQueryForVmVolumes() (and maybe add a test)? Please don't kill me but > it is kind of hard to read like this. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/apache/cloudstack/pull/3486?email_source=notifications&email_token=AK5V4KUUUIZGCO5PFRUMVQDQAWKYBA5CNFSM4IBRW5M2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB7D46SA#pullrequestreview-264752968>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AK5V4KWWJPKIFE5FMEN7GLTQAWKYBANCNFSM4IBRW5MQ> > . >
---------------------------------------------------------------- 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] With regards, Apache Git Services
