Allon Mureinik has posted comments on this change. Change subject: core: Use storage helper to fetch (related to BZ882825) ......................................................................
Patch Set 3: (6 inline comments) see inline .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java Line 125: List<StorageServerConnections> connections) { Line 126: return true; Line 127: } Line 128: Line 129: public static Map<StorageType, List<StorageServerConnections>> filterLUNsByStorageType(LUNs lun) { I don't get this function. Can a LUN have several connections from different types? Line 130: Map<StorageType, List<StorageServerConnections>> storageConnectionsForStorageTypeMap = Line 131: new EnumMap<StorageType, List<StorageServerConnections>>(StorageType.class); Line 132: for (StorageServerConnections lunConnections : lun.getLunConnections()) { Line 133: StorageType storageType = lunConnections.getstorage_type(); Line 133: StorageType storageType = lunConnections.getstorage_type(); Line 134: if (!storageConnectionsForStorageTypeMap.containsKey(storageType)) { Line 135: storageConnectionsForStorageTypeMap.put(storageType, new LinkedList<StorageServerConnections>()); Line 136: } Line 137: storageConnectionsForStorageTypeMap.get(storageType).add(lunConnections); Just use MultiValueMapUtils Line 138: } Line 139: return storageConnectionsForStorageTypeMap; Line 140: } Line 141: .................................................... File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHelperBaseTest.java Line 16: Line 17: @Test Line 18: public void getLunConnectionsForFC() { Line 19: LUNs lun = new LUNs(); Line 20: ArrayList<StorageServerConnections> connections = new ArrayList<StorageServerConnections>(); can't this be declared as a List? Line 21: lun.setLunConnections(connections); Line 22: Map<StorageType, List<StorageServerConnections>> connectionsByType = Line 23: StorageHelperBase.filterLUNsByStorageType(lun); Line 24: assertTrue("Map of storage connections should be empty.", connectionsByType.isEmpty()); Line 26: Line 27: @Test Line 28: public void getLunConnectionsForISCSI() { Line 29: LUNs lun = new LUNs(); Line 30: ArrayList<StorageServerConnections> connections = new ArrayList<StorageServerConnections>(); this too Line 31: connections.add(new StorageServerConnections("Some LUN connection", Line 32: "id", Line 33: "iqn", Line 34: "password", Line 59: Line 60: @Test Line 61: public void getMixedLunConnections() { Line 62: LUNs lun = new LUNs(); Line 63: ArrayList<StorageServerConnections> connections = new ArrayList<StorageServerConnections>(); and here Line 64: connections.add(new StorageServerConnections("Some LUN connection", Line 65: "id", Line 66: "iqn", Line 67: "password", .................................................... Commit Message Line 8: Line 9: Added engine support for connecting LUN with multiple connections of different Line 10: storage types. Line 11: Line 12: Change-Id: Ida526dd9e08bc9881e65a529cb1b00aee0786af1 add Related-To: http://bugzila... -- To view, visit http://gerrit.ovirt.org/10141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida526dd9e08bc9881e65a529cb1b00aee0786af1 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches