Daniel Erez has posted comments on this change.
Change subject: engine,webadmin: Limit usage of Local ISO SD to Local DC
......................................................................
Patch Set 2: Code-Review-1
(3 comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/ImportStorageModelBehavior.java
Line 72: {
Line 73: Model model = (Model) item;
Line 74: StoragePool dataCenter = (StoragePool)
getModel().getDataCenter().getSelectedItem();
Line 75:
Line 76: if (!isStorageDomainSelectableForDataCenter(dataCenter, item))
{
see comment in NewEditStorageModelBehavior.java
Line 77: model.setIsSelectable(false);
Line 78: } else {
Line 79: // available type/function items are:
Line 80: // all in case of Unassigned DC.
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/NewEditStorageModelBehavior.java
Line 79: boolean isData = item.getRole() == StorageDomainType.Data;
Line 80: boolean isExportOrIso = item.getRole() ==
StorageDomainType.ImportExport || item.getRole() == StorageDomainType.ISO;
Line 81:
Line 82: boolean canAttachData = isData && item.getType() ==
dataCenter.getStorageType();
Line 83: boolean canAttachExportOrIso = isExportOrIso &&
isNoExportOrIsoStorageAttached &&
i think it would be nicer to add 'canAttachLocalIso' flag instead of calling
'isStorageDomainSelectableForDataCenter'.
I.e.:
* boolean canAttachLocalSD = dataCenter.getStorageType() == StorageType.LOCALFS
&& item.getStorageType() == StorageType.LOCALFS
* model.setIsSelectable(isExistingStorage || (isNoneDataCenter && isData) ||
(!isNoneDataCenter && (canAttachData || canAttachExportOrIso ||
canAttachLocalSD)))
Line 84: dataCenter.getstatus() !=
StoragePoolStatus.Uninitialized;
Line 85:
Line 86: model.setIsSelectable(isExistingStorage ||
(isNoneDataCenter && isData) ||
Line 87: (!isNoneDataCenter && (canAttachData ||
canAttachExportOrIso)));
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModelBehavior.java
Line 107: getModel().frontend_QueryComplete();
Line 108: }
Line 109: }
Line 110:
Line 111: public boolean isStorageDomainSelectableForDataCenter(StoragePool
dataCenter, IStorageModel storageModel) {
since this method verification is specific for local sds,
i think it would be preferable to:
* return: dataCenter.getStorageType() == StorageType.LOCALFS &&
item.getStorageType() == StorageType.LOCALFS
* rename to something like: ' canAttachLocalSD'
* use in the behavior models as suggested
Line 112: boolean isLocalIsoStorageDomain = (storageModel.getRole() ==
StorageDomainType.ISO &&
Line 113: storageModel.getType() == StorageType.LOCALFS);
Line 114: boolean isLocalDataCenter = (dataCenter != null &&
dataCenter.getStorageType() == StorageType.LOCALFS);
Line 115:
--
To view, visit http://gerrit.ovirt.org/18229
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4cfc3bb68581664fa6729e5cb8a163324e7fbc29
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[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
