Daniel Erez has posted comments on this change.

Change subject: core,webadmin: Added flag on storage pool that marks a local fs 
pool
......................................................................


Patch Set 8:

(3 comments)

generally +1 for UI - please verify NPE is not possible in the mentioned flows 
(commented inline).

http://gerrit.ovirt.org/#/c/23072/8/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/ImportStorageModelBehavior.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/ImportStorageModelBehavior.java:

Line 78:     }
Line 79: 
Line 80:     private boolean isItemSelectable(IStorageModel item, StoragePool 
dataCenter, boolean isNoStorageAttached) {
Line 81:         // Local SD can be attached to a local DC only
Line 82:         if (isLocalStorage(item) && !dataCenter.isLocal()) {
verify that dataCenter can't be null in this flow - iirc it should be fine.
Line 83:             return false;
Line 84:         }
Line 85: 
Line 86:         // All storage domains can be attached to Unassigned DC


http://gerrit.ovirt.org/#/c/23072/8/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/NewEditStorageModelBehavior.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/NewEditStorageModelBehavior.java:

Line 81:         if (isExistingStorage) {
Line 82:             return true;
Line 83:         }
Line 84: 
Line 85:         if (isLocalStorage(item) != dataCenter.isLocal()) {
same
Line 86:             return false;
Line 87:         }
Line 88: 
Line 89:         boolean isNoneDataCenter = 
dataCenter.getId().equals(StorageModel.UnassignedDataCenterId);


Line 85:         if (isLocalStorage(item) != dataCenter.isLocal()) {
Line 86:             return false;
Line 87:         }
Line 88: 
Line 89:         boolean isNoneDataCenter = 
dataCenter.getId().equals(StorageModel.UnassignedDataCenterId);
did you verify that dataCenter can't be null in this flow?
Line 90:         boolean isDataDomain = item.getRole() == 
StorageDomainType.Data;
Line 91: 
Line 92:         if (isNoneDataCenter) {
Line 93:             // 'None' Data Center can create only Data Storage Domain


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f1067fd1d299a93b9555c4714b4e85ff980a830
Gerrit-PatchSet: 8
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: Tal Nisan <[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