Sergey Gotliv has uploaded a new change for review. Change subject: engine: Improve read-only disk validations ......................................................................
engine: Improve read-only disk validations 1. Validate that boot disk is read write. 2. Allow edit read-only attribute of unplugged disk when VM is running. 3. Create a single API that performs all read-only related validations. Change-Id: I36cdd4821168b529642f33e04fe9878183305b8e Bug-Url: https://bugzilla.redhat.com/1057658 Bug-Url: https://bugzilla.redhat.com/1050840 Signed-off-by: Sergey Gotliv <[email protected]> --- 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/UpdateVmDiskCommandTest.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 11 files changed, 37 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/80/24680/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 10e3ca1..8b6770a 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 @@ -101,7 +101,7 @@ } DiskValidator diskValidator = getDiskValidator(getParameters().getDiskInfo()); - if (!validate(diskValidator.isReadOnlyPropertyCompatibleWithInterface())) { + if (!validate(diskValidator.isReadOnlySupported())) { return false; } 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 385fecf..5d58625 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,7 +158,7 @@ DiskValidator diskValidator = getDiskValidator(getNewDisk()); return validateCanUpdateShareable() && validateCanUpdateReadOnly() && validate(diskValidator.isVirtIoScsiValid(getVm())) && - validate(diskValidator.isReadOnlyPropertyCompatibleWithInterface()) && + validate(diskValidator.isReadOnlySupported()) && (getOldDisk().getDiskInterface() == getNewDisk().getDiskInterface() || validate(diskValidator.isDiskInterfaceSupported(getVm()))); } @@ -258,7 +258,7 @@ } private boolean validateCanUpdateReadOnly() { - if (shouldUpdateReadOnly() && getVm().getStatus() != VMStatus.Down) { + if (shouldUpdateReadOnly() && getVm().getStatus() != VMStatus.Down && vmDeviceForVm.getIsPlugged()) { return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN); } return true; 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 12f00ea..b00d3c9 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,9 +99,15 @@ } - public ValidationResult isReadOnlyPropertyCompatibleWithInterface() { - if (Boolean.TRUE.equals(disk.getReadOnly()) && disk.getDiskInterface() == DiskInterface.IDE) { - return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR); + public ValidationResult isReadOnlySupported() { + if (Boolean.TRUE.equals(disk.getReadOnly())) { + if (disk.getDiskInterface() == DiskInterface.IDE) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR); + } + + if (disk.isBoot()) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_BOOT_DISK_CANNOT_BE_READ_ONLY); + } } return ValidationResult.VALID; 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 8086369..91fdf98 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 @@ -126,7 +126,7 @@ mockStorageDomain(storageId); mockStoragePoolIsoMap(); mockMaxPciSlots(); - when(diskValidator.isReadOnlyPropertyCompatibleWithInterface()).thenReturn(ValidationResult.VALID); + when(diskValidator.isReadOnlySupported()).thenReturn(ValidationResult.VALID); when(diskValidator.isDiskInterfaceSupported(any(VM.class))).thenReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_DISK_INTERFACE_UNSUPPORTED)); when(diskValidator.isVirtIoScsiValid(any(VM.class))).thenReturn(ValidationResult.VALID); when(command.getDiskValidator(any(Disk.class))).thenReturn(diskValidator); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java index 3ff4da7..6bc0067 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java @@ -356,7 +356,7 @@ initializeCommand(parameters); doReturn(true).when(command).validatePciAndIdeLimit(any(List.class)); - when(diskValidator.isReadOnlyPropertyCompatibleWithInterface()).thenReturn(ValidationResult.VALID); + when(diskValidator.isReadOnlySupported()).thenReturn(ValidationResult.VALID); when(diskValidator.isDiskInterfaceSupported(any(VM.class))).thenReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_DISK_INTERFACE_UNSUPPORTED)); when(diskValidator.isVirtIoScsiValid(any(VM.class))).thenReturn(ValidationResult.VALID); when(command.getDiskValidator(any(Disk.class))).thenReturn(diskValidator); 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 5e82da2..9eb6913 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 @@ -141,22 +141,33 @@ } @Test - public void readOnlyIsNotSupportedByDiskInterface() { + public void readOnlyIsNotSupportedByIDEDiskInterface() { disk.setReadOnly(true); disk.setDiskInterface(DiskInterface.IDE); - assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(), + assertThat(validator.isReadOnlySupported(), failsWith(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR)); } @Test - public void readOnlyIsSupportedByDiskInterface() { + public void readOnlyIsNotSupportedByBootDisk() { disk.setReadOnly(true); + disk.setBoot(true); disk.setDiskInterface(DiskInterface.VirtIO); - assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(), isValid()); + + assertThat(validator.isReadOnlySupported(), + failsWith(VdcBllMessages.ACTION_TYPE_FAILED_BOOT_DISK_CANNOT_BE_READ_ONLY)); + } + + @Test + public void readOnlyIsSupported() { + disk.setReadOnly(true); + disk.setBoot(false); + disk.setDiskInterface(DiskInterface.VirtIO); + assertThat(validator.isReadOnlySupported(), isValid()); disk.setReadOnly(false); disk.setDiskInterface(DiskInterface.IDE); - assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(), isValid()); + assertThat(validator.isReadOnlySupported(), 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 bb30c0a..532cec0 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 @@ -139,6 +139,7 @@ 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_BOOT_DISK_CANNOT_BE_READ_ONLY(ErrorType.CONFLICT), 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 e5c502a..558e517 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -1152,3 +1152,4 @@ 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=Cannot ${action} ${type}. An IDE disk can't be read-only. +ACTION_TYPE_FAILED_BOOT_DISK_CANNOT_BE_READ_ONLY=Cannot ${action} ${type}. A boot 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 e91bb78..a74b49a 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 @@ -3066,4 +3066,7 @@ @DefaultStringValue("Cannot ${action} ${type}. An IDE disk can't be read-only.") String ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR(); + + @DefaultStringValue("Cannot ${action} ${type}. A boot disk can't be read-only.") + String ACTION_TYPE_FAILED_BOOT_DISK_CANNOT_BE_READ_ONLY(); } 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 3b63342..abbd283 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 @@ -984,3 +984,4 @@ 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=Cannot ${action} ${type}. An IDE disk can't be read-only. +ACTION_TYPE_FAILED_BOOT_DISK_CANNOT_BE_READ_ONLY=Cannot ${action} ${type}. A boot disk can't be read-only. 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 0891d4f..fdad696 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 @@ -1122,3 +1122,4 @@ 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=Cannot ${action} ${type}. An IDE disk can't be read-only. +ACTION_TYPE_FAILED_BOOT_DISK_CANNOT_BE_READ_ONLY=Cannot ${action} ${type}. A boot disk can't be read-only. -- To view, visit http://gerrit.ovirt.org/24680 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I36cdd4821168b529642f33e04fe9878183305b8e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
