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 <wu...@linux.vnet.ibm.com> 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 <wu...@linux.vnet.ibm.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com> Gerrit-Reviewer: Sharad Mishra <snmis...@linux.vnet.ibm.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches