Maor Lipchuk has posted comments on this change.

Change subject: core: Use storage helper to fetch (related to BZ882825)
......................................................................


Patch Set 1: (6 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java
Line 19
Line 20
Line 21
Line 22
Line 23
Done


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java
Line 124:     public boolean isConnectSucceeded(Map<String, String> returnValue,
Line 125:             List<storage_server_connections> connections) {
Line 126:         return true;
Line 127:     }
Line 128: 
Will change.
Line 129:     public static Map<StorageType, List<storage_server_connections>> 
filterLUNsByStorageType(LUNs lun) {
Line 130:         Map<StorageType, List<storage_server_connections>> 
storageConnectionsForStorageTypeMap =
Line 131:                 new HashMap<StorageType, 
List<storage_server_connections>>();
Line 132:         for (storage_server_connections lunConnections : 
lun.getLunConnections()) {


Line 125:             List<storage_server_connections> connections) {
Line 126:         return true;
Line 127:     }
Line 128: 
Line 129:     public static Map<StorageType, List<storage_server_connections>> 
filterLUNsByStorageType(LUNs lun) {
done.
Line 130:         Map<StorageType, List<storage_server_connections>> 
storageConnectionsForStorageTypeMap =
Line 131:                 new HashMap<StorageType, 
List<storage_server_connections>>();
Line 132:         for (storage_server_connections lunConnections : 
lun.getLunConnections()) {
Line 133:             StorageType storageType = 
lunConnections.getstorage_type();


Line 127:     }
Line 128: 
Line 129:     public static Map<StorageType, List<storage_server_connections>> 
filterLUNsByStorageType(LUNs lun) {
Line 130:         Map<StorageType, List<storage_server_connections>> 
storageConnectionsForStorageTypeMap =
Line 131:                 new HashMap<StorageType, 
List<storage_server_connections>>();
See 
http://gerrit.ovirt.org/#/c/9724/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java.

 I prefer not to change the BE in this patch scope, until we know which design 
we will use for the storage helpers 
http://www.ovirt.org/Features/Design/StorageHelpers.
Line 132:         for (storage_server_connections lunConnections : 
lun.getLunConnections()) {
Line 133:             StorageType storageType = 
lunConnections.getstorage_type();
Line 134:             if 
(!storageConnectionsForStorageTypeMap.containsKey(storageType)) {
Line 135:                 storageConnectionsForStorageTypeMap.put(storageType, 
new ArrayList<storage_server_connections>());


Line 130:         Map<StorageType, List<storage_server_connections>> 
storageConnectionsForStorageTypeMap =
Line 131:                 new HashMap<StorageType, 
List<storage_server_connections>>();
Line 132:         for (storage_server_connections lunConnections : 
lun.getLunConnections()) {
Line 133:             StorageType storageType = 
lunConnections.getstorage_type();
Line 134:             if 
(!storageConnectionsForStorageTypeMap.containsKey(storageType)) {
Changed to linked list
Line 135:                 storageConnectionsForStorageTypeMap.put(storageType, 
new ArrayList<storage_server_connections>());
Line 136:             }
Line 137:             
storageConnectionsForStorageTypeMap.get(storageType).add(lunConnections);
Line 138:         }


....................................................
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHelperBaseTest.java
Line 26: 
Line 27:     @Test
Line 28:     public void getLunConnectionsForISCSI() {
Line 29:         LUNs lun = new LUNs();
Line 30:         ArrayList<storage_server_connections> connections = new 
ArrayList<storage_server_connections>();
It has different name, I don't see much benefit in doing so, we already calling 
a method here
Line 31:         connections.add(new storage_server_connections("Some LUN 
connection",
Line 32:                 "id",
Line 33:                 "iqn",
Line 34:                 "password",


--
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@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