Daniel Erez has uploaded a new change for review. Change subject: core: block virtio-scsi for unsupported OSes ......................................................................
core: block virtio-scsi for unsupported OSes Block VirtIO-SCSI for unsupported operation systems: * Added isOsTypeSupportedForVirtioScsi method to VmHandler. * Added an appropriate VdcBllMessage. * Modified AddVmCommand and UpdateVmCommand accordingly. * Updated tests. Change-Id: Id9907d35b53c0447662bc638146c4c61ac5cce8a Bug-Url: https://bugzilla.redhat.com/1038613 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.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, 107 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/48/22648/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index 8e83341..d4bf8b0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -22,6 +22,7 @@ import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.bll.validator.VmWatchdogValidator; +import org.ovirt.engine.core.bll.validator.VmValidationUtils; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.VdcObjectType; @@ -459,9 +460,17 @@ return failCanDoAction(VdcBllMessages.QOS_CPU_SHARES_OUT_OF_RANGE); } - if (Boolean.TRUE.equals(getParameters().isVirtioScsiEnabled()) && - !FeatureSupported.virtIoScsi(getVdsGroup().getcompatibility_version())) { - return failCanDoAction(VdcBllMessages.VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL); + if (Boolean.TRUE.equals(getParameters().isVirtioScsiEnabled())) { + // Verify cluster compatibility + if (!FeatureSupported.virtIoScsi(getVdsGroup().getcompatibility_version())) { + return failCanDoAction(VdcBllMessages.VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL); + } + + // Verify OS compatibility + if (!VmHandler.isOsTypeSupportedForVirtioScsi(vmFromParams.getOs(), getVdsGroup().getcompatibility_version(), + getReturnValue().getCanDoActionMessages())) { + return false; + } } return true; @@ -1048,8 +1057,11 @@ protected boolean isVirtioScsiEnabled() { Boolean virtioScsiEnabled = getParameters().isVirtioScsiEnabled(); + boolean isOsSupportedForVirtIoScsi = VmValidationUtils.isOsSupportedForVirtIoScsi( + getParameters().getVm().getOs(), getVdsGroup().getcompatibility_version()); + return virtioScsiEnabled != null ? virtioScsiEnabled : - FeatureSupported.virtIoScsi(getVdsGroup().getcompatibility_version()); + FeatureSupported.virtIoScsi(getVdsGroup().getcompatibility_version()) && isOsSupportedForVirtIoScsi; } protected boolean hasWatchdog() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java index 921bef7..0790f54 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java @@ -367,9 +367,17 @@ return failCanDoAction(VdcBllMessages.QOS_CPU_SHARES_OUT_OF_RANGE); } - if (Boolean.TRUE.equals(getParameters().isVirtioScsiEnabled()) && - !FeatureSupported.virtIoScsi(getVdsGroup().getcompatibility_version())) { - return failCanDoAction(VdcBllMessages.VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL); + if (Boolean.TRUE.equals(getParameters().isVirtioScsiEnabled()) || isVirtioScsiEnabledForVm(getVmId())) { + // Verify cluster compatibility + if (!FeatureSupported.virtIoScsi(getVdsGroup().getcompatibility_version())) { + return failCanDoAction(VdcBllMessages.VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL); + } + + // Verify OS compatibility + if (!VmHandler.isOsTypeSupportedForVirtioScsi(vmFromParams.getOs(), getVdsGroup().getcompatibility_version(), + getReturnValue().getCanDoActionMessages())) { + return false; + } } if (Boolean.FALSE.equals(getParameters().isVirtioScsiEnabled())) { @@ -545,8 +553,11 @@ protected boolean isVirtioScsiEnabled() { Boolean virtioScsiEnabled = getParameters().isVirtioScsiEnabled(); - return virtioScsiEnabled != null ? virtioScsiEnabled : - VmDeviceUtils.isVirtioScsiControllerAttached(getVmId()); + return virtioScsiEnabled != null ? virtioScsiEnabled : isVirtioScsiEnabledForVm(getVmId()); + } + + public boolean isVirtioScsiEnabledForVm(Guid vmId) { + return VmDeviceUtils.isVirtioScsiControllerAttached(vmId); } protected boolean hasWatchdog() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java index ec7a2b7..5da78ca 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java @@ -392,6 +392,27 @@ } /** + * Check if the OS type is supported for VirtIO-SCSI. + * + * @param osId + * Type of the OS + * @param clusterVersion + * Cluster's version + * @param reasons + * Reasons List + * @return + */ + public static boolean isOsTypeSupportedForVirtioScsi(int osId, + Version clusterVersion, + List<String> reasons) { + boolean result = VmValidationUtils.isOsSupportedForVirtIoScsi(osId, clusterVersion); + if (!result) { + reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI.toString()); + } + return result; + } + + /** * Check if the interface name is not duplicate in the list of interfaces. * * @param interfaces diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java index 6df2a9e..b395558 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java @@ -221,9 +221,37 @@ } @Test + public void canAddVmWithVirtioScsiControllerNotSupportedOs() { + VM vm = createVm(); + AddVmFromTemplateCommand<AddVmFromTemplateParameters> cmd = createVmFromTemplateCommand(vm); + + mockOsRepository(); + mockStorageDomainDAOGetForStoragePool(); + mockVmTemplateDAOReturnVmTemplate(); + mockDiskImageDAOGetSnapshotById(); + mockVerifyAddVM(cmd); + mockConfig(); + mockConfigSizeDefaults(); + mockStorageDomainDaoGetAllStoragesForPool(20); + mockUninterestingMethods(cmd); + doReturn(true).when(cmd).checkCpuSockets(); + + doReturn(createVdsGroup()).when(cmd).getVdsGroup(); + cmd.getParameters().setVirtioScsiEnabled(true); + when(osRepository.getDiskInterfaces(any(Integer.class), any(Version.class))).thenReturn( + new ArrayList<>(Arrays.asList("VirtIO"))); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, + VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI); + } + + @Test public void isVirtioScsiEnabledDefaultedToTrue() { + mockOsRepository(); AddVmCommand<VmManagementParametersBase> cmd = setupCanAddVmTests(0, 0); doReturn(createVdsGroup()).when(cmd).getVdsGroup(); + when(osRepository.getDiskInterfaces(any(Integer.class), any(Version.class))).thenReturn( + new ArrayList<>(Arrays.asList("VirtIO_SCSI"))); assertTrue("isVirtioScsiEnabled hasn't been defaulted to true on cluster >= 3.3.", cmd.isVirtioScsiEnabled()); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java index 828bc92..84f365b 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java @@ -9,6 +9,7 @@ import static org.mockito.Mockito.when; import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -78,6 +79,9 @@ mockConfig(ConfigValues.MaxNumOfVmCpus, "3.0", 16), mockConfig(ConfigValues.MaxNumOfVmSockets, "3.0", 16), mockConfig(ConfigValues.MaxNumOfCpuPerSocket, "3.0", 16), + mockConfig(ConfigValues.MaxNumOfVmCpus, "3.3", 16), + mockConfig(ConfigValues.MaxNumOfVmSockets, "3.3", 16), + mockConfig(ConfigValues.MaxNumOfCpuPerSocket, "3.3", 16), mockConfig(ConfigValues.VirtIoScsiEnabled, Version.v3_3.toString(), true) ); @@ -111,6 +115,8 @@ } }); doReturn(vm).when(command).getVm(); + + doReturn(false).when(command).isVirtioScsiEnabledForVm(any(Guid.class)); } @Test @@ -235,6 +241,19 @@ VdcBllMessages.CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS); } + @Test + public void testCannotUpdateOSNotSupportVirtioScsi() { + prepareVmToPassCanDoAction(); + group.setcompatibility_version(Version.v3_3); + + when(command.isVirtioScsiEnabledForVm(any(Guid.class))).thenReturn(true); + when(osRepository.getDiskInterfaces(any(Integer.class), any(Version.class))).thenReturn( + new ArrayList<>(Arrays.asList("VirtIO"))); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, + VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI); + } + private void prepareVmToPassCanDoAction() { vmStatic.setName("vm1"); vmStatic.setMemSizeMb(256); 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 fd7125c..1e6d0a1 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 @@ -229,6 +229,7 @@ ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_DISPLAY_TYPE(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_NOT_SUPPORTED_BY_ARCHITECTURE_TYPE(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_ILLEGAL_WATCHDOG_MODEL_IS_NOT_SUPPORTED_BY_OS(ErrorType.BAD_PARAMETERS), + ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_OS_TYPE(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_INCOMPATIBLE_VERSION(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME(ErrorType.BAD_PARAMETERS), 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 1f13889..2515055 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -601,6 +601,7 @@ ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_DISPLAY_TYPE=Cannot ${action} ${type}. Cannot set single display device via VNC display. ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_NOT_SUPPORTED_BY_ARCHITECTURE_TYPE=Cannot ${action} ${type}. Selected operating system is not supported by the architecture. ACTION_TYPE_FAILED_ILLEGAL_WATCHDOG_MODEL_IS_NOT_SUPPORTED_BY_OS=Cannot ${action} ${type}. Selected watchdog model is not supported by the operating system. +ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI=Cannot ${action} ${type}. Selected operating system does not support VirtIO-SCSI. Please disable VirtIO-SCSI for the VM. ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_INCOMPATIBLE_VERSION=Cannot ${action} ${type}. Cluster does not support Single Qxl Pci devices. ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. Illegal Domain name: ${Domain}. Domain name has unsupported special character ${Char}. ACTION_TYPE_FAILED_ILLEGAL_ARCHITECTURE_TYPE_INCOMPATIBLE=Cannot ${action} ${type}. Architecture does not match the expected value. 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 17a88a0..6efe329 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 @@ -1627,6 +1627,9 @@ @DefaultStringValue("Cannot ${action} ${type}. Selected watchdog model is not supported by the operating system.") String ACTION_TYPE_FAILED_ILLEGAL_WATCHDOG_MODEL_IS_NOT_SUPPORTED_BY_OS(); + @DefaultStringValue("Cannot ${action} ${type}. Selected operating system does not support VirtIO-SCSI. Please disable VirtIO-SCSI for the VM.") + String ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI(); + @DefaultStringValue("Cannot ${action} ${type}. Cannot set single display device to non Linux operating system.") String ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_OS_TYPE(); 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 63c2905..3fb2e24 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 @@ -582,6 +582,7 @@ ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_DISPLAY_TYPE=Cannot ${action} ${type}. Cannot set single display device via VNC display. ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_NOT_SUPPORTED_BY_ARCHITECTURE_TYPE=Cannot ${action} ${type}. Selected operating system is not supported by the architecture. ACTION_TYPE_FAILED_ILLEGAL_WATCHDOG_MODEL_IS_NOT_SUPPORTED_BY_OS=Cannot ${action} ${type}. Selected watchdog model is not supported by the operating system. +ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI=Cannot ${action} ${type}. Selected operating system does not support VirtIO-SCSI. Please disable VirtIO-SCSI for the VM. ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_INCOMPATIBLE_VERSION=Cannot ${action} ${type}. Cluster does not support Single Qxl Pci devices. ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. Illegal Domain name: ${Domain}. Domain name has unsupported special character ${Char}. ACTION_TYPE_FAILED_ILLEGAL_ARCHITECTURE_TYPE_INCOMPATIBLE=Cannot ${action} ${type}. Architecture does not match the expected value. 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 d656ae3..7994be6 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 @@ -601,6 +601,7 @@ ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_DISPLAY_TYPE=Cannot ${action} ${type}. Cannot set single display device via VNC display. ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_NOT_SUPPORTED_BY_ARCHITECTURE_TYPE=Cannot ${action} ${type}. Selected operating system is not supported by the architecture. ACTION_TYPE_FAILED_ILLEGAL_WATCHDOG_MODEL_IS_NOT_SUPPORTED_BY_OS=Cannot ${action} ${type}. Selected watchdog model is not supported by the operating system. +ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_DOES_NOT_SUPPORT_VIRTIO_SCSI=Cannot ${action} ${type}. Selected operating system does not support VirtIO-SCSI. Please disable VirtIO-SCSI for the VM. ACTION_TYPE_FAILED_ILLEGAL_SINGLE_DEVICE_INCOMPATIBLE_VERSION=Cannot ${action} ${type}. Cluster does not support Single Qxl Pci devices. ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. Illegal Domain name: ${Domain}. Domain name has unsupported special character ${Char}. ACTION_TYPE_FAILED_ILLEGAL_ARCHITECTURE_TYPE_INCOMPATIBLE=Cannot ${action} ${type}. Architecture does not match the expected value. -- To view, visit http://gerrit.ovirt.org/22648 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id9907d35b53c0447662bc638146c4c61ac5cce8a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
