Arik Hadas has uploaded a new change for review. Change subject: restapi,engine: fix usb mapping on update vm call ......................................................................
restapi,engine: fix usb mapping on update vm call Change the way usb policy is set on update vm request that is received through rest api, according to the mapping table attached in the bug-url. Specifically, the new mapping states that if no usb setting is defined in the update vm request, the previous usb policy will remain as is (instead of being set to DISABLED as it was before), thus it solves bug #875881. The new mapping also states that if the current usb policy is set to DISABLED and the update vm request is going to enable it but no explicit policy is declared (either legacy or native), then an error should be raised. thus, in this case a null policy is sent to the engine and the engine returns an appropriate error. Change-Id: If74bf6ede2fc0f932db8dfcd7951b6bcf90b7744 Bug-Url: https://bugzilla.redhat.com/875881 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/UsbResourceUtils.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/utils/test/UsbResourceUtilsTest.java M backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java M backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java 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 14 files changed, 258 insertions(+), 61 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/92/9792/1 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 89a28f0..4139073 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 @@ -422,7 +422,10 @@ VDSGroup vdsGroup, List<String> messages) { boolean retVal = true; - if (UsbPolicy.ENABLED_NATIVE.equals(usbPolicy)) { + if (usbPolicy == null) { + messages.add(VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_USB_POLICY.toString()); + retVal = false; + } else if (UsbPolicy.ENABLED_NATIVE.equals(usbPolicy)) { if (!Config.<Boolean> GetValue(ConfigValues.NativeUSBEnabled, vdsGroup.getcompatibility_version() .getValue())) { messages.add(VdcBllMessages.USB_NATIVE_SUPPORT_ONLY_AVAILABLE_ON_CLUSTER_LEVEL.toString()); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java index 5128be3..f43acb0 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java @@ -151,6 +151,7 @@ ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE, ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS, ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME, + ACTION_TYPE_FAILED_ILLEGAL_USB_POLICY, ACTION_TYPE_FAILED_MAX_NUM_CPU, ACTION_TYPE_FAILED_MAX_NUM_SOCKETS, ACTION_TYPE_FAILED_MIN_NUM_SOCKETS, 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 379e566..23d3156 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -479,6 +479,7 @@ ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE=Cannot ${action} ${type}. Illegal memory size is provided, size needs to be between ${minMemorySize} MB and ${maxMemorySize} MB. ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS=Cannot ${action} ${type}. Illegal number of monitors is provided, max allowed number of monitors is 1 for VNC and the max number in the ValidNumOfMonitors configuration variable for Spice. ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. Illegal Domain name: ${Domain}. Domain name has unsupported special character ${Char}. +ACTION_TYPE_FAILED_ILLEGAL_USB_POLICY=Cannot ${action} ${type}. Illegal USB policy is provided, cannot determine whether a legacy or native USB policy should be set. ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION=Cannot ${action} ${type}. Compatibility version is invalid, previous value restored. ACTION_TYPE_FAILED_GIVEN_VERSION_NOT_SUPPORTED=Cannot ${action} ${type}. Selected Compatibility Version is not supported. NETWORK_ADDR_MANDATORY_IN_STATIC_IP=Netwrok address must be specify when using static ip diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java index b36d519..8d7294d 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java @@ -2,6 +2,7 @@ import javax.ws.rs.core.Response; + import org.ovirt.engine.api.model.Action; import org.ovirt.engine.api.model.CdRom; import org.ovirt.engine.api.model.CdRoms; @@ -21,15 +22,11 @@ import org.ovirt.engine.core.common.action.UpdateVmTemplateParameters; import org.ovirt.engine.core.common.action.VdcActionParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; -import org.ovirt.engine.core.common.businessentities.UsbPolicy; -import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.queries.GetPermissionsForObjectParameters; -import org.ovirt.engine.core.common.queries.GetVdsGroupByVdsGroupIdParameters; -import org.ovirt.engine.core.common.queries.GetVmTemplatesDisksParameters; import org.ovirt.engine.core.common.queries.GetVmTemplateParameters; +import org.ovirt.engine.core.common.queries.GetVmTemplatesDisksParameters; import org.ovirt.engine.core.common.queries.VdcQueryType; -import org.ovirt.engine.core.compat.Guid; public class BackendTemplateResource extends AbstractBackendActionableResource<Template, VmTemplate> @@ -112,17 +109,9 @@ @Override public VdcActionParametersBase getParameters(Template incoming, VmTemplate entity) { VmTemplate updated = getMapper(modelType, VmTemplate.class).map(incoming, entity); - UsbPolicy usbPolicy = UsbResourceUtils.getUsbPolicy(incoming.getUsb(), lookupCluster(updated.getvds_group_id())); - if (usbPolicy != null) { - updated.setusb_policy(usbPolicy); - } + updated.setusb_policy(UsbResourceUtils.getUsbPolicyOnUpdate(incoming.getUsb(), entity.getusb_policy())); return new UpdateVmTemplateParameters(updated); } } - - private VDSGroup lookupCluster(Guid id) { - return getEntity(VDSGroup.class, VdcQueryType.GetVdsGroupByVdsGroupId, new GetVdsGroupByVdsGroupIdParameters(id), "GetVdsGroupByVdsGroupId"); - } - } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java index 0771225..bab797f 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java @@ -17,7 +17,6 @@ import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VmTemplateParametersBase; import org.ovirt.engine.core.common.businessentities.DiskImage; -import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.VmTemplate; @@ -63,10 +62,7 @@ staticVm.setvds_group_id(getClusterId(template)); } - UsbPolicy usbPolicy = UsbResourceUtils.getUsbPolicy(template.getUsb(), lookupCluster(staticVm.getvds_group_id())); - if (usbPolicy != null) { - staticVm.setusb_policy(usbPolicy); - } + staticVm.setusb_policy(UsbResourceUtils.getUsbPolicyOnCreate(template.getUsb(), lookupCluster(staticVm.getvds_group_id()))); // REVISIT: powershell has a IsVmTemlateWithSameNameExist safety check AddVmTemplateParameters params = new AddVmTemplateParameters(staticVm, diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java index 43d3704..f77ee44 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java @@ -50,7 +50,6 @@ import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.action.VmOperationParameterBase; -import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VmStatic; @@ -58,7 +57,6 @@ import org.ovirt.engine.core.common.interfaces.SearchType; import org.ovirt.engine.core.common.queries.GetAllDisksByVmIdParameters; import org.ovirt.engine.core.common.queries.GetPermissionsForObjectParameters; -import org.ovirt.engine.core.common.queries.GetVdsGroupByVdsGroupIdParameters; import org.ovirt.engine.core.common.queries.GetVmByVmIdParameters; import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; import org.ovirt.engine.core.common.queries.VdcQueryType; @@ -380,10 +378,7 @@ VmStatic updated = getMapper(modelType, VmStatic.class).map(incoming, entity.getStaticData()); - UsbPolicy usbPolicy = UsbResourceUtils.getUsbPolicy(incoming.getUsb(), lookupCluster(updated.getvds_group_id())); - if (usbPolicy != null) { - updated.setusb_policy(usbPolicy); - } + updated.setusb_policy(UsbResourceUtils.getUsbPolicyOnUpdate(incoming.getUsb(), entity.getUsbPolicy())); VmManagementParametersBase params = new VmManagementParametersBase(updated); @@ -395,10 +390,6 @@ } return params; } - } - - private VDSGroup lookupCluster(Guid id) { - return getEntity(VDSGroup.class, VdcQueryType.GetVdsGroupByVdsGroupId, new GetVdsGroupByVdsGroupIdParameters(id), "GetVdsGroupByVdsGroupId"); } @Override diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java index c8862aa..c516773 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java @@ -34,7 +34,6 @@ import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.businessentities.DiskImage; -import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VmPayload; @@ -90,10 +89,9 @@ if (namedCluster(vm)) { staticVm.setvds_group_id(getClusterId(vm)); } - UsbPolicy usbPolicy = UsbResourceUtils.getUsbPolicy(vm.getUsb(), lookupCluster(staticVm.getvds_group_id())); - if (usbPolicy != null) { - staticVm.setusb_policy(usbPolicy); - } + + staticVm.setusb_policy(UsbResourceUtils.getUsbPolicyOnCreate(vm.getUsb(), lookupCluster(staticVm.getvds_group_id()))); + if (!isFiltered()) { // if the user set the host-name within placement-policy, rather than the host-id (legal) - // resolve the host's ID, because it will be needed down the line diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/UsbResourceUtils.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/UsbResourceUtils.java index bc6a3e3..4de77d8 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/UsbResourceUtils.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/UsbResourceUtils.java @@ -7,7 +7,7 @@ import org.ovirt.engine.core.compat.Version; public class UsbResourceUtils { - public static UsbPolicy getUsbPolicy(Usb usb, VDSGroup vdsGroup) { + public static UsbPolicy getUsbPolicyOnCreate(Usb usb, VDSGroup vdsGroup) { UsbPolicy usbPolicy = null; if (usb == null || !usb.isSetEnabled() || !usb.isEnabled()) { usbPolicy = UsbPolicy.DISABLED; @@ -29,4 +29,52 @@ } return usbPolicy; } + + public static UsbPolicy getUsbPolicyOnUpdate(Usb usb, UsbPolicy currentPolicy) { + if (usb == null) { + return currentPolicy; + } + + if (!usb.isSetEnabled()) { + if (usb.isSetType()) { + if (currentPolicy == UsbPolicy.DISABLED) { + return currentPolicy; + } + else { + UsbType usbType = UsbType.fromValue(usb.getType()); + return mapUsbTypeToPolicy(usbType); + } + } + else { + return currentPolicy; + } + } + else { + if (!usb.isEnabled()) + return UsbPolicy.DISABLED; + else { + if (usb.isSetType()) { + UsbType usbType = UsbType.fromValue(usb.getType()); + return mapUsbTypeToPolicy(usbType); + } + else { + return currentPolicy == UsbPolicy.DISABLED ? null : currentPolicy; + } + } + } + } + + private static UsbPolicy mapUsbTypeToPolicy(UsbType usbType) { + if (usbType == null) + return null; + + switch (usbType) { + case LEGACY: + return UsbPolicy.ENABLED_LEGACY; + case NATIVE: + return UsbPolicy.ENABLED_NATIVE; + default: + return null; + } + } } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/utils/test/UsbResourceUtilsTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/utils/test/UsbResourceUtilsTest.java index 7ae2209..9cb719f 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/utils/test/UsbResourceUtilsTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/utils/test/UsbResourceUtilsTest.java @@ -1,6 +1,7 @@ package org.ovirt.engine.api.restapi.resource.utils.test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import org.junit.Test; import org.ovirt.engine.api.model.Usb; @@ -14,14 +15,14 @@ public void getUsbPolicyNullUsb() { Usb usb = null; VDSGroup vdsGroup = new VDSGroup(); - assertEquals(UsbResourceUtils.getUsbPolicy(usb, vdsGroup), UsbPolicy.DISABLED); + assertEquals(UsbResourceUtils.getUsbPolicyOnCreate(usb, vdsGroup), UsbPolicy.DISABLED); } @Test public void getUsbPolicyIsSetDisabled() { Usb usb = new Usb(); VDSGroup vdsGroup = new VDSGroup(); - assertEquals(UsbResourceUtils.getUsbPolicy(usb, vdsGroup), UsbPolicy.DISABLED); + assertEquals(UsbResourceUtils.getUsbPolicyOnCreate(usb, vdsGroup), UsbPolicy.DISABLED); } @Test @@ -29,7 +30,7 @@ Usb usb = new Usb(); usb.setEnabled(false); VDSGroup vdsGroup = new VDSGroup(); - assertEquals(UsbResourceUtils.getUsbPolicy(usb, vdsGroup), UsbPolicy.DISABLED); + assertEquals(UsbResourceUtils.getUsbPolicyOnCreate(usb, vdsGroup), UsbPolicy.DISABLED); } @Test @@ -38,7 +39,7 @@ usb.setEnabled(true); usb.setType("native"); VDSGroup vdsGroup = new VDSGroup(); - assertEquals(UsbResourceUtils.getUsbPolicy(usb, vdsGroup), UsbPolicy.ENABLED_NATIVE); + assertEquals(UsbResourceUtils.getUsbPolicyOnCreate(usb, vdsGroup), UsbPolicy.ENABLED_NATIVE); } @Test @@ -47,7 +48,7 @@ usb.setEnabled(true); usb.setType("legacy"); VDSGroup vdsGroup = new VDSGroup(); - assertEquals(UsbResourceUtils.getUsbPolicy(usb, vdsGroup), UsbPolicy.ENABLED_LEGACY); + assertEquals(UsbResourceUtils.getUsbPolicyOnCreate(usb, vdsGroup), UsbPolicy.ENABLED_LEGACY); } @Test @@ -56,7 +57,7 @@ usb.setEnabled(true); VDSGroup vdsGroup = new VDSGroup(); vdsGroup.setcompatibility_version(Version.v3_1); - assertEquals(UsbResourceUtils.getUsbPolicy(usb, vdsGroup), UsbPolicy.ENABLED_NATIVE); + assertEquals(UsbResourceUtils.getUsbPolicyOnCreate(usb, vdsGroup), UsbPolicy.ENABLED_NATIVE); } @Test @@ -65,7 +66,175 @@ usb.setEnabled(true); VDSGroup vdsGroup = new VDSGroup(); vdsGroup.setcompatibility_version(Version.v3_0); - assertEquals(UsbResourceUtils.getUsbPolicy(usb, vdsGroup), UsbPolicy.ENABLED_LEGACY); + assertEquals(UsbResourceUtils.getUsbPolicyOnCreate(usb, vdsGroup), UsbPolicy.ENABLED_LEGACY); } + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotNullUsb() { + Usb usb = null; + UsbPolicy currentPolicy = UsbPolicy.DISABLED; + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, currentPolicy), currentPolicy); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyLegacyGotNullUsb() { + Usb usb = null; + UsbPolicy currentPolicy = UsbPolicy.ENABLED_LEGACY; + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, currentPolicy), currentPolicy); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotNullUsb() { + Usb usb = null; + UsbPolicy currentPolicy = UsbPolicy.ENABLED_NATIVE; + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, currentPolicy), currentPolicy); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotEnabledNotSetPolicyNotSetUsb() { + Usb usb = new Usb(); + UsbPolicy currentPolicy = UsbPolicy.DISABLED; + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, currentPolicy), currentPolicy); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyLegacyGotEnabledNotSetPolicyNotSetUsb() { + Usb usb = new Usb(); + UsbPolicy currentPolicy = UsbPolicy.ENABLED_LEGACY; + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, currentPolicy), currentPolicy); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotEnabledNotSetPolicyNotSetUsb() { + Usb usb = new Usb(); + UsbPolicy currentPolicy = UsbPolicy.ENABLED_NATIVE; + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, currentPolicy), currentPolicy); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotEnabledNotSetLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.DISABLED), UsbPolicy.DISABLED); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotEnabledNotSetNativePolicyUsb() { + Usb usb = new Usb(); + usb.setType("native"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.DISABLED), UsbPolicy.DISABLED); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyLegacyGotEnabledNotSetLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_LEGACY), UsbPolicy.ENABLED_LEGACY); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotEnabledNotSetLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_NATIVE), UsbPolicy.ENABLED_LEGACY); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyLegacyGotEnabledNotSetNativePolicyUsb() { + Usb usb = new Usb(); + usb.setType("native"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_LEGACY), UsbPolicy.ENABLED_NATIVE); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotEnabledNotSetNativePolicyUsb() { + Usb usb = new Usb(); + usb.setType("native"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_NATIVE), UsbPolicy.ENABLED_NATIVE); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotDisabledLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(false); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.DISABLED), UsbPolicy.DISABLED); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotDisabledNativePolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(false); + usb.setType("native"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.DISABLED), UsbPolicy.DISABLED); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyLegacyGotDisabledLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(false); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_LEGACY), UsbPolicy.DISABLED); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotDisabledLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(false); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_NATIVE), UsbPolicy.DISABLED); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotEnabledPolicyNotSetUsb() { + Usb usb = new Usb(); + usb.setEnabled(true); + assertNull(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.DISABLED)); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyLegacyGotEnabledPolicyNotSetUsb() { + Usb usb = new Usb(); + usb.setEnabled(true); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_LEGACY), UsbPolicy.ENABLED_LEGACY); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotEnabledPolicyNotSetUsb() { + Usb usb = new Usb(); + usb.setEnabled(true); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_NATIVE), UsbPolicy.ENABLED_NATIVE); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotEnabledLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(true); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_NATIVE), UsbPolicy.ENABLED_LEGACY); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotEnabledLegacyPolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(true); + usb.setType("legacy"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.DISABLED), UsbPolicy.ENABLED_LEGACY); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyNativeGotEnabledNativePolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(true); + usb.setType("native"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.ENABLED_NATIVE), UsbPolicy.ENABLED_NATIVE); + } + + @Test + public void getUsbPolicyOnUpdateCurrentlyDisabledGotEnabledNativePolicyUsb() { + Usb usb = new Usb(); + usb.setEnabled(true); + usb.setType("native"); + assertEquals(UsbResourceUtils.getUsbPolicyOnUpdate(usb, UsbPolicy.DISABLED), UsbPolicy.ENABLED_NATIVE); + } } diff --git a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java index 5d93e91..80736ae 100644 --- a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java +++ b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java @@ -3,8 +3,8 @@ import org.ovirt.engine.api.common.util.StatusUtils; import org.ovirt.engine.api.common.util.TimeZoneMapping; import org.ovirt.engine.api.model.Boot; -import org.ovirt.engine.api.model.Cluster; import org.ovirt.engine.api.model.CPU; +import org.ovirt.engine.api.model.Cluster; import org.ovirt.engine.api.model.CpuTopology; import org.ovirt.engine.api.model.Display; import org.ovirt.engine.api.model.DisplayType; @@ -19,10 +19,9 @@ import org.ovirt.engine.api.model.VmType; import org.ovirt.engine.api.restapi.utils.UsbMapperUtils; import org.ovirt.engine.core.common.businessentities.OriginType; -import org.ovirt.engine.core.common.businessentities.UsbPolicy; -import org.ovirt.engine.core.common.businessentities.VmTemplateStatus; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.VmTemplate; +import org.ovirt.engine.core.common.businessentities.VmTemplateStatus; import org.ovirt.engine.core.compat.Guid; public class TemplateMapper { @@ -31,6 +30,8 @@ @Mapping(from = Template.class, to = VmTemplate.class) public static VmTemplate map(Template model, VmTemplate incoming) { + // USB policy will be updated outside of this method because there's + // a different mapping for it on create and update VM operations VmTemplate entity = incoming != null ? incoming : new VmTemplate(); if (model.isSetName()) { entity.setname(model.getName()); @@ -120,9 +121,6 @@ } if (model.isSetTimezone()) { entity.settime_zone(TimeZoneMapping.getWindows(model.getTimezone())); - } - if (model.isSetUsb() && model.getUsb().isSetEnabled()) { - entity.setusb_policy(model.getUsb().isEnabled() ? UsbPolicy.ENABLED_LEGACY : UsbPolicy.DISABLED); } return entity; } diff --git a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java index 7ed8606..9fb5d51 100644 --- a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java +++ b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java @@ -1,5 +1,7 @@ package org.ovirt.engine.api.restapi.types; +import static org.ovirt.engine.core.compat.NGuid.createGuidFromString; + import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; @@ -23,9 +25,11 @@ import org.ovirt.engine.api.model.Host; import org.ovirt.engine.api.model.IP; import org.ovirt.engine.api.model.IPs; +import org.ovirt.engine.api.model.MemoryPolicy; import org.ovirt.engine.api.model.OperatingSystem; import org.ovirt.engine.api.model.OsType; import org.ovirt.engine.api.model.Payload; +import org.ovirt.engine.api.model.PayloadFile; import org.ovirt.engine.api.model.Quota; import org.ovirt.engine.api.model.Template; import org.ovirt.engine.api.model.Usb; @@ -33,30 +37,25 @@ import org.ovirt.engine.api.model.VCpuPin; import org.ovirt.engine.api.model.VM; import org.ovirt.engine.api.model.VmAffinity; -import org.ovirt.engine.api.model.PayloadFile; import org.ovirt.engine.api.model.VmPlacementPolicy; -import org.ovirt.engine.api.model.MemoryPolicy; import org.ovirt.engine.api.model.VmPool; import org.ovirt.engine.api.model.VmStatus; import org.ovirt.engine.api.model.VmType; -import org.ovirt.engine.core.common.utils.VmDeviceType; +import org.ovirt.engine.api.restapi.utils.CustomPropertiesParser; +import org.ovirt.engine.api.restapi.utils.UsbMapperUtils; import org.ovirt.engine.core.common.action.RunVmOnceParams; import org.ovirt.engine.core.common.businessentities.BootSequence; import org.ovirt.engine.core.common.businessentities.MigrationSupport; import org.ovirt.engine.core.common.businessentities.OriginType; -import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.businessentities.VmOsType; +import org.ovirt.engine.core.common.businessentities.VmPayload; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.VmTemplate; -import org.ovirt.engine.core.common.businessentities.VmPayload; +import org.ovirt.engine.core.common.utils.VmDeviceType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.NGuid; import org.ovirt.engine.core.compat.StringHelper; -import org.ovirt.engine.api.restapi.utils.CustomPropertiesParser; -import org.ovirt.engine.api.restapi.utils.UsbMapperUtils; - -import static org.ovirt.engine.core.compat.NGuid.createGuidFromString; public class VmMapper { @@ -99,6 +98,8 @@ } @Mapping(from = VM.class, to = VmStatic.class) public static VmStatic map(VM vm, VmStatic template) { + // USB policy will be updated outside of this method because there's + // a different mapping for it on create and update VM operations VmStatic staticVm = template != null ? template : new VmStatic(); if (vm.isSetName()) { staticVm.setvm_name(vm.getName()); @@ -216,9 +217,6 @@ } if (vm.isSetCustomProperties() && vm.getCustomProperties().isSetCustomProperty()) { staticVm.setCustomProperties(CustomPropertiesParser.parse(vm.getCustomProperties().getCustomProperty())); - } - if (vm.isSetUsb() && vm.getUsb().isSetEnabled()) { - staticVm.setusb_policy(vm.getUsb().isEnabled() ? UsbPolicy.ENABLED_LEGACY : UsbPolicy.DISABLED); } if (vm.isSetQuota() && vm.getQuota().isSetId()) { staticVm.setQuotaId(new Guid(vm.getQuota().getId())); 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 517a7e6..cb3c230 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 @@ -1268,6 +1268,9 @@ @DefaultStringValue("Cannot ${action} ${type}. Illegal Domain name: ${Domain}. Domain name has unsupported special character ${Char}.") String ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME(); + @DefaultStringValue("Cannot ${action} ${type}. Illegal USB policy is provided, cannot determine whether a legacy or native USB policy should be set.") + String ACTION_TYPE_FAILED_ILLEGAL_USB_POLICY(); + @DefaultStringValue("Cannot ${action} ${type}. Compatibility version is invalid, previous value restored.") String ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION(); 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 3a5d18f..58e258e 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 @@ -475,6 +475,7 @@ ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE=Cannot ${action} ${type}. Illegal memory size is provided, size needs to be between ${minMemorySize} MB and ${maxMemorySize} MB. ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS=Cannot ${action} ${type}. Illegal number of monitors is provided, max allowed number of monitors is 1 for VNC and the max number in the ValidNumOfMonitors configuration variable for Spice. ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. Illegal Domain name: ${Domain}. Domain name has unsupported special character ${Char}. +ACTION_TYPE_FAILED_ILLEGAL_USB_POLICY=Cannot ${action} ${type}. Illegal USB policy is provided, cannot determine whether a legacy or native USB policy should be set. ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION=Cannot ${action} ${type}. Compatibility version is invalid, previous value restored. ACTION_TYPE_FAILED_GIVEN_VERSION_NOT_SUPPORTED=Cannot ${action} ${type}. Selected Compatibility Version is not supported. NETWORK_ADDR_MANDATORY_IN_STATIC_IP=Network address must be specified when using static ip 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 bf8f913..3a57535 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 @@ -473,6 +473,7 @@ ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE=Cannot ${action} ${type}. Illegal memory size is provided, size needs to be between ${minMemorySize} MB and ${maxMemorySize} MB. ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS=Cannot ${action} ${type}. Illegal number of monitors is provided, max allowed number of monitors is 1 for VNC and the max number in the ValidNumOfMonitors configuration variable for Spice. ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. Illegal Domain name: ${Domain}. Domain name has unsupported special character ${Char}. +ACTION_TYPE_FAILED_ILLEGAL_USB_POLICY=Cannot ${action} ${type}. Illegal USB policy is provided, cannot determine whether a legacy or native USB policy should be set. ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION=Cannot ${action} ${type}. Compatibility version is invalid, previous value restored. ACTION_TYPE_FAILED_GIVEN_VERSION_NOT_SUPPORTED=Cannot ${action} ${type}. Selected Compatibility Version is not supported. NETWORK_ADDR_MANDATORY_IN_STATIC_IP=Network address must be specified when using static ip -- To view, visit http://gerrit.ovirt.org/9792 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If74bf6ede2fc0f932db8dfcd7951b6bcf90b7744 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
