Mark Wu has posted comments on this change.
Change subject: Allow creating ISO domain on localfs
......................................................................
Patch Set 3: (10 inline comments)
Allon,
I really appreciate for your detailed review and great suggestion. I will add
the posix/ISO support in next patch as you suggested.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddLocalStorageDomainCommand.java
Line 46: setStoragePool(storagePool);
Line 47: }
Line 48:
Line 49: if (retVal && getStorageDomain().getStorageDomainType() !=
StorageDomainType.ISO
Line 50: && storagePool.getstorage_pool_type() !=
StorageType.LOCALFS) {
Sorry, I don't understand comments. My code only allows ISO domain to be
added to a non localfs DC. You proposed code allows ISO domain and export
domain. Please correct me if I am wrong. Thanks.
Line 51:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_IS_NOT_LOCAL);
Line 52: retVal = false;
Line 53: }
Line 54:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java
Line 124: returnValue = false;
Line 125: }
Line 126: if (returnValue && getStorageDomain().getStorageDomainType()
== StorageDomainType.ISO
Line 127: && !(getStorageDomain().getStorageType() ==
StorageType.NFS
Line 128: || getStorageDomain().getStorageType() ==
StorageType.LOCALFS)) {
Done
Line 129:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_TYPE_ILLEGAL);
Line 130: returnValue = false;
Line 131: }
Line 132: if (returnValue && getStorageDomain().getStorageDomainType()
== StorageDomainType.ImportExport
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java
Line 78
Line 79
Line 80
Line 81
Line 82
When I can add a localfs/ISO domain to a NFS DC, it raises an error of
'.VDS_CANNOT_ADD_LOCAL_STORAGE_TO_NON_LOCAL_H
OST'. I can't figure out how to tell this connection will be used for ISO
domain or Data domain here. So I just simply remove the validation. It's a
little bit impolite, but we have already guaranteed localfs/Data can't be added
to non local host in upper level, right. Is it acceptable?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DisconnectStorageServerConnectionCommand.java
Line 35
Line 36
Line 37
Line 38
Line 39
The same reason as AddStorageServerConnectionCommand. The raised error message
lead me remove this validation.
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 542
Line 543
Line 544
Line 545
Line 546
Done
....................................................
Commit Message
Line 3: AuthorDate: 2013-02-28 17:36:26 +0800
Line 4: Commit: Mark Wu <[email protected]>
Line 5: CommitDate: 2013-03-08 16:18:02 +0800
Line 6:
Line 7: Allow creating ISO domain on localfs
Done
Line 8:
Line 9: This patch allows creating ISO domain on localfs. Even though localfs
Line 10: can't be shared among the hosts in cluster, it could help in the case
Line 11: of no nfs available. VM can be created on the host which has the ISO
Line 8:
Line 9: This patch allows creating ISO domain on localfs. Even though localfs
Line 10: can't be shared among the hosts in cluster, it could help in the case
Line 11: of no nfs available. VM can be created on the host which has the ISO
Line 12: domain attached, and then be migrated to any other host in the cluster.
Done
Line 13:
Line 14: Change-Id: I2a8d3ea8ab4ac10353ec8574287458e8eb63e882
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterStorageListModel.java
Line 562: String localStorgaeDC = null;
Line 563: for (StorageDomain a : Linq.<StorageDomain>
Cast(getSelectedItems()))
Line 564: {
Line 565: // For local storage - remove; otherwise - detach
Line 566: if (a.getStorageType() == StorageType.LOCALFS &&
a.getStorageDomainType() != StorageDomainType.ISO)
In my test, NFS ISO domain is not destroyed after detaching. I think it
doesn't make sense that forcibly destroy the domain and ISO images on detach.
We could leave the choice to user.
Line 567: {
Line 568: getpb_remove().add(new
RemoveStorageDomainParameters(a.getId()));
Line 569: localStorgaeDC = a.getStoragePoolName();
Line 570: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/NewEditStorageModelBehavior.java
Line 77: ||
(!dataCenter.getId().equals(StorageModel.UnassignedDataCenterId) &&
((item.getRole() == StorageDomainType.Data && item.getType() ==
dataCenter.getstorage_pool_type())
Line 78: || (item.getRole() ==
StorageDomainType.ImportExport
Line 79: && item.getType() ==
StorageType.NFS
Line 80: && dataCenter.getstatus() !=
StoragePoolStatus.Uninitialized && isNoStorageAttached) || item.getRole() ==
StorageDomainType.ISO
Line 81: && (item.getType() == StorageType.NFS
|| item.getType() == StorageType.LOCALFS)
Done
Line 82: && dataCenter.getstatus() !=
StoragePoolStatus.Uninitialized && isNoStorageAttached)) ||
(getModel().getStorage() != null && item.getType() == getModel().getStorage()
Line 83: .getStorageType())));
Line 84:
Line 85: behavior.OnStorageModelUpdated(item);
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java
Line 296: localDataModel.setRole(StorageDomainType.Data);
Line 297: items.add(localDataModel);
Line 298:
Line 299: LocalStorageModel localIsoModel = new LocalStorageModel();
Line 300: localIsoModel.setRole(StorageDomainType.ISO);
Done
Line 301: items.add(localIsoModel);
Line 302:
Line 303: PosixStorageModel posixDataModel = new PosixStorageModel();
Line 304: posixDataModel.setRole(StorageDomainType.Data);
--
To view, visit http://gerrit.ovirt.org/12687
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a8d3ea8ab4ac10353ec8574287458e8eb63e882
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
