Maor Lipchuk has posted comments on this change. Change subject: core: Add a query for attached Storage Domains ......................................................................
Patch Set 16: (4 comments) http://gerrit.ovirt.org/#/c/34233/16/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 43: // Get a Host which is at UP state to connect to the Storage Domain. Line 44: List<VDS> hosts = Line 45: getDbFacade().getVdsDao().getAllForStoragePoolAndStatus(getParameters().getId(), VDSStatus.Up); Line 46: if (!hosts.isEmpty()) { Line 47: vdsId = hosts.get(new Random().nextInt(hosts.size())).getId(); > This should be removed Eventually I kept this but I prefer not to use RandomUtils for consistency issues. also RandomUtils.instance() is not implemented, it should be used differently then what was mentioned Line 48: log.info("vds id '{}' was chosen to fetch the Storage domain info", vdsId); Line 49: } else { Line 50: log.warn("There is no available vds in UP state to fetch the Storage domain info from VDSM"); Line 51: return; Line 69: private boolean isFileDomain() { Line 70: return getParameters().getStorageServerConnection() != null; Line 71: } Line 72: Line 73: private List<StorageDomainStatic> getBlockStorageDomains() { > the query shouldn't be aware that it's executed by the UI, tomorrow it can ok, I will add a disconnect Line 74: VDSReturnValue vdsReturnValue = null; Line 75: List<StorageDomain> storageDomains = getParameters().getStorageDomainList(); Line 76: List<StorageDomainStatic> storageDomainsWithAttachedStoragePoolId = new ArrayList<>(); Line 77: Line 88: } catch (RuntimeException e) { Line 89: logErrorMessage(storageDomain); Line 90: continue; Line 91: } Line 92: if (!vdsReturnValue.getSucceeded()) { > ? in one you return null and in one you continue. missed that, I will change this to continue Line 93: logErrorMessage(storageDomain); Line 94: return null; Line 95: } Line 96: Line 128: } Line 129: } Line 130: Line 131: getBackend().runInternalAction(VdcActionType.DisconnectStorageServerConnection, Line 132: new StorageServerConnectionParametersBase(getParameters().getStorageServerConnection(), vdsId)); > 1. in some actions you use the storagehelper and in some you are not, we sh Will be changed as discussed, GUI should be refactored, the Query should make connect and disconnect right after. For block Storage Domain the query should do only connect without disconnect Line 133: } Line 134: return storageDomains; Line 135: } Line 136: -- 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: 16 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
