Vered Volansky has uploaded a new change for review. Change subject: core: Disallow RO LUN ISCSI disks ......................................................................
core: Disallow RO LUN ISCSI disks Qemu currently does not support Direct-LUN disks connected using Virt-IO-SCSI. This patch adds a validation to block this in AddDiskCommand CDA. Also added a new VdcBllMessage and a test for this case. Change-Id: Id84346098d42f20c593c2682dc9ff99a3e77d534 Bug-Url: https://bugzilla.redhat.com/1082673 Signed-off-by: Vered Volansky <[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/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 9 files changed, 55 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/52/27452/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 ef115a1..beae404 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 @@ -157,6 +157,10 @@ return false; } + if (!validate(diskValidator.isReadOnlyPropertyCompatibleWithLunInterface())) { + return false; + } + 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 855a4b8..f58fca0 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 @@ -107,6 +107,16 @@ return ValidationResult.VALID; } + public ValidationResult isReadOnlyPropertyCompatibleWithLunInterface() { + if (Boolean.TRUE.equals(disk.getReadOnly()) && disk.getDiskInterface() == DiskInterface.VirtIO_SCSI && + disk.getDiskStorageType() == DiskStorageType.LUN) { + return new ValidationResult( + VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_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 8086369..5679fc4 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 @@ -850,6 +850,32 @@ VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR); } + @Test + public void testCanDoFailOnAddLunVirtIOSCSIReadOnlyDisk() { + LunDisk disk = createISCSILunDisk(); + disk.setDiskInterface(DiskInterface.VirtIO_SCSI); + disk.setReadOnly(true); + + AddDiskParameters parameters = createParameters(); + parameters.setDiskInfo(disk); + + initializeCommand(Guid.newGuid(), parameters); + doReturn(true).when(command).isDiskCanBeAddedToVm(any(Disk.class), any(VM.class)); + doReturn(true).when(command).isDiskPassPciAndIdeLimit(any(Disk.class)); + + mockVm(); + + when(diskValidator.isVirtIoScsiValid(any(VM.class))).thenReturn(ValidationResult.VALID); + when(diskValidator.isDiskInterfaceSupported(any(VM.class))).thenReturn(ValidationResult.VALID); + when(diskValidator.isReadOnlyPropertyCompatibleWithInterface()).thenReturn(ValidationResult.VALID); + when(diskValidator.isReadOnlyPropertyCompatibleWithLunInterface()).thenReturn( + new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR)); + doReturn(diskValidator).when(command).getDiskValidator(any(Disk.class)); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, + VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR); + } + private void fillDiskMap(LunDisk disk, VM vm, int expectedMapSize) { Map<Guid, Disk> diskMap = new HashMap<Guid, Disk>(); for (int i = 0; i < expectedMapSize; i++) { 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..93ca481 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 @@ -147,6 +147,10 @@ assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(), failsWith(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR)); + + disk.setDiskInterface(DiskInterface.VirtIO_SCSI); + assertThat(validator.isReadOnlyPropertyCompatibleWithLunInterface(), + failsWith(VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR)); } @Test @@ -155,6 +159,10 @@ disk.setDiskInterface(DiskInterface.VirtIO); assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(), isValid()); + disk.setReadOnly(true); + disk.setDiskInterface(DiskInterface.VirtIO_SCSI); + assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(), isValid()); + disk.setReadOnly(false); disk.setDiskInterface(DiskInterface.IDE); assertThat(validator.isReadOnlyPropertyCompatibleWithInterface(), 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 2cc038d..5ad73aa 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_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_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 20683ef..49db338 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -1165,3 +1165,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_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR=Cannot ${action} ${type}. A VirtIO-SCSI LUN disks 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 d108ca2..347b882 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 @@ -3107,4 +3107,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 VirtIO-SCSI LUN disk can't be read-only.") + String ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_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 c4f4409..d4f4cc3 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 @@ -997,3 +997,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_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR=Cannot ${action} ${type}. A VirtIO-SCSI LUN 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 b328e9f..3187683 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 @@ -1137,3 +1137,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_VIRT_IO_SCSI_INTERFACE_FOR_LUN_DISKS_DOES_NOT_SUPPORT_READ_ONLY_ATTR=Cannot ${action} ${type}. A VirtIO-SCSI LUN disk can't be read-only. -- To view, visit http://gerrit.ovirt.org/27452 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id84346098d42f20c593c2682dc9ff99a3e77d534 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Vered Volansky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
