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