Moti Asayag has uploaded a new change for review. Change subject: engine: Report unsupported vnic profile features used by vnics ......................................................................
engine: Report unsupported vnic profile features used by vnics Unsupported feature of vnic profiles by vms running in cluster with incompatible version will trigger an event log stating which feature were used by the vm network interface. Change-Id: I98f621a64684deb8a19e0657c8268788b0fda275 Bug-Url: https://bugzilla.redhat.com/1024209 Signed-off-by: Moti Asayag <masa...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmNicValidatorTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java M backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java M packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql 9 files changed, 112 insertions(+), 60 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/09/20909/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java index e085e46..50fcc10 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java @@ -64,8 +64,6 @@ * <li>{@code VdcBllMessages.ACTION_TYPE_FAILED_VNIC_PROFILE_NOT_EXISTS} if the profile doesn't exist.</li> * <li>{@code VdcBllMessages.NETWORK_NOT_EXISTS_IN_CURRENT_CLUSTER} if the network is not in the current * cluster.</li> - * <li>{@code VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_IS_NOT_SUPPORTED} if the profile contains QoS - * and it is not supported in the current cluster's version.</li> * <li>{@code ValidationResult.VALID} otherwise.</li> * </ul> */ @@ -82,12 +80,8 @@ if (network == null || !isNetworkInCluster(network, clusterId)) { return new ValidationResult(VdcBllMessages.NETWORK_NOT_EXISTS_IN_CURRENT_CLUSTER); } - - // Check that if the profile contains QoS it is supported in the current cluster's version - if (!FeatureSupported.networkQoS(version) && vnicProfile.getNetworkQosId() != null) - return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_IS_NOT_SUPPORTED, - clusterVersion()); } + return ValidationResult.VALID; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmNicValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmNicValidatorTest.java index 7257b63..52af93d 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmNicValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmNicValidatorTest.java @@ -178,67 +178,36 @@ @Test public void vnicProfileExist() throws Exception { - vnicProfileValidationTest(isValid(), true, true, false, false); + vnicProfileValidationTest(isValid(), true, true); } @Test public void vnicProfileNotExist() throws Exception { vnicProfileValidationTest(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_VNIC_PROFILE_NOT_EXISTS), false, - false, - false, false); } @Test - public void networkQosSupported() throws Exception { - vnicProfileValidationTest(isValid(), true, true, true, true); - } - - @Test - public void networkQosNotSupported() throws Exception { - vnicProfileValidationTest(both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_IS_NOT_SUPPORTED)) - .and(replacements(hasItem(CLUSTER_VERSION_REPLACEMENT))), true, true, true, false); - } - - @Test - public void networkQosNullAndSupported() throws Exception { - vnicProfileValidationTest(isValid(), true, true, false, true); - } - - @Test - public void networkQosNullAndNotSupported() throws Exception { - vnicProfileValidationTest(isValid(), true, true, false, false); - } - - @Test public void networkInCluster() throws Exception { - vnicProfileValidationTest(isValid(), true, true, false, false); + vnicProfileValidationTest(isValid(), true, true); } @Test public void networkNotInCluster() throws Exception { vnicProfileValidationTest(failsWith(VdcBllMessages.NETWORK_NOT_EXISTS_IN_CURRENT_CLUSTER), true, - false, - false, false); } private void vnicProfileValidationTest(Matcher<ValidationResult> matcher, boolean profileExist, - boolean networkExist, - boolean qosNotNull, - boolean qosSupported) { + boolean networkExist) { when(dbFacade.getVnicProfileDao()).thenReturn(vnicProfileDao); when(vnicProfileDao.get(any(Guid.class))).thenReturn(profileExist ? vnicProfile : null); when(vnicProfile.getNetworkId()).thenReturn(DEFAULT_GUID); - doReturn(networkExist ? network : null).when(validator).getNetworkByVnicProfile(vnicProfile); doReturn(networkExist).when(validator).isNetworkInCluster(any(Network.class), any(Guid.class)); - - mockConfigRule.mockConfigValue(ConfigValues.NetworkQosSupported, version, qosSupported); - when(vnicProfile.getNetworkQosId()).thenReturn(qosNotNull ? DEFAULT_GUID : null); when(nic.getVnicProfileId()).thenReturn(VNIC_PROFILE_ID); assertThat(validator.profileValid(OTHER_GUID), matcher); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java index 7235d2b..7761d17 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java @@ -619,6 +619,7 @@ REMOVE_VNIC_PROFILE(1126), REMOVE_VNIC_PROFILE_FAILED(1127), NETWORK_WITHOUT_INTERFACES(1128), + VNIC_PROFILE_UNSUPPORTED_FEATURES(1129, AuditLogTimeInterval.DAY.getValue()), // Import/Export IMPORTEXPORT_STARTING_EXPORT_VM(1162), diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java index ac2e5ab..2a58e47 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java @@ -35,6 +35,15 @@ /** * @param version * Compatibility version to check for. + * @return <code>true</code> if port mirroring is supported for the version, <code>false</code> if it's not. + */ + public static boolean portMirroring(Version version) { + return supportedInConfig(ConfigValues.PortMirroringSupported, version); + } + + /** + * @param version + * Compatibility version to check for. * @return <code>true</code> if non-VM network is supported for the version, <code>false</code> if it's not. */ public static boolean nonVmNetwork(Version version) { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java index d47d6fc..76b87a0 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java @@ -1103,6 +1103,10 @@ @DefaultValueAttribute("true") MTUOverrideSupported, + @TypeConverterAttribute(Boolean.class) + @DefaultValueAttribute("true") + PortMirroringSupported, + @Reloadable @TypeConverterAttribute(Integer.class) @DefaultValueAttribute("1800") diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java index fc92dce..1badce0 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java @@ -818,6 +818,7 @@ severities.put(AuditLogType.REMOVE_VNIC_PROFILE, AuditLogSeverity.NORMAL); severities.put(AuditLogType.REMOVE_VNIC_PROFILE_FAILED, AuditLogSeverity.ERROR); severities.put(AuditLogType.NETWORK_WITHOUT_INTERFACES, AuditLogSeverity.WARNING); + severities.put(AuditLogType.VNIC_PROFILE_UNSUPPORTED_FEATURES, AuditLogSeverity.WARNING); } private static void initExtrnalEvents() { diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties index 955b756..2871666 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties @@ -451,6 +451,7 @@ VDS_SET_NONOPERATIONAL_IFACE_DOWN=Host ${VdsName} moved to Non-Operational state because interfaces '${Interfaces}' are down but are needed by networks '${Networks}' in the current cluster BRIDGED_NETWORK_OVER_MULTIPLE_INTERFACES=Bridged network ${NetworkName} is attached to multiple interfaces: ${Interfaces} on Host ${VdsName}. NETWORK_WITHOUT_INTERFACES=Network ${NetworkName} is not attached to any interface on host ${VdsName}. +VNIC_PROFILE_UNSUPPORTED_FEATURES=VM ${VmName} has network interface ${NicName} which is using profile ${VnicProfile} with unsupported feature(s) '${UnsupportedFeatures}' by VM cluster ${VdsGroupName} (version ${CompatibilityVersion}). PROVIDER_ADDED=Provider ${ProviderName} was added. (User: ${UserName}) PROVIDER_ADDITION_FAILED=Failed to add provider ${ProviderName}. (User: ${UserName}) PROVIDER_UPDATED=Provider ${ProviderName} was updated. (User: ${UserName}) diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java index 8520078..7365006 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java @@ -10,6 +10,7 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType; @@ -34,6 +35,8 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcStringUtils; @SuppressWarnings({"rawtypes", "unchecked"}) @@ -545,6 +548,7 @@ VnicProfile vnicProfile = null; Network network = null; String networkName = ""; + List<VNIC_PROFILE_PROPERTIES> unsupportedFeatures = new ArrayList<>(); if (nic.getVnicProfileId() != null) { vnicProfile = DbFacade.getInstance().getVnicProfileDao().get(nic.getVnicProfileId()); if (vnicProfile != null) { @@ -552,30 +556,72 @@ networkName = network.getName(); log.debugFormat("VNIC {0} is using profile {1} on network {2}", nic.getName(), vnicProfile, networkName); - addQosForDevice(struct, vnicProfile, vm.getVdsGroupCompatibilityVersion()); + if (!addQosForDevice(struct, vnicProfile, vm.getVdsGroupCompatibilityVersion())) { + unsupportedFeatures.add(VNIC_PROFILE_PROPERTIES.NETWORK_QOS); + } } } struct.put(VdsProperties.NETWORK, networkName); - if (vnicProfile != null && vnicProfile.isPortMirroring()) { - struct.put(VdsProperties.PORT_MIRRORING, network == null - ? Collections.<String> emptyList() : Collections.singletonList(networkName)); + if (!addPortMirroringToVmInterface(struct, vnicProfile, vm.getVdsGroupCompatibilityVersion(), + network == null ? Collections.<String> emptyList() : Collections.singletonList(networkName))) { + unsupportedFeatures.add(VNIC_PROFILE_PROPERTIES.PORT_MIRRORING); } - addCustomPropertiesForDevice(struct, + if (!addCustomPropertiesForDevice(struct, vm, vmDevice, vm.getVdsGroupCompatibilityVersion(), - getVnicCustomProperties(vnicProfile)); + getVnicCustomProperties(vnicProfile))) { + unsupportedFeatures.add(VNIC_PROFILE_PROPERTIES.CUSTOM_PROPERTIES); + } + + if (!unsupportedFeatures.isEmpty()) { + AuditLogableBase event = new AuditLogableBase(); + event.setVmId(vm.getId()); + event.setVdsGroupId(vm.getVdsGroupId()); + event.setCustomId(nic.getId().hashCode()); + event.setCompatibilityVersion(vm.getVdsGroupCompatibilityVersion().toString()); + event.addCustomValue("NicName", nic.getName()); + event.addCustomValue("VnicProfile", vnicProfile.getName()); + String[] unsupportedFeatureNames = new String[unsupportedFeatures.size()]; + for (int i = 0; i < unsupportedFeatures.size(); i++) { + unsupportedFeatureNames[i] = unsupportedFeatures.get(i).getFeatureName(); + } + + event.addCustomValue("UnsupportedFeatures", StringUtils.join(unsupportedFeatureNames, ',')); + AuditLogDirector.log(event, AuditLogType.VNIC_PROFILE_UNSUPPORTED_FEATURES); + } } - private static void addQosForDevice(Map<String, Object> struct, + private static boolean addPortMirroringToVmInterface(Map<String, Object> struct, + VnicProfile vnicProfile, + Version version, + List<String> networkNames) { + + if (vnicProfile.isPortMirroring() && !FeatureSupported.portMirroring(version)) { + return false; + } + + if (vnicProfile != null && vnicProfile.isPortMirroring()) { + struct.put(VdsProperties.PORT_MIRRORING, networkNames); + } + + return true; + } + + private static boolean addQosForDevice(Map<String, Object> struct, VnicProfile vnicProfile, Version vdsGroupCompatibilityVersion) { - if (FeatureSupported.networkQoS(vdsGroupCompatibilityVersion) - && vnicProfile.getNetworkQosId() != null) { + + boolean networkQosSupported = FeatureSupported.networkQoS(vdsGroupCompatibilityVersion); + if (vnicProfile.getNetworkQosId() != null && !networkQosSupported) { + return false; + } + + if (networkQosSupported && vnicProfile.getNetworkQosId() != null) { NetworkQoS networkQoS = DbFacade.getInstance().getQosDao().get(vnicProfile.getNetworkQosId()); if (networkQoS != null) { Map<String, Object> specParams = (Map<String, Object>) struct.get(VdsProperties.SpecParams); @@ -594,6 +640,8 @@ networkQoS.getOutboundBurst()); } } + + return true; } private static void addQosData(Map<String, Object> specParams, @@ -620,24 +668,32 @@ return customProperties == null ? new HashMap<String, String>() : customProperties; } - public static void addCustomPropertiesForDevice(Map<String, Object> struct, + public static boolean addCustomPropertiesForDevice(Map<String, Object> struct, VM vm, VmDevice vmDevice, Version clusterVersion, Map<String, String> customProperties) { - if (FeatureSupported.deviceCustomProperties(clusterVersion)) { - if (customProperties == null) { - customProperties = new HashMap<>(); - } - customProperties.putAll(vmDevice.getCustomProperties()); - Map<String, String> runtimeCustomProperties = vm.getRuntimeDeviceCustomProperties().get(vmDevice); - if (runtimeCustomProperties != null) { - customProperties.putAll(runtimeCustomProperties); - } + if (customProperties == null) { + customProperties = new HashMap<>(); + } + customProperties.putAll(vmDevice.getCustomProperties()); + Map<String, String> runtimeCustomProperties = vm.getRuntimeDeviceCustomProperties().get(vmDevice); + if (runtimeCustomProperties != null) { + customProperties.putAll(runtimeCustomProperties); + } + + boolean deviceCustomPropertiesSupported = FeatureSupported.deviceCustomProperties(clusterVersion); + if (!customProperties.isEmpty() && !deviceCustomPropertiesSupported) { + return false; + } + + if (deviceCustomPropertiesSupported) { struct.put(VdsProperties.Custom, customProperties); } + + return true; } public static void addNetworkFiltersToNic(Map<String, Object> struct, Version clusterVersion) { @@ -879,4 +935,19 @@ } } + private static enum VNIC_PROFILE_PROPERTIES { + PORT_MIRRORING("Port Mirroring"), + CUSTOM_PROPERTIES("Custom Properties"), + NETWORK_QOS("Network QoS"); + + private String featureName; + + private VNIC_PROFILE_PROPERTIES(String featureName) { + this.featureName = featureName; + } + + public String getFeatureName() { + return featureName; + } + }; } diff --git a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql index 33f7c99..0f8ccf4 100644 --- a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql +++ b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql @@ -390,6 +390,8 @@ select fn_db_add_config_value('MTUOverrideSupported','true','3.1'); select fn_db_add_config_value('MTUOverrideSupported','true','3.2'); select fn_db_add_config_value('MTUOverrideSupported','true','3.3'); +select fn_db_add_config_value('PortMirroringSupported','false','3.0'); +select fn_db_add_config_value('PortMirroringSupported','false','3.1'); --Handling Organization Name select fn_db_add_config_value('OrganizationName','oVirt','general'); select fn_db_add_config_value('OriginType','OVIRT','general'); -- To view, visit http://gerrit.ovirt.org/20909 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I98f621a64684deb8a19e0657c8268788b0fda275 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <masa...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
