Sergey Gotliv has posted comments on this change. Change subject: engine: Setup multiple iscsi sessions with the iscsi target ......................................................................
Patch Set 1: (5 comments) http://gerrit.ovirt.org/#/c/23198/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDao.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDao.java: Line 166: * @return all labels defined for the data-center's networks Line 167: */ Line 168: Set<String> getAllNetworkLabelsForDataCenter(Guid id); Line 169: Line 170: List<VdsNetworkInterface> getIscsiInitiatorsByVdsIdAndStorageTargetId(Guid vdsId, String storageTargetId); > please add java doc Done http://gerrit.ovirt.org/#/c/23198/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java: Line 247: return labels; Line 248: Line 249: } Line 250: Line 251: @Override > Please add unit test (should you add data to the fixtures.xml ?) Yes, I have to update fixtures.xml Line 252: public List<VdsNetworkInterface> getIscsiInitiatorsByVdsIdAndStorageTargetId(Guid vdsId, String storageTargetId) { Line 253: return getCallsHandler().executeReadList("GetIscsiInitiatorsByVdsIdAndStorageTargetId", Line 254: vdsNetworkInterfaceRowMapper, Line 255: getCustomMapSqlParameterSource().addValue("vds_id", vdsId).addValue("target_id", storageTargetId)); Line 248: Line 249: } Line 250: Line 251: @Override Line 252: public List<VdsNetworkInterface> getIscsiInitiatorsByVdsIdAndStorageTargetId(Guid vdsId, String storageTargetId) { > s/vdsId/hostId Done Line 253: return getCallsHandler().executeReadList("GetIscsiInitiatorsByVdsIdAndStorageTargetId", Line 254: vdsNetworkInterfaceRowMapper, Line 255: getCustomMapSqlParameterSource().addValue("vds_id", vdsId).addValue("target_id", storageTargetId)); Line 256: } http://gerrit.ovirt.org/#/c/23198/1/packaging/dbscripts/network_sp.sql File packaging/dbscripts/network_sp.sql: Line 1243: END; $procedure$ Line 1244: LANGUAGE plpgsql; Line 1245: Line 1246: Line 1247: Create or replace FUNCTION GetIscsiInitiatorsByVdsIdAndStorageTargetId(v_vds_id UUID, v_target_id varchar(50)) RETURNS SETOF vds_interface_view STABLE > s/GetIscsiInitiatorsByVdsIdAndStorageTargetId/GetIscsiInitiatorsByHostIdAnd Done Line 1248: AS $procedure$ Line 1249: BEGIN Line 1250: RETURN QUERY SELECT vds_interface_view.* Line 1251: FROM vds_interface_view, Line 1248: AS $procedure$ Line 1249: BEGIN Line 1250: RETURN QUERY SELECT vds_interface_view.* Line 1251: FROM vds_interface_view, Line 1252: vds, > this query can be improved by replacing the join of 'vds' view with extensi I agree. Actually, I thought about another improvement - I think that vds_interface_view should contain network_id in addition to the network_name as it today. In that way I can save 2-3 joins and not only for that query. Line 1253: network_cluster, Line 1254: network, Line 1255: iscsi_bonds_networks_map, Line 1256: iscsi_bonds_storage_connections_map -- To view, visit http://gerrit.ovirt.org/23198 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I779f6dd95dfbfc2b74ad7ba3ce2271b7c9ad94db Gerrit-PatchSet: 1 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: Moti Asayag <masa...@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
