Maor Lipchuk has posted comments on this change.

Change subject: engine: Only iSCSI storage connections are viable for iSCSI Bond
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.ovirt.org/#/c/23720/2//COMMIT_MSG
Commit Message:

Line 4: Commit:     Sergey Gotliv <sgot...@redhat.com>
Line 5: CommitDate: 2014-01-30 13:31:49 +0200
Line 6: 
Line 7: engine: Only iSCSI storage connections are viable for iSCSI Bond
Line 8: 
Please add aditinal information such as:
* which DAO methods has been added and why
* Which query command has been added and why
What this patch intend to fix
Line 9: Change-Id: Iae30db6aa0c5d2989e9e31566d6555d70259799e


http://gerrit.ovirt.org/#/c/23720/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetConnectionsByDataCenterAndStorageTypeQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetConnectionsByDataCenterAndStorageTypeQuery.java:

Line 11:     }
Line 12: 
Line 13:     @Override
Line 14:     protected void executeQueryCommand() {
Line 15:         
getQueryReturnValue().setReturnValue(getDbFacade().getStorageServerConnectionDao()
The if condition implemented in the DAO should be here, or in the helper class
Line 16:                 
.getConnectableStorageConnectionsByStorageType(getParameters().getId(), 
getParameters().getStorageType()));
Line 17:     }


http://gerrit.ovirt.org/#/c/23720/2/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAODbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAODbFacadeImpl.java:

Line 64:         }  else if (storageType.isFileDomain()) {
Line 65:             storedProcedure = 
"GetConnectableFileStorageConnectionsByType";
Line 66:         }  else {
Line 67:             return Collections.emptyList();
Line 68:         }
DAO should not contain this logic, this should be at the Helper class
Line 69: 
Line 70:         return getCallsHandler().executeReadList(storedProcedure,
Line 71:                 mapper,
Line 72:                 getCustomMapSqlParameterSource()


http://gerrit.ovirt.org/#/c/23720/2/packaging/dbscripts/storages_san_sp.sql
File packaging/dbscripts/storages_san_sp.sql:

Line 554: Create or replace FUNCTION 
GetConnectableBlockStorageConnectionsByType(v_storage_pool_id UUID, 
v_storage_type integer)
Line 555: RETURNS SETOF storage_server_connections STABLE
Line 556:    AS $procedure$
Line 557: BEGIN
Line 558: RETURN QUERY SELECT distinct connection.*
why distinct?
Line 559:    FROM storage_server_connections connection, storage_domains 
domain, LUNs lun, LUN_storage_server_connection_map lun_connection
Line 560:    WHERE
Line 561:     connection.storage_type = v_storage_type and
Line 562:     connection.id = lun_connection.storage_server_connection and


Line 571: Create or replace FUNCTION 
GetConnectableFileStorageConnectionsByType(v_storage_pool_id UUID, 
v_storage_type integer)
Line 572: RETURNS SETOF storage_server_connections STABLE
Line 573:    AS $procedure$
Line 574: BEGIN
Line 575: RETURN QUERY SELECT distinct connection.*
why distinct?
Line 576:    FROM storage_server_connections connection, storage_domains domain
Line 577:    WHERE
Line 578:     connection.storage_type = v_storage_type and
Line 579:     connection.id = domain.storage and


-- 
To view, visit http://gerrit.ovirt.org/23720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae30db6aa0c5d2989e9e31566d6555d70259799e
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
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

Reply via email to