Sergey Gotliv has uploaded a new change for review. Change subject: engine: Validate that IDE disk is not read-only ......................................................................
engine: Validate that IDE disk is not read-only Real IDE disks can't be read-only therefore Engine has to validate it. Change-Id: I441362adf2f4833b034ede4093fc8195debd2ed5 Bug-Url: https://bugzilla.redhat.com/1057546 Signed-off-by: Sergey Gotliv <sgot...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 10 files changed, 55 insertions(+), 14 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/53/23953/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java index 91d2428..f248bc2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java @@ -100,20 +100,24 @@ return failCanDoAction(VdcBllMessages.CANNOT_ADD_FLOATING_DISK_WITH_PLUG_VM_SET); } + DiskValidator diskValidator = getDiskValidator(getParameters().getDiskInfo()); + if (!validate(diskValidator.isDiskInterfaceSupportReadOnly())) { + return false; + } + if (DiskStorageType.IMAGE == getParameters().getDiskInfo().getDiskStorageType()) { - return checkIfImageDiskCanBeAdded(vm); + return checkIfImageDiskCanBeAdded(vm, diskValidator); } if (DiskStorageType.LUN == getParameters().getDiskInfo().getDiskStorageType()) { - return checkIfLunDiskCanBeAdded(); + return checkIfLunDiskCanBeAdded(diskValidator); } return true; } - protected boolean checkIfLunDiskCanBeAdded() { + protected boolean checkIfLunDiskCanBeAdded(DiskValidator diskValidator) { LUNs lun = ((LunDisk) getParameters().getDiskInfo()).getLun(); - DiskValidator diskValidator = getDiskValidator(getParameters().getDiskInfo()); switch (lun.getLunType()) { case UNKNOWN: @@ -151,9 +155,8 @@ return true; } - private boolean checkIfImageDiskCanBeAdded(VM vm) { + private boolean checkIfImageDiskCanBeAdded(VM vm, DiskValidator diskValidator) { boolean returnValue; - DiskValidator diskValidator = getDiskValidator(getParameters().getDiskInfo()); StorageDomainValidator storageDomainValidator = createStorageDomainValidator(); // vm agnostic checks returnValue = diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java index 1d94e42..caa8a7f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java @@ -158,6 +158,7 @@ DiskValidator diskValidator = getDiskValidator(getNewDisk()); return validateCanUpdateShareable() && validateCanUpdateReadOnly() && validate(diskValidator.isVirtIoScsiValid(getVm())) && + validate(diskValidator.isDiskInterfaceSupportReadOnly()) && (getOldDisk().getDiskInterface() == getNewDisk().getDiskInterface() || validate(diskValidator.isDiskInterfaceSupported(getVm()))); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java index 6a0bfeaa..a1c0496 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java @@ -99,6 +99,14 @@ } + public ValidationResult isDiskInterfaceSupportReadOnly() { + if (disk.getReadOnly() && disk.getDiskInterface() == DiskInterface.IDE) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR); + } + + return ValidationResult.VALID; + } + protected VmDAO getVmDAO() { return DbFacade.getInstance().getVmDao(); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java index 999eddc..b6fe98a 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java @@ -588,7 +588,7 @@ parameters.setDiskInfo(disk); initializeCommand(Guid.newGuid(), parameters); when(diskLunMapDAO.getDiskIdByLunId(disk.getLun().getLUN_id())).thenReturn(null); - assertTrue("checkIfLunDiskCanBeAdded() failed for valid iscsi lun", command.checkIfLunDiskCanBeAdded()); + assertTrue("checkIfLunDiskCanBeAdded() failed for valid iscsi lun", command.checkIfLunDiskCanBeAdded(diskValidator)); } @Test @@ -598,7 +598,7 @@ parameters.setDiskInfo(disk); initializeCommand(Guid.newGuid(), parameters); disk.getLun().setLunType(StorageType.UNKNOWN); - assertFalse("checkIfLunDiskCanBeAdded() succeded for LUN with UNKNOWN type", command.checkIfLunDiskCanBeAdded()); + assertFalse("checkIfLunDiskCanBeAdded() succeded for LUN with UNKNOWN type", command.checkIfLunDiskCanBeAdded(diskValidator)); assertTrue("checkIfLunDiskCanBeAdded() failed but correct can do action hasn't been added to the return response", verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_HAS_NO_VALID_TYPE)); } @@ -609,13 +609,13 @@ parameters.setDiskInfo(disk); initializeCommand(Guid.newGuid(), parameters); disk.getLun().getLunConnections().get(0).setiqn(null); - assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a null iqn", command.checkIfLunDiskCanBeAdded()); + assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a null iqn", command.checkIfLunDiskCanBeAdded(diskValidator)); assertTrue("checkIfLunDiskCanBeAdded() failed but correct can do action hasn't been added to the return response", verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS)); clearCanDoActionMessages(); disk.getLun().getLunConnections().get(0).setiqn(""); - assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with an empty iqn", command.checkIfLunDiskCanBeAdded()); + assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with an empty iqn", command.checkIfLunDiskCanBeAdded(diskValidator)); assertTrue("checkIfLunDiskCanBeAdded() failed but correct can do action hasn't been added to the return response", verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS)); } @@ -626,13 +626,13 @@ parameters.setDiskInfo(disk); initializeCommand(Guid.newGuid(), parameters); disk.getLun().getLunConnections().get(0).setconnection(null); - assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a null address", command.checkIfLunDiskCanBeAdded()); + assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a null address", command.checkIfLunDiskCanBeAdded(diskValidator)); assertTrue("checkIfLunDiskCanBeAdded() failed but correct can do action hasn't been added to the return response", verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS)); clearCanDoActionMessages(); disk.getLun().getLunConnections().get(0).setconnection(""); - assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a empty address", command.checkIfLunDiskCanBeAdded()); + assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a empty address", command.checkIfLunDiskCanBeAdded(diskValidator)); assertTrue("checkIfLunDiskCanBeAdded() failed but correct can do action hasn't been added to the return response", verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS)); } @@ -643,13 +643,13 @@ parameters.setDiskInfo(disk); initializeCommand(Guid.newGuid(), parameters); disk.getLun().getLunConnections().get(0).setport(null); - assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a null port", command.checkIfLunDiskCanBeAdded()); + assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a null port", command.checkIfLunDiskCanBeAdded(diskValidator)); assertTrue("checkIfLunDiskCanBeAdded() failed but correct can do action hasn't been added to the return response", verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS)); clearCanDoActionMessages(); disk.getLun().getLunConnections().get(0).setport(""); - assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a empty port", command.checkIfLunDiskCanBeAdded()); + assertFalse("checkIfLunDiskCanBeAdded() succeded for ISCSI lun which LUNs has storage_server_connection with a empty port", command.checkIfLunDiskCanBeAdded(diskValidator)); assertTrue("checkIfLunDiskCanBeAdded() failed but correct can do action hasn't been added to the return response", verifyCanDoActionMessagesContainMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS)); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java index d3627d4..e46f0d4 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskValidatorTest.java @@ -139,4 +139,24 @@ initializeOsRepository(vm.getOs(), DiskInterface.VirtIO); assertThat(validator.isDiskInterfaceSupported(vm), failsWith(VdcBllMessages.ACTION_TYPE_DISK_INTERFACE_UNSUPPORTED)); } + + @Test + public void readOnlyIsNotSupportedByDiskInterface() { + disk.setReadOnly(true); + disk.setDiskInterface(DiskInterface.IDE); + + assertThat(validator.isDiskInterfaceSupportReadOnly(), + failsWith(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR)); + } + + @Test + public void readOnlyIsSupportedByDiskInterface() { + disk.setReadOnly(true); + disk.setDiskInterface(DiskInterface.VirtIO); + assertThat(validator.isDiskInterfaceSupportReadOnly(), isValid()); + + disk.setReadOnly(false); + disk.setDiskInterface(DiskInterface.IDE); + assertThat(validator.isDiskInterfaceSupportReadOnly(), isValid()); + } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 29faffc..e797c3f 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -138,6 +138,7 @@ ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISKS_LOCKED(ErrorType.CONFLICT), ACTION_TYPE_DISK_INTERFACE_UNSUPPORTED(ErrorType.BAD_PARAMETERS), + ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_DISKS_ILLEGAL(ErrorType.INTERNAL_ERROR), ACTION_TYPE_FAILED_MOVE_DISKS_MIXED_PLUGGED_STATUS(ErrorType.INTERNAL_ERROR), ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST(ErrorType.CONFLICT), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 7f240a1..4ae85ef 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -1148,3 +1148,4 @@ ISCSI_BOND_NOT_EXIST=Cannot ${action} ${type}. The specified iSCSI bond doesn't exist. ISCSI_BOND_WITH_SAME_NAME_EXIST_IN_DATA_CENTER=Cannot ${action} ${type}. iSCSI bond with the same name already exists in the Data Center. +ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR=IDE disk can't be read-only. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index f21c221..872503f 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -3054,4 +3054,7 @@ @DefaultStringValue("Cannot ${action} ${type}. The specified iSCSI bond doesn't exist.") String ISCSI_BOND_NOT_EXIST(); + + @DefaultStringValue("Cannot ${action} ${type}. IDE disk can't be read-only.") + String ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR(); } diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 0f70123..b93a583 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -981,3 +981,5 @@ ISCSI_BOND_NOT_EXIST=Cannot ${action} ${type}. The specified iSCSI bond doesn't exist. ISCSI_BOND_WITH_SAME_NAME_EXIST_IN_DATA_CENTER=Cannot ${action} ${type}. iSCSI bond with the same name already exists in the Data Center. + +ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR=IDE disk can't be read-only. \ No newline at end of file diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 106df1c..12eb37c 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -1118,3 +1118,5 @@ ISCSI_BOND_NOT_EXIST=Cannot ${action} ${type}. The specified iSCSI bond doesn't exist. ISCSI_BOND_WITH_SAME_NAME_EXIST_IN_DATA_CENTER=Cannot ${action} ${type}. iSCSI bond with the same name already exists in the Data Center. + +ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR=IDE disk can't be read-only. \ No newline at end of file -- To view, visit http://gerrit.ovirt.org/23953 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I441362adf2f4833b034ede4093fc8195debd2ed5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
