Liron Aravot has posted comments on this change.
Change subject: core: Use storage helper to fetch (related to BZ882825)
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java
Line 53: if (disk.getDiskStorageType() == DiskStorageType.LUN) {
Line 54: LunDisk lunDisk = (LunDisk) disk;
Line 55: if (commandType == VDSCommandType.HotPlugDisk) {
Line 56: LUNs lun = lunDisk.getLun();
Line 57: updateLUNConnectionsInfo(lun);
if this is the current use, why do we need a map and not a set? we don't use
the map values but only the keys..so we can just iterate over the connection
and add to a set.
Line 58: Map<StorageType, List<storage_server_connections>>
lunsByStorageType =
Line 59: StorageHelperBase.filterLUNsByStorageType(lun);
Line 60: for (StorageType storageType :
lunsByStorageType.keySet()) {
Line 61: if
(!getStorageHelper(storageType).connectStorageToLunByVdsId(null,
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHelperBase.java
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>>();
I don't see reason to change classes to use it and then delete the added code,
if we want a map of connections and not a list (I'm sure that we need map or
filter, as we know that it will have only iscsi connections..but ok) ..there's
no reason to keep the list which is not relevant anyway as we will always
create a map.
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>());
--
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 <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
