Tomas Jelinek has uploaded a new change for review. Change subject: core: do not copy RNG device from instance type if not possible ......................................................................
core: do not copy RNG device from instance type if not possible The rng device was copied from the instance type to newly created VM without checking if the virtio rng is actually supported on this cluster. Fixed by extracting the logic to a standalone validator and using this validator in commands and also in VmDeviceUtils. Change-Id: Ic016df28bd0484735c74308750fe03ab9036f59d Bug-Url: https://bugzilla.redhat.com/1177234 Signed-off-by: Tomas Jelinek <tjeli...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddRngDeviceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateRngDeviceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VirtIoRngValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateRngDeviceTest.java 6 files changed, 52 insertions(+), 20 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/97/36797/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java index 60b4d03..1d506ac 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java @@ -5,19 +5,16 @@ import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.utils.PermissionSubject; -import org.ovirt.engine.core.common.FeatureSupported; +import org.ovirt.engine.core.bll.validator.VirtIoRngValidator; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.RngDeviceParameters; -import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VmBase; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; import org.ovirt.engine.core.common.businessentities.VmEntityType; -import org.ovirt.engine.core.common.businessentities.VmRngDevice; import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; -import org.ovirt.engine.core.compat.Version; import org.ovirt.engine.core.dao.VmDeviceDAO; /** @@ -85,15 +82,9 @@ return true; } - protected boolean isRngSupportedByCluster() { - VDSGroup cluster = getVdsGroup(); - VmRngDevice.Source source = getParameters().getRngDevice().getSource(); - return cluster != null && isFeatureSupported(cluster.getcompatibility_version()) - && cluster.getRequiredRngSources().contains(source); - } - - boolean isFeatureSupported(Version clusterVersion) { - return FeatureSupported.virtIoRngSupported(clusterVersion); + // it is here only to make it possible to mock it + protected VirtIoRngValidator getVirtioRngValidator() { + return new VirtIoRngValidator(); } protected List<VmDevice> getRngDevices() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddRngDeviceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddRngDeviceCommand.java index efa99a5..4348ad7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddRngDeviceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddRngDeviceCommand.java @@ -23,8 +23,8 @@ return false; } - if (!isRngSupportedByCluster() && getTemplateType() != VmEntityType.INSTANCE_TYPE) { - return failCanDoAction(VdcBllMessages.ACTION_NOT_SUPPORTED_FOR_CLUSTER_POOL_LEVEL); + if (getTemplateType() != VmEntityType.INSTANCE_TYPE) { + validate(getVirtioRngValidator().canAddRngDevice(getVdsGroup(), getParameters().getRngDevice())); } if (!getRngDevices().isEmpty()) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateRngDeviceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateRngDeviceCommand.java index ba4142c..fbbcf42 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateRngDeviceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateRngDeviceCommand.java @@ -24,8 +24,8 @@ return false; } - if (!isRngSupportedByCluster() && getTemplateType() != VmEntityType.INSTANCE_TYPE) { - return failCanDoAction(VdcBllMessages.ACTION_NOT_SUPPORTED_FOR_CLUSTER_POOL_LEVEL); + if (getTemplateType() != VmEntityType.INSTANCE_TYPE) { + validate(getVirtioRngValidator().canAddRngDevice(getVdsGroup(), getParameters().getRngDevice())); } if (getRngDevices().isEmpty()) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java index d82ce30..6f68a9c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java @@ -4,6 +4,7 @@ import org.ovirt.engine.core.bll.VmHandler; import org.ovirt.engine.core.bll.network.VmInterfaceManager; import org.ovirt.engine.core.bll.smartcard.SmartcardSpecParams; +import org.ovirt.engine.core.bll.validator.VirtIoRngValidator; import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.businessentities.BaseDisk; import org.ovirt.engine.core.common.businessentities.Disk; @@ -18,6 +19,7 @@ import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; import org.ovirt.engine.core.common.businessentities.VmDeviceId; +import org.ovirt.engine.core.common.businessentities.VmRngDevice; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.VmType; import org.ovirt.engine.core.common.businessentities.network.VmNic; @@ -292,6 +294,8 @@ boolean hasAlreadyConsoleDevice = false; boolean hasVirtioScsiController = false; + VDSGroup cluster = vmBase.getVdsGroupId() != null ? DbFacade.getInstance().getVdsGroupDao().get(vmBase.getVdsGroupId()) : null; + for (VmDevice device : devicesDataToUse) { if (device.getSnapshotId() != null && !copySnapshotDevices) { continue; @@ -376,6 +380,9 @@ if (hasVmRngDevice(dstId)) { continue; // don't copy rng device if we already have it } + if (!new VirtIoRngValidator().canAddRngDevice(cluster, new VmRngDevice(device)).isValid()) { + continue; + } specParams.putAll(device.getSpecParams()); break; @@ -421,7 +428,6 @@ if (isVm) { addSoundCard(vm.getStaticData(), vm.getVdsGroupCompatibilityVersion()); } else { - VDSGroup cluster = vmBase.getVdsGroupId() != null ? DbFacade.getInstance().getVdsGroupDao().get(vmBase.getVdsGroupId()) : null; addSoundCard(vmBase, cluster != null ? cluster.getcompatibility_version() : null); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VirtIoRngValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VirtIoRngValidator.java new file mode 100644 index 0000000..9c63b08 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VirtIoRngValidator.java @@ -0,0 +1,29 @@ +package org.ovirt.engine.core.bll.validator; + + +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.FeatureSupported; +import org.ovirt.engine.core.common.businessentities.VDSGroup; +import org.ovirt.engine.core.common.businessentities.VmRngDevice; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Version; + +public class VirtIoRngValidator { + + public ValidationResult canAddRngDevice(VDSGroup cluster, VmRngDevice rngDevice) { + VmRngDevice.Source source = rngDevice.getSource(); + boolean supported = cluster != null && + isFeatureSupported(cluster.getcompatibility_version()) && + cluster.getRequiredRngSources().contains(source); + + if (supported) { + return ValidationResult.VALID; + } + + return new ValidationResult(VdcBllMessages.ACTION_NOT_SUPPORTED_FOR_CLUSTER_POOL_LEVEL); + } + + protected boolean isFeatureSupported(Version clusterVersion) { + return FeatureSupported.virtIoRngSupported(clusterVersion); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateRngDeviceTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateRngDeviceTest.java index 5f80378..b5c3bd9 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateRngDeviceTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateRngDeviceTest.java @@ -4,6 +4,7 @@ import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; +import org.ovirt.engine.core.bll.validator.VirtIoRngValidator; import org.ovirt.engine.core.common.action.RngDeviceParameters; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VmDevice; @@ -65,8 +66,13 @@ } @Override - boolean isFeatureSupported(Version clusterVersion) { - return mockCompatibilityVersion.compareTo(Version.v3_5) >= 0; + protected VirtIoRngValidator getVirtioRngValidator() { + return new VirtIoRngValidator() { + @Override + protected boolean isFeatureSupported(Version clusterVersion) { + return mockCompatibilityVersion.compareTo(Version.v3_5) >= 0; + } + }; } }; -- To view, visit http://gerrit.ovirt.org/36797 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic016df28bd0484735c74308750fe03ab9036f59d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
