Allon Mureinik has posted comments on this change. Change subject: engine: Only iSCSI storage connections are viable for iSCSI Bond ......................................................................
Patch Set 3: Code-Review+1 (4 comments) see inline http://gerrit.ovirt.org/#/c/23720/3/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAO.java: Line 65: * @param storageType Line 66: * @return Line 67: */ Line 68: List<StorageServerConnections> getConnectableStorageConnectionsByStorageType(Guid storagePoolId, Line 69: StorageType storageType); This doesn't seem the way the formatter does things - please check Line 70: Line 71: /** Line 72: * Retrieves all connections for the specified volume group. Line 73: * http://gerrit.ovirt.org/#/c/23720/3/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: } I'd move this "if" inside the stored procedure, but I guess it's a matter of taste. Line 69: Line 70: return getCallsHandler().executeReadList(storedProcedure, Line 71: mapper, Line 72: getCustomMapSqlParameterSource() http://gerrit.ovirt.org/#/c/23720/3/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StorageServerConnectionDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StorageServerConnectionDAOTest.java: Line 106: assertNotNull(conns); Line 107: assertEquals(2, conns.size()); Line 108: Line 109: for (StorageServerConnections conn : conns) { Line 110: assertTrue(conn.getstorage_type() == StorageType.ISCSI); please user assertEquals Line 111: } Line 112: } Line 113: Line 114: @Test Line 119: assertNotNull(conns); Line 120: assertEquals(1, conns.size()); Line 121: Line 122: for (StorageServerConnections conn : conns) { Line 123: assertTrue(conn.getstorage_type() == StorageType.NFS); please user assertEquals Line 124: } Line 125: } Line 126: Line 127: /** -- 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: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[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
