Daniel Erez has uploaded a new change for review. Change subject: core: verify VirtIO-SCSI enabled on DiskValidator ......................................................................
core: verify VirtIO-SCSI enabled on DiskValidator * DiskValidator -> isVirtIoScsiValid: VirtIO-SCSI interface is valid only when VirtIO-SCSI is enabled for the VM (i.e. a controller is attached to the VM). * Altered relevant commands/tests accordingly. Change-Id: Iff20860a8462df49afbc21a66faf2614904a7a72 Signed-off-by: Daniel Erez <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java 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/AttachDiskToVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.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/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, 72 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/36/19636/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java index 3c6a6dc..e98df03 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java @@ -8,6 +8,7 @@ import org.ovirt.engine.core.bll.storage.IStorageHelper; import org.ovirt.engine.core.bll.storage.StorageHelperBase; import org.ovirt.engine.core.bll.storage.StorageHelperDirector; +import org.ovirt.engine.core.bll.validator.DiskValidator; import org.ovirt.engine.core.bll.validator.LocalizedVmStatus; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.bll.validator.VmValidator; @@ -225,6 +226,10 @@ return new SnapshotsValidator(); } + protected DiskValidator getDiskValidator(Disk disk) { + return new DiskValidator(disk); + } + protected boolean isVmNotInPreviewSnapshot() { final SnapshotsValidator snapshotsValidator = getSnapshotsValidator(); return 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 45f64a8..cd4c413 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 @@ -112,7 +112,7 @@ protected boolean checkIfLunDiskCanBeAdded() { LUNs lun = ((LunDisk) getParameters().getDiskInfo()).getLun(); - DiskValidator diskValidator = new DiskValidator(getParameters().getDiskInfo()); + DiskValidator diskValidator = getDiskValidator(getParameters().getDiskInfo()); switch (lun.getLunType()) { case UNKNOWN: @@ -148,7 +148,7 @@ private boolean checkIfImageDiskCanBeAdded(VM vm) { boolean returnValue; - DiskValidator diskValidator = new DiskValidator(getParameters().getDiskInfo()); + DiskValidator diskValidator = getDiskValidator(getParameters().getDiskInfo()); // vm agnostic checks returnValue = diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java index 0c1c092..3d49f65 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java @@ -110,7 +110,7 @@ return false; } } - DiskValidator diskValidator = new DiskValidator(disk); + DiskValidator diskValidator = getDiskValidator(disk); if (!validate(diskValidator.isVirtIoScsiValid(getVm()))) { return false; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java index 4353900..de2930f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java @@ -5,6 +5,7 @@ import java.util.Map; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; +import org.ovirt.engine.core.bll.validator.DiskValidator; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.HotPlugDiskToVmParameters; @@ -46,7 +47,13 @@ isDiskExist(getDisk()) && checkCanPerformPlugUnPlugDisk() && isVmNotInPreviewSnapshot() && - imageStorageValidation(); + imageStorageValidation() && + virtIoScsiDiskValidation(); + } + + private boolean virtIoScsiDiskValidation() { + DiskValidator diskValidator = getDiskValidator(disk); + return validate(diskValidator.isVirtIoScsiValid(getVm())); } private boolean imageStorageValidation() { 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 98b59ca..b072ae9 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 @@ -3,6 +3,7 @@ import java.util.List; import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType; @@ -13,6 +14,7 @@ import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.osinfo.OsRepository; import org.ovirt.engine.core.common.utils.SimpleDependecyInjector; +import org.ovirt.engine.core.compat.Guid; /** * A validator for the {@link Disk} class. @@ -45,6 +47,10 @@ return new ValidationResult(VdcBllMessages.VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL); } + if (!isVirtioScsiControllerAttached(vm.getId())) { + return new ValidationResult(VdcBllMessages.CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED); + } + return isOsSupportedForVirtIoScsi(vm); } @@ -65,4 +71,8 @@ } return ValidationResult.VALID; } + + public boolean isVirtioScsiControllerAttached(Guid vmId) { + return VmDeviceUtils.isVirtioScsiControllerAttached(vmId); + } } 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 a1240d8..c6445cf 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 @@ -22,6 +22,7 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; +import org.ovirt.engine.core.bll.validator.DiskValidator; import org.ovirt.engine.core.common.action.AddDiskParameters; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; @@ -375,6 +376,12 @@ return snapshotsValidator; } + private DiskValidator spyDiskValidator(Disk disk) { + DiskValidator diskValidator = spy(new DiskValidator(disk)); + doReturn(diskValidator).when(command).getDiskValidator(disk); + return diskValidator; + } + /** * Mock a {@link StoragePool}. * @@ -632,12 +639,36 @@ vm.setVmOs(7); + DiskValidator diskValidator = spyDiskValidator(disk); + doReturn(true).when(diskValidator).isVirtioScsiControllerAttached(any(Guid.class)); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.ACTION_TYPE_FAILED_GUEST_OS_VERSION_IS_NOT_SUPPORTED); } @Test - public void testLunDiskWithSgioCanBeAdded() { + public void testVirtioScsiDiskWithoutControllerCantBeAdded() { + DiskImage disk = createVirtIoScsiDiskImage(); + AddDiskParameters parameters = createParameters(); + parameters.setDiskInfo(disk); + + Guid storageId = Guid.newGuid(); + initializeCommand(storageId, parameters); + mockStorageDomain(storageId); + mockStoragePoolIsoMap(); + + VM vm = mockVm(); + vm.setVdsGroupCompatibilityVersion(Version.v3_3); + + DiskValidator diskValidator = spyDiskValidator(disk); + doReturn(false).when(diskValidator).isVirtioScsiControllerAttached(any(Guid.class)); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, + VdcBllMessages.CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED); + } + + @Test + public void testDiskImageWithSgioCantBeAdded() { DiskImage disk = createVirtIoScsiDiskImage(); disk.setSgio(ScsiGenericIO.UNFILTERED); @@ -652,12 +683,15 @@ VM vm = mockVm(); vm.setVdsGroupCompatibilityVersion(Version.v3_3); + DiskValidator diskValidator = spyDiskValidator(disk); + doReturn(true).when(diskValidator).isVirtioScsiControllerAttached(any(Guid.class)); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK); } @Test - public void testDiskImageWithSgioCantBeAdded() { + public void testLunDiskWithSgioCanBeAdded() { LunDisk disk = createISCSILunDisk(); disk.setDiskInterface(DiskInterface.VirtIO_SCSI); disk.setSgio(ScsiGenericIO.UNFILTERED); @@ -669,6 +703,9 @@ VM vm = mockVm(); vm.setVdsGroupCompatibilityVersion(Version.v3_3); + DiskValidator diskValidator = spyDiskValidator(disk); + doReturn(true).when(diskValidator).isVirtioScsiControllerAttached(any(Guid.class)); + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); } 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 2ce81c7..59c5bb3 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 @@ -672,6 +672,7 @@ VM_CANNOT_RUN_FROM_DISK_WITHOUT_PLUGGED_DISK(ErrorType.CONFLICT), SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK(ErrorType.NOT_SUPPORTED), VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL(ErrorType.INCOMPATIBLE_VERSION), + CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_QUOTA_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_QUOTA_IS_NOT_VALID(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_NO_QUOTA_SET_FOR_DOMAIN(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 c11ed2b..145ee71 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -860,6 +860,7 @@ DISK_IS_ALREADY_SHARED_BETWEEN_VMS=Cannot ${action} ${type}. Disk is shared between vms and cannot become unshareable . Detach the disk from the rest of the vms it is attached to and then update the disk to be unshareable. SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK="Cannot ${action} ${type}. SCSI Generic IO is not supported for image disk." VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL=Virtio-SCSI interface is only available on cluster level 3.3 or higher. +CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED=Cannot ${action} ${type}. VirtIO-SCSI is disabled for the VM #Suspected (not in use?) USER_PASSWORD_EXPIRED=Cannot Login. User Password has expired, Please change your password. 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 1ccf6be..0b26802 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 @@ -2326,6 +2326,9 @@ @DefaultStringValue("VirtIO-SCSI interface is only available on cluster level 3.3 or higher.") String VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL(); + @DefaultStringValue("Cannot ${action} ${type}. VirtIO-SCSI is disabled for the VM") + String CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED(); + // Suspected (not in use?) @DefaultStringValue("Cannot Login. User Password has expired, Please change your password.") String USER_PASSWORD_EXPIRED(); 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 836dbd7..7eb9fc0 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 @@ -838,6 +838,7 @@ DISK_IS_ALREADY_SHARED_BETWEEN_VMS=Cannot ${action} ${type}. Disk is shared between vms and cannot become unshareable . Detach the disk from the rest of the vms it is attached to and then update the disk to be unshareable. SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK="Cannot ${action} ${type}. SCSI Generic IO is not supported for image disk." VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL=Virtio-SCSI interface is only available on cluster level 3.3 or higher. +CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED=Cannot ${action} ${type}. VirtIO-SCSI is disabled for the VM #Suspected (not in use?) USER_PASSWORD_EXPIRED=Cannot Login. User Password has expired, Please change your password. 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 79fcc2a..f486e7f 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 @@ -860,6 +860,7 @@ DISK_IS_ALREADY_SHARED_BETWEEN_VMS=Cannot ${action} ${type}. Disk is shared between vms and cannot become unshareable . Detach the disk from the rest of the vms it is attached to and then update the disk to be unshareable. SCSI_GENERIC_IO_IS_NOT_SUPPORTED_FOR_IMAGE_DISK="Cannot ${action} ${type}. SCSI Generic IO is not supported for image disk." VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL=Virtio-SCSI interface is only available on cluster level 3.3 or higher. +CANNOT_PERFORM_ACTION_VIRTIO_SCSI_IS_DISABLED=Cannot ${action} ${type}. VirtIO-SCSI is disabled for the VM #Suspected (not in use?) USER_PASSWORD_EXPIRED=Cannot Login. User Password has expired, Please change your password. -- To view, visit http://gerrit.ovirt.org/19636 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iff20860a8462df49afbc21a66faf2614904a7a72 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
