Allon Mureinik has posted comments on this change. Change subject: core: add ability edit NFS path in webadmin ......................................................................
Patch Set 7: I would prefer that you didn't submit this (5 inline comments) Please see my and Ayal's comment inline .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java Line 96: @Override Line 97: protected Map<String, Pair<String, String>> getExclusiveLocks() { Line 98: //lock the path to NFS to avoid at the same time if some other user tries to: Line 99: // add new storage domain to same path or edit another storage server connection to point to same path Line 100: return Collections.singletonMap(getParameters().getStorageServerConnection().getconnection(), LockMessagesMatchUtil.STORAGE_CONNECTION); Note to self: revisit. Line 101: } .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java Line 94: return super.canDoAction(); Line 95: } Line 96: Line 97: Line 98: protected String createDomainNamesList(List<StorageDomain> domains) { LinqUtils.aggregate should do the trick Line 99: //Build domain names list to display in the error Line 100: StringBuilder domainNames = new StringBuilder(); Line 101: for(StorageDomain domain : domains) { Line 102: domainNames.append(domain.getStorageName()); Line 108: } Line 109: Line 110: protected boolean isConnectionEditable(StorageDomain storageDomain) { Line 111: boolean isEditable = Line 112: (storageDomain.getStorageDomainType() == StorageDomainType.Data || storageDomain.getStorageDomainType() == StorageDomainType.Master) Why not just add StorageDomainType.isData() ? Line 113: && storageDomain.getStatus() == StorageDomainStatus.Maintenance; Line 114: return isEditable; Line 115: } Line 116: Line 119: StoragePoolIsoMap map = getStoragePoolIsoMap(); Line 120: Line 121: changeStorageDomainStatusInTransaction(map,StorageDomainStatus.Locked); Line 122: //connect to the new path Line 123: boolean hasConnectStorageSucceeded = connectToStorage(); The one passed to the function from the UI, like all storage operations Line 124: VDSReturnValue returnValueUpdatedStorageDomain = null; Line 125: //update info such as free space - because we switched to a different server Line 126: if(!hasConnectStorageSucceeded) { Line 127: setSucceeded(false); Line 127: setSucceeded(false); Line 128: VdcFault f = new VdcFault(); Line 129: f.setError(VdcBllErrors.StorageServerConnectionError); Line 130: getReturnValue().setFault(f); Line 131: changeStorageDomainStatusInTransaction(map,StorageDomainStatus.Maintenance); Just revert the compensation Line 132: return; Line 133: } Line 134: Line 135: returnValueUpdatedStorageDomain = updateStatsForDomain(); -- To view, visit http://gerrit.ovirt.org/12372 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifaff5344ff191d6bdf53cc706c7bb796167a56b3 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alissa Bonas <abo...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches