Michael Kublin has posted comments on this change.
Change subject: engine: connectStorageServer is not sent for inactive domains
before connectStoragePool
......................................................................
Patch Set 2: (3 inline comments)
....................................................
File backend/manager/dbscripts/storages_san_sp.sql
Line 537: LANGUAGE plpgsql;
Line 538:
Line 539:
Line 540:
Line 541: Create or replace FUNCTION
Getstorage_server_connectionsActiveUnknownInactiveByPoolId(v_storage_pool_id
UUID)
Domains has statuses : storage_domains.status . Status of domains
Line 542: RETURNS SETOF storage_server_connections
Line 543: AS $procedure$
Line 544: BEGIN
Line 545: RETURN QUERY SELECT distinct storage_server_connections.*
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectHostToStoragePooServerCommandBase.java
Line 79: Set<StorageServerConnections> isoConnections =
Line 80: new HashSet<StorageServerConnections>(
Line 81:
StorageHelperDirector.getInstance().getItem(getIsoType())
Line 82:
.getStorageServerConnectionsByDomain(isoDomains.get(0).getStorageStaticData()));
Line 83: if (_isoType != getStoragePool().getstorage_pool_type()) {
How this related to patch. How this comment is related to patch idea: wrong
filtering of the storage connections?
Line 84: connections.removeAll(isoConnections);
Line 85: } else {
Line 86: isoConnections.removeAll(connections);
Line 87: }
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAO.java
Line 37: */
Line 38: List<StorageServerConnections> getAll();
Line 39:
Line 40: /**
Line 41: * Retrieves all Active, Unknown , InActive connections for the
specified storage pool.
I will not . We have not Inactive status, we have InActive status, comment
which is using a constant that is not defined in the code is wrong by
definition and also it is bad practice. If you will try to search around a code
"Inactive" storage domain status you will not find anything, because such
status does not exists. Comment should help to understand a code and to help
find a reference to other parts of code, beauty is not purpose of comments
Line 42: *
Line 43: * @param pool
Line 44: * the storage pool
Line 45: * @return the list of connections
--
To view, visit http://gerrit.ovirt.org/12805
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb3710bb83ef81485715577ae939014ffcae693f
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
