Yair Zaslavsky has posted comments on this change.
Change subject: engine: connectStorageServer is not sent for inactive domains
before connectStoragePool
......................................................................
Patch Set 3: (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)
I saw the previous comments,
How about -
replace all the name (not just the prefix) with
GetAllConncetableStorageSeverConnection ? (I also don't like the mentioning of
the statuses at the names, so took me time to think of a better name :) )
Line 542: RETURNS SETOF storage_server_connections
Line 543: AS $procedure$
Line 544: BEGIN
Line 545: RETURN QUERY SELECT distinct storage_server_connections.*
Line 547: LUN_storage_server_connection_map LUN_storage_server_connection_map
Line 548: INNER JOIN LUNs ON LUN_storage_server_connection_map.LUN_id =
LUNs.LUN_id
Line 549: INNER JOIN storage_domains ON LUNs.volume_group_id =
storage_domains.storage
Line 550: INNER JOIN storage_server_connections ON
LUN_storage_server_connection_map.storage_server_connection =
storage_server_connections.id
Line 551: WHERE (storage_domains.storage_pool_id = v_storage_pool_id
and storage_domains.status in(0,3,4))
I agree that filtering should be done at stored procedure - this is more
efficient.
If we provide a meaningful name as I suggested it will be more clear to the
developers what the function is used for.
Line 552: UNION
Line 553: SELECT distinct storage_server_connections.*
Line 554: FROM storage_server_connections
Line 555: INNER JOIN storage_domains ON storage_server_connections.id =
storage_domains.storage
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectHostToStoragePooServerCommandBase.java
Line 72: List<StorageDomain> exportDomains =
Line 73: getStorageDomainsByStoragePoolId(allDomains,
StorageDomainType.ImportExport);
Line 74:
Line 75: Set<StorageServerConnections> connections = new
HashSet<StorageServerConnections>(
Line 76:
DbFacade.getInstance().getStorageServerConnectionDao().getActiveUnknownInActiveForStoragePool(getStoragePool().getId()));
Comment is similar to sp, No need to go over all the code (just reminding it
once) and reminding it again.
Line 77: if (isoDomains.size() != 0) {
Line 78: _isoType = isoDomains.get(0).getStorageType();
Line 79: Set<StorageServerConnections> isoConnections =
Line 80: new HashSet<StorageServerConnections>(
--
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: 3
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
