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

Reply via email to