Daniel Erez has posted comments on this change. Change subject: core: Add vdc query for attached Storage Domain ......................................................................
Patch Set 1: (12 comments) http://gerrit.ovirt.org/#/c/36447/1/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 25: public GetStorageDomainsWithAttachedStoragePoolGuidQuery(P parameters) { Line 26: super(parameters); Line 27: } Line 28: Line 29: private Guid vdsId; move members to top Line 30: protected List<StorageDomainStatic> storageDomainsWithAttachedStoragePoolId = new ArrayList<>(); Line 31: Line 32: public Guid setVdsForConnectStorage() { Line 33: vdsId = getParameters().getVdsId(); Line 26: super(parameters); Line 27: } Line 28: Line 29: private Guid vdsId; Line 30: protected List<StorageDomainStatic> storageDomainsWithAttachedStoragePoolId = new ArrayList<>(); why initialization is needed? Line 31: Line 32: public Guid setVdsForConnectStorage() { Line 33: vdsId = getParameters().getVdsId(); Line 34: if (vdsId == null) { Line 28: Line 29: private Guid vdsId; Line 30: protected List<StorageDomainStatic> storageDomainsWithAttachedStoragePoolId = new ArrayList<>(); Line 31: Line 32: public Guid setVdsForConnectStorage() { it's a bit weird that a setter returns a value... rename to getVdsForConnectStorage Line 33: vdsId = getParameters().getVdsId(); Line 34: if (vdsId == null) { Line 35: // Get a Host which is at UP state to connect to the Storage Domain. Line 36: List<VDS> hosts = Line 48: Line 49: @Override Line 50: protected void executeQueryCommand() { Line 51: if (setVdsForConnectStorage() == null) { Line 52: return; set the return value to an empty in this case Line 53: } Line 54: storageDomainsWithAttachedStoragePoolId = filterAttachedStorageDomains(); Line 55: getQueryReturnValue().setReturnValue(storageDomainsWithAttachedStoragePoolId); Line 56: } Line 66: } Line 67: Line 68: storageDomainsWithAttachedStoragePoolId = getAttachedStorageDomains(connectedStorageDomainsToVds); Line 69: for (StorageDomain storageDomain : connectedStorageDomainsToVds) { Line 70: if (!disConnectStorageDomain(storageDomain)) { /s/disConnect/disconnect Line 71: log.warn("Could not disconnect Storage Domain {} from VDS '{}'. ", storageDomain.getName(), getVdsId()); Line 72: } Line 73: } Line 74: return storageDomainsWithAttachedStoragePoolId; Line 81: protected List<StorageDomainStatic> getAttachedStorageDomains(List<StorageDomain> storageDomains) { Line 82: VDSReturnValue vdsReturnValue = null; Line 83: List<StorageDomainStatic> storageDomainsWithAttachedStoragePoolId = new ArrayList<>(); Line 84: Line 85: // Go over the list of Storage Domains and try to get the Storage Domain info to check if it attached to another s/it/it is Line 86: // Storage Pool Line 87: for (StorageDomain storageDomain : storageDomains) { Line 88: try { Line 89: vdsReturnValue = Line 106: return storageDomainsWithAttachedStoragePoolId; Line 107: } Line 108: Line 109: protected VDSBrokerFrontend getVdsBroker() { Line 110: return Backend.getInstance().getResourceManager(); use getBackend Line 111: } Line 112: Line 113: protected VDSReturnValue runVdsCommand(VDSCommandType commandType, VDSParametersBase parameters) Line 114: throws VdcBLLException { Line 120: .getItem(storageDomain.getStorageType()) Line 121: .connectStorageToDomainByVdsId(storageDomain, getVdsId()); Line 122: } Line 123: Line 124: protected boolean disConnectStorageDomain(StorageDomain storageDomain) { s/disConnect/disconnect Line 125: return StorageHelperDirector.getInstance() Line 126: .getItem(storageDomain.getStorageType()) Line 127: .disconnectStorageFromDomainByVdsId(storageDomain, getVdsId()); Line 128: } Line 131: if (storageDomain != null) { Line 132: log.error("Could not get Storage Domain info for Storage Domain (name:'{}', id:'{}') with VDS '{}'. ", Line 133: storageDomain.getName(), Line 134: storageDomain.getId(), Line 135: getVdsId()); should probably check geVdsId() nullity as well Line 136: } else { Line 137: log.error("Could not get Storage Domain info with VDS '{}'. ", getVdsId()); Line 138: } Line 139: } http://gerrit.ovirt.org/#/c/36447/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/StorageDomainsAndStoragePoolIdQueryParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/StorageDomainsAndStoragePoolIdQueryParameters.java: Line 11: private List<StorageDomain> storageDomainList; Line 12: private StorageServerConnections storageServerConnection; Line 13: private Guid vdsId; Line 14: Line 15: public Guid getVdsId() { convention - move getters/setters to the bottom Line 16: return vdsId; Line 17: } Line 18: Line 19: public void setVdsId(Guid vdsId) { Line 38: Line 39: public StorageDomainsAndStoragePoolIdQueryParameters() { Line 40: } Line 41: Line 42: public StorageDomainsAndStoragePoolIdQueryParameters(List<StorageDomain> storageDomainList, Guid storagePoolId, Guid vdsId) { consider adding a ctr without vdsId (for attach usage) Line 43: super(storagePoolId); Line 44: this.storageDomainList = storageDomainList; Line 45: this.vdsId = vdsId; Line 46: } http://gerrit.ovirt.org/#/c/36447/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java: Line 373: GetAttachedClustersByClusterPolicyId, Line 374: GetAffinityGroupById, Line 375: GetAffinityGroupsByClusterId, Line 376: GetAffinityGroupsByVmId, Line 377: GetStorageDomainsWithAttachedStoragePoolGuid, this is Scheduling section, move it to a more suitable location... Line 378: Line 379: GetAllDisksPartialDataByVmId(VdcQueryAuthType.User), Line 380: GetVmTemplateCount, Line 381: -- To view, visit http://gerrit.ovirt.org/36447 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I749cad223d90dcc8d193765ed842d7e062ad566c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
