Tomas Jelinek has uploaded a new change for review. Change subject: restapi: made sure only the supported template entities are copied ......................................................................
restapi: made sure only the supported template entities are copied The blank template is now editable. It is not connected to any cluster (similar to instance types) and it supportrs all options from the highest cluster version. When a VM or pool is created from this template in the older cluster, it is important not to copy fields which are not supported on the target cluster. Change-Id: I7430bca85b95ddf97eaa93abbcdf13885e017f9e Bug-Url: https://bugzilla.redhat.com/1130174 Signed-off-by: Tomas Jelinek <[email protected]> --- M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolsResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java A backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/VmMapperMockConfigUtils.java M backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java 6 files changed, 96 insertions(+), 39 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/08/38008/1 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 9b0a65d..cef794a 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 @@ -113,7 +113,10 @@ Guid templateId = getTemplateId(vm.getTemplate()); VmTemplate templateEntity = lookupTemplate(templateId); - VmStatic builtFromTemplate = getMapper(VmTemplate.class, VmStatic.class).map(templateEntity, null); + + VDSGroup cluster = getCluster(vm); + + VmStatic builtFromTemplate = VmMapper.map(templateEntity, null, cluster.getCompatibilityVersion()); // if VM is based on a template, and going to be on another cluster then template, clear the cpu_profile // since the template cpu_profile doesn't match cluster. if (!vm.isSetCpuProfile() && vm.isSetCluster() @@ -132,10 +135,8 @@ VmStatic staticVm = getMapper(VM.class, VmStatic.class).map(vm, builtFromInstanceType != null ? builtFromInstanceType : builtFromTemplate); if (namedCluster(vm)) { - staticVm.setVdsGroupId(getClusterId(vm)); + staticVm.setVdsGroupId(cluster.getId()); } - - VDSGroup cluster = lookupCluster(staticVm.getVdsGroupId()); if (Guid.Empty.equals(templateId) && !vm.isSetOs()) { staticVm.setOsId(OsRepository.AUTO_SELECT_OS); @@ -245,7 +246,7 @@ VmMapper.map(vm, vmConfiguration.getStaticData()); - Guid clusterId = namedCluster(vm) ? getClusterId(vm) : asGuid(vm.getCluster().getId()); + Guid clusterId = namedCluster(vm) ? getCluster(vm).getId() : asGuid(vm.getCluster().getId()); ImportVmParameters parameters = new ImportVmParameters(); parameters.setVm(vmConfiguration); parameters.setVdsGroupId(clusterId); @@ -396,7 +397,8 @@ /** * Returns true if the device should be copied from the template or instance type * If the instance type is selected, than the device will be copied from the instance type only if the device is compatible with the cluster and os - * If the instance type is not set and the template is set, than it is copied from the template (e.g. the cluster compatibility is not checked since the template lives in a cluster) + * If the instance type is not set and the template is set the + * compatibility has to be checked as well because the blank template can be set which does not live on a cluster */ private boolean shouldCopyDevice(boolean isCompatibleWithCluster, Guid templateId, Guid instanceTypeId) { if (instanceTypeId == null && templateId == null) { @@ -404,18 +406,8 @@ return false; } - if (instanceTypeId == null && templateId != null) { - // template is set and is not overridden by instance type, copy device config - return true; - } - - if (instanceTypeId != null && isCompatibleWithCluster) { - // copy from instance type and the device is compatible with cluster, copy - return true; - } - - // not compatible with the cluster, do not copy from instance type - return false; + // copy only if compatible with cluster (instance type or blank template can contain unsupported devices) + return isCompatibleWithCluster; } private HashMap<Guid, DiskImage> getDisksToClone(Disks disks, Guid templateId) { @@ -664,11 +656,15 @@ return vm.isSetCluster() && vm.getCluster().isSetName() && !vm.getCluster().isSetId(); } - protected Guid getClusterId(VM vm) { - return isFiltered() ? lookupClusterByName(vm.getCluster().getName()).getId() : getEntity(VDSGroup.class, - VdcQueryType.GetVdsGroupByName, - new NameQueryParameters(vm.getCluster().getName()), - "Cluster: name=" + vm.getCluster().getName()).getId(); + protected VDSGroup getCluster(VM vm) { + if (namedCluster(vm)) { + return isFiltered() ? lookupClusterByName(vm.getCluster().getName()) : getEntity(VDSGroup.class, + VdcQueryType.GetVdsGroupByName, + new NameQueryParameters(vm.getCluster().getName()), + "Cluster: name=" + vm.getCluster().getName()); + } + + return lookupCluster(asGuid(vm.getCluster().getId())); } public VDSGroup lookupClusterByName(String name) { diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResourceTest.java index 3e3e1c2..10b777d 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResourceTest.java @@ -31,6 +31,7 @@ import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.Version; public abstract class AbstractBackendCollectionResourceTest<R extends BaseResource, Q /* extends IVdcQueryable */, C extends AbstractBackendCollectionResource<R, Q>> extends AbstractBackendResourceTest<R, Q> { @@ -280,6 +281,7 @@ protected VDSGroup setUpVDSGroup(Guid id) { VDSGroup cluster = control.createMock(VDSGroup.class); expect(cluster.getId()).andReturn(id).anyTimes(); + expect(cluster.getCompatibilityVersion()).andReturn(Version.getLast()).anyTimes(); return cluster; } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolsResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolsResourceTest.java index e4bfed3..7247df4 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolsResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolsResourceTest.java @@ -10,6 +10,7 @@ import org.ovirt.engine.core.common.businessentities.VmPoolType; import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.businessentities.VmType; +import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.interfaces.SearchType; import org.ovirt.engine.core.common.queries.GetVmTemplateParameters; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -30,6 +31,12 @@ } @Override + protected void init() { + super.init(); + Config.setConfigUtils(new VmMapperMockConfigUtils()); + } + + @Override protected List<VmPool> getCollection() { return collection.list().getVmPools(); } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java index ee23f9f..28dd418 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java @@ -55,6 +55,7 @@ import org.ovirt.engine.core.common.businessentities.VmPayload; import org.ovirt.engine.core.common.businessentities.VmStatistics; import org.ovirt.engine.core.common.businessentities.VmType; +import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.interfaces.SearchType; import org.ovirt.engine.core.common.osinfo.OsRepository; import org.ovirt.engine.core.common.queries.GetVmFromConfigurationQueryParameters; @@ -88,6 +89,8 @@ OsTypeMockUtils.mockOsTypes(); osRepository = control.createMock(OsRepository.class); SimpleDependecyInjector.getInstance().bind(OsRepository.class, osRepository); + + Config.setConfigUtils(new VmMapperMockConfigUtils()); } @Test @@ -456,11 +459,6 @@ new String[] { "Id" }, new Object[] { GUIDS[0] }, getEntity(0)); - setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByVdsGroupId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[1] }, - getVdsGroupEntity()); setUpEntityQueryExpectations(VdcQueryType.GetVmTemplate, GetVmTemplateParameters.class, new String[] { "Id" }, @@ -1052,11 +1050,6 @@ new String[] { "Id" }, new Object[] { GUIDS[1] }, getTemplateEntity(0)); - setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByVdsGroupId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[2] }, - getVdsGroupEntity()); setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByName, NameQueryParameters.class, new String[] { "Name" }, @@ -1524,6 +1517,7 @@ protected org.ovirt.engine.core.common.businessentities.VDSGroup getVdsGroupEntity() { VDSGroup cluster = new VDSGroup(); cluster.setArchitecture(ArchitectureType.x86_64); + cluster.setCompatibilityVersion(Version.getLast()); return cluster; } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/VmMapperMockConfigUtils.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/VmMapperMockConfigUtils.java new file mode 100644 index 0000000..f301428 --- /dev/null +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/VmMapperMockConfigUtils.java @@ -0,0 +1,36 @@ +package org.ovirt.engine.api.restapi.resource; + +import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.config.DataType; +import org.ovirt.engine.core.utils.ConfigUtilsBase; + +import java.util.Arrays; +import java.util.List; + +public class VmMapperMockConfigUtils extends ConfigUtilsBase { + List<ConfigValues> booleanValues = Arrays.asList( + ConfigValues.SerialNumberPolicySupported, + ConfigValues.SpiceFileTransferToggleSupported, + ConfigValues.SpiceCopyPasteToggleSupported, + ConfigValues.AutoConvergenceSupported, + ConfigValues.MigrationCompressionSupported + ); + + @Override + protected void setValue(String name, String value, String version) { + + } + + @Override + protected Object getValue(DataType type, String name, String defaultValue) { + return Boolean.TRUE; + } + + @Override + public <T> T getValue(ConfigValues configValue, String version) { + if (booleanValues.contains(configValue)) { + return (T) Boolean.TRUE; + } + return null; + } +} 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 8275372b..ffef9e3 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 @@ -55,6 +55,7 @@ import org.ovirt.engine.api.model.VmType; import org.ovirt.engine.api.restapi.utils.CustomPropertiesParser; import org.ovirt.engine.api.restapi.utils.GuidUtils; +import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.action.RunVmOnceParams; import org.ovirt.engine.core.common.businessentities.BootSequence; import org.ovirt.engine.core.common.businessentities.GraphicsInfo; @@ -86,7 +87,12 @@ // REVISIT once #712661 implemented by BE @Mapping(from = VmTemplate.class, to = VmStatic.class) public static VmStatic map(VmTemplate entity, VmStatic template) { + return map(entity, template, null); + } + + public static VmStatic map(VmTemplate entity, VmStatic template, Version version) { VmStatic staticVm = template != null ? template : new VmStatic(); + Version clusterVersion = version == null ? Version.getLast() : version; staticVm.setId(Guid.Empty); staticVm.setVmtGuid(entity.getId()); @@ -107,14 +113,30 @@ staticVm.setAllowConsoleReconnect(entity.isAllowConsoleReconnect()); staticVm.setVncKeyboardLayout(entity.getVncKeyboardLayout()); staticVm.setVmInit(entity.getVmInit()); - staticVm.setSerialNumberPolicy(entity.getSerialNumberPolicy()); - staticVm.setCustomSerialNumber(entity.getCustomSerialNumber()); - staticVm.setSpiceFileTransferEnabled(entity.isSpiceFileTransferEnabled()); - staticVm.setSpiceCopyPasteEnabled(entity.isSpiceCopyPasteEnabled()); + + if (FeatureSupported.serialNumberPolicy(clusterVersion)) { + staticVm.setSerialNumberPolicy(entity.getSerialNumberPolicy()); + staticVm.setCustomSerialNumber(entity.getCustomSerialNumber()); + } + + if (FeatureSupported.isSpiceFileTransferToggleSupported(clusterVersion)) { + staticVm.setSpiceFileTransferEnabled(entity.isSpiceFileTransferEnabled()); + } + + if (FeatureSupported.isSpiceCopyPasteToggleSupported(clusterVersion)) { + staticVm.setSpiceCopyPasteEnabled(entity.isSpiceCopyPasteEnabled()); + } + staticVm.setRunAndPause(entity.isRunAndPause()); staticVm.setCpuProfileId(entity.getCpuProfileId()); - staticVm.setAutoConverge(entity.getAutoConverge()); - staticVm.setMigrateCompressed(entity.getMigrateCompressed()); + if (FeatureSupported.autoConvergence(clusterVersion)) { + staticVm.setAutoConverge(entity.getAutoConverge()); + } + + if (FeatureSupported.migrationCompression(clusterVersion)) { + staticVm.setMigrateCompressed(entity.getMigrateCompressed()); + } + staticVm.setCustomProperties(entity.getCustomProperties()); staticVm.setCustomEmulatedMachine(entity.getCustomEmulatedMachine()); staticVm.setCustomCpuName(entity.getCustomCpuName()); -- To view, visit http://gerrit.ovirt.org/38008 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7430bca85b95ddf97eaa93abbcdf13885e017f9e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
