Ayal Baron has posted comments on this change.

Change subject: core: Changed storage type checks in commands
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.ovirt.org/#/c/23295/2//COMMIT_MSG
Commit Message:

Line 9: As a part of the effort to remove the type of storage pool the following
Line 10: changes are introduced in this patch:
Line 11: 
Line 12: - Removed unused methods in StorageDomainStaticDao concerning pool 
types
Line 13:   and their tests
should be a separate patch
Line 14: 
Line 15: - Replaced StoragerHelper getter in ConnectAllHostsToLun command from 
the
Line 16:   storage pool type to static ISCSI type as this is the only option for
Line 17:   this command


Line 13:   and their tests
Line 14: 
Line 15: - Replaced StoragerHelper getter in ConnectAllHostsToLun command from 
the
Line 16:   storage pool type to static ISCSI type as this is the only option for
Line 17:   this command
should not be at all (Explained in the code)
Line 18: 
Line 19: - Changed connection to new master in RecoveryStoragePool command to 
use
Line 20:   the storage helper of the type of new master, until now the old 
master
Line 21:   type was used since it didn't make any difference since the types of 
the


Line 18: 
Line 19: - Changed connection to new master in RecoveryStoragePool command to 
use
Line 20:   the storage helper of the type of new master, until now the old 
master
Line 21:   type was used since it didn't make any difference since the types of 
the
Line 22:   old and new were always the same
+1
Line 23: 
Line 24: Relates-To: https://bugzilla.redhat.com/1038053
Line 25: Change-Id: Iab29347dd82ea47bf6e0d1d5542db9d6d52c0b08


http://gerrit.ovirt.org/#/c/23295/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectAllHostsToLunCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectAllHostsToLunCommand.java:

Line 100:             Map<String, List<Guid>> processed = result.getSecond();
Line 101:             for (Map.Entry<String, List<Guid>> entry : 
processed.entrySet()) {
Line 102:                 for (Guid vdsId : entry.getValue()) {
Line 103:                     LUNs lun = lunsMap.get(entry.getKey());
Line 104:                     
StorageHelperDirector.getInstance().getItem(StorageType.ISCSI)
this change looks redundant.
1. StorageHelperDirector should probably be removed in general so no point in 
touching this.
2. if it isn't going then in the future we may support other types of 
connections to LUNs (FCoE software initiator and other) so making this ISCSI 
would be too limiting.  just change to getStorageDomain().getType
Line 105:                             
.disconnectStorageFromLunByVdsId(getStorageDomain(), vdsId, lun);
Line 106:                 }
Line 107:             }
Line 108:         }


-- 
To view, visit http://gerrit.ovirt.org/23295
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab29347dd82ea47bf6e0d1d5542db9d6d52c0b08
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to