Vered Volansky has uploaded a new change for review. Change subject: core: change storage_domain_static update policy ......................................................................
core: change storage_domain_static update policy The storage domain's name can only be changed if the storage domain in Active. Comment and description have the same policy, though this shouldn't be so. Comment and Description are only a DB change, and should be allowed unless the storage domain is locked. Change-Id: I2dfa97b1dbe047d98f9f1e7f7ec2d53ae5c8a16b Bug-Url: https://bugzilla.redhat.com/tbd Signed-off-by: Vered Volansky <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommandTest.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java 5 files changed, 50 insertions(+), 35 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/51/36151/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java index 37392f9..ed33bdd 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java @@ -338,7 +338,7 @@ runSynchronizeOperation(new RefreshStoragePoolAndDisconnectAsyncOperationFactory()); return null; } - }); + }); } /** diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java index 839b5e4..12ecc0a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java @@ -22,25 +22,35 @@ super(parameters); } - private boolean _storageDomainNameChanged; + private boolean storageDomainNameChanged; private StorageDomainStatic oldDomain; @Override protected boolean canDoAction() { - if (!super.canDoAction() || !checkStorageDomain() - || !checkStorageDomainStatus(StorageDomainStatus.Active) || !checkStorageDomainNameLengthValid()) { + if (!super.canDoAction() || !checkStorageDomain()) { return false; } - // Only after validating the existing of the storage domain in DB, we set the field lastTimeUsedAsMaster in the + // Only after validating the existence of the storage domain in DB, we set the field lastTimeUsedAsMaster in the // storage domain which is about to be updated. oldDomain = getStorageDomainStaticDAO().get(getStorageDomain().getId()); getStorageDomain().setLastTimeUsedAsMaster(oldDomain.getLastTimeUsedAsMaster()); + + return validateStoragePropertiesUpdate(); + } + + private boolean validateStoragePropertiesUpdate() { + if (!checkStorageDomainStatusNotEqual(StorageDomainStatus.Locked) || !validateStorageNameUpdate()) { + return false; + } // Collect changed fields to update in a list. List<String> props = ObjectIdentityChecker.GetChangedFields(oldDomain, getStorageDomain() .getStorageStaticData()); + if (props.isEmpty()) { + return true; + } // Allow change only to name & description field props.remove("storageName"); props.remove("description"); @@ -50,19 +60,28 @@ StringUtils.join(props, ",")); return failCanDoAction(VdcBllMessages.ERROR_CANNOT_CHANGE_STORAGE_DOMAIN_FIELDS); } + return true; + } - _storageDomainNameChanged = + private boolean validateStorageNameUpdate() { + storageDomainNameChanged = !StringUtils.equals(oldDomain.getStorageName(), getStorageDomain().getStorageName()); - // if domain is part of pool, and name changed, check that pool is up in - // order to change description in spm - if (_storageDomainNameChanged && !validate(new StoragePoolValidator(getStoragePool()).isUp())) { - return false; - } - if (_storageDomainNameChanged && isStorageWithSameNameExists()) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST); - } + if (storageDomainNameChanged) { + if (!checkStorageDomainStatus(StorageDomainStatus.Active) || !checkStorageDomainNameLengthValid()) { + return false; + } + // if domain is part of pool, and name changed, check that pool is up in + // order to change description in spm + if (storageDomainNameChanged && !validate(new StoragePoolValidator(getStoragePool()).isUp())) { + return false; + } + + if (storageDomainNameChanged && isStorageWithSameNameExists()) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST); + } + } return true; } @@ -81,7 +100,7 @@ @Override protected void executeCommand() { getStorageDomainStaticDAO().update(getStorageDomain().getStorageStaticData()); - if (_storageDomainNameChanged && getStoragePool() != null) { + if (storageDomainNameChanged && getStoragePool() != null) { runVdsCommand( VDSCommandType.SetStorageDomainDescription, new SetStorageDomainDescriptionVDSCommandParameters(getStoragePool().getId(), diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommandTest.java index a8b3bfe..0fa32e6 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommandTest.java @@ -111,7 +111,7 @@ @Test public void canDoActionWrongStatus() { - sd.setStatus(StorageDomainStatus.Maintenance); + sd.setStatus(StorageDomainStatus.Locked); CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL2); } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java index 31cf37e..a98560e 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java @@ -345,17 +345,18 @@ model.getDataCenter().setIsChangable(false); model.getFormat().setIsChangable(false); - boolean isStorageEditable = model.isStorageActive() || model.isNewStorage(); + boolean isStorageNameEditable = model.isStorageActive() || model.isNewStorage(); + boolean isStorageEditable = model.isStorageNotLocked() || model.isNewStorage(); model.getHost().setIsChangable(false); - model.getName().setIsChangable(isStorageEditable); + model.getName().setIsChangable(isStorageNameEditable); model.getDescription().setIsChangable(isStorageEditable); model.getComment().setIsChangable(isStorageEditable); //set the field domain type to non editable model.getAvailableStorageItems().setIsChangable(false); - model.setIsChangable(isStorageEditable); + model.setIsChangable(isStorageNameEditable || isStorageEditable); boolean isPathEditable = isPathEditable(storage); - isStorageEditable = isStorageEditable || isPathEditable; + isStorageNameEditable = isStorageNameEditable || isPathEditable; IStorageModel item = null; switch (storage.getStorageType()) { @@ -404,7 +405,7 @@ UICommand command; - if (isStorageEditable) { + if (isStorageNameEditable || isStorageEditable) { command = createOKCommand("OnSave"); //$NON-NLS-1$ model.getCommands().add(command); @@ -704,7 +705,6 @@ String tempVar = storageModel.getOriginalName(); String originalName = (tempVar != null) ? tempVar : ""; //$NON-NLS-1$ boolean isNameUnique = (Boolean) returnValue; - if (!isNameUnique && name1.compareToIgnoreCase(originalName) != 0) { storageModel.getName() .getInvalidityReasons() @@ -882,8 +882,7 @@ model); } - private void onSave() - { + private void onSave() { storageNameValidation(); } @@ -1133,7 +1132,7 @@ } private boolean isEditAvailable(StorageDomain storageDomain) { - if (storageDomain == null) { + if (storageDomain == null && storageDomain.getStorageDomainSharedStatus() != StorageDomainSharedStatus.Locked) { return false; } @@ -1245,9 +1244,7 @@ if (isPathEditable(storageDomain)) { updatePath(); } - else { - updateStorageDomain(); - } + updateStorageDomain(); } } @@ -1369,7 +1366,6 @@ } }), null, path); } else { - updateStorageDomain(); } } @@ -1495,9 +1491,7 @@ if (isPathEditable(storageDomain)) { updatePath(); } - else { - updateStorageDomain(); - } + updateStorageDomain(); } } @@ -1733,9 +1727,7 @@ if (isPathEditable(storageDomain)) { updatePath(); } - else { - updateStorageDomain(); - } + updateStorageDomain(); } } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java index 3d21510..b2bb561 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java @@ -733,6 +733,10 @@ || getStorage().getStorageDomainSharedStatus() == StorageDomainSharedStatus.Mixed; } + public boolean isStorageNotLocked() { + return getStorage().getStorageDomainSharedStatus() != StorageDomainSharedStatus.Locked; + } + public boolean isNewStorage() { return getStorage() == null; } -- To view, visit http://gerrit.ovirt.org/36151 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2dfa97b1dbe047d98f9f1e7f7ec2d53ae5c8a16b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
