Tal Nisan has posted comments on this change. Change subject: core: Add a query for attached Storage Domains ......................................................................
Patch Set 8: (3 comments) http://gerrit.ovirt.org/#/c/34233/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetStorageDomainsWithAttachedStoragePoolGuidQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetStorageDomainsWithAttachedStoragePoolGuidQuery.java: Line 33: // Get a Host which is at UP state to connect to the Storage Domain. Line 34: List<VDS> hosts = Line 35: getDbFacade().getVdsDao().getAllForStoragePoolAndStatus(getParameters().getId(), VDSStatus.Up); Line 36: if (!hosts.isEmpty()) { Line 37: vdsId = RandomUtils.instance().pickRandom(hosts).getId(); > You should probably log which host you're using I agree, nice thinking btw with picking a random one from the collection but perhaps it will be worth to try and avoid using the SPM unless it's the only one in UP status to avoid adding load on a host that mostly has the most load? Line 38: } else { Line 39: return; Line 40: } Line 41: Line 35: getDbFacade().getVdsDao().getAllForStoragePoolAndStatus(getParameters().getId(), VDSStatus.Up); Line 36: if (!hosts.isEmpty()) { Line 37: vdsId = RandomUtils.instance().pickRandom(hosts).getId(); Line 38: } else { Line 39: return; Should we use getQueryReturnValue().setSucceeded(false) here so we will at least have an indication that the query was failed? Line 40: } Line 41: Line 42: // Go over the list of Storage Domains and try to get the Storage Domain info to check if it attached to another Line 43: // Storage Pool Line 43: // Storage Pool Line 44: List<StorageDomain> storageDomains = getParameters().getStorageDomainList(); Line 45: for (StorageDomain storageDomain : storageDomains) { Line 46: try { Line 47: boolean connectResult = connectStorageDomain(storageDomain); We don't need to disconnect at some point? Line 48: if (connectResult) { Line 49: vdsReturnValue = Line 50: getVdsBroker().RunVdsCommand(VDSCommandType.HSMGetStorageDomainInfo, Line 51: new HSMGetStorageDomainInfoVDSCommandParameters(vdsId, storageDomain.getId())); -- To view, visit http://gerrit.ovirt.org/34233 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id6fc7a2d722611b1bccecee9868223ff437596ed Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
