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 <[email protected]> --- 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, 119 insertions(+), 62 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/70/20970/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 989bc5a..8acd822 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 @@ -611,6 +611,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 cc0eea6..cd779c7 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 b43410a..2aab8f3 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 @@ -1533,6 +1533,10 @@ @DefaultValueAttribute("500") GlanceImageTotalListSize(543), + @TypeConverterAttribute(Boolean.class) + @DefaultValueAttribute("true") + PortMirroringSupported(544), + Invalid(65535); private int intValue; 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 9bee221..99cc9dd 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 @@ -810,6 +810,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 fdcd2fe..1a92473 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties @@ -450,6 +450,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..1709dfb 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,78 @@ 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)) { + 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()) { + reportUnsupportedVnicProfileFeatures(vm, nic, vnicProfile, unsupportedFeatures); + } } - private static void addQosForDevice(Map<String, Object> struct, + private static void reportUnsupportedVnicProfileFeatures(VM vm, + VmNic nic, + VnicProfile vnicProfile, + List<VNIC_PROFILE_PROPERTIES> unsupportedFeatures) { + 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 boolean addPortMirroringToVmInterface(Map<String, Object> struct, + VnicProfile vnicProfile, + Version version, + Network network) { + + if (vnicProfile != null && vnicProfile.isPortMirroring()) { + if (FeatureSupported.portMirroring(version)) { + struct.put(VdsProperties.PORT_MIRRORING, network == null ? Collections.<String> emptyList() + : Collections.singletonList(network.getName())); + } else { + return false; + } + } + + return true; + } + + private static boolean addQosForDevice(Map<String, Object> struct, VnicProfile vnicProfile, Version vdsGroupCompatibilityVersion) { - if (FeatureSupported.networkQoS(vdsGroupCompatibilityVersion) - && vnicProfile.getNetworkQosId() != null) { + + if (vnicProfile.getNetworkQosId() != null) { + if (!FeatureSupported.networkQoS(vdsGroupCompatibilityVersion)) { + return false; + } + NetworkQoS networkQoS = DbFacade.getInstance().getQosDao().get(vnicProfile.getNetworkQosId()); if (networkQoS != null) { Map<String, Object> specParams = (Map<String, Object>) struct.get(VdsProperties.SpecParams); @@ -594,6 +646,8 @@ networkQoS.getOutboundBurst()); } } + + return true; } private static void addQosData(Map<String, Object> specParams, @@ -620,24 +674,31 @@ 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); - } - - struct.put(VdsProperties.Custom, customProperties); + 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.isEmpty()) { + if (FeatureSupported.deviceCustomProperties(clusterVersion)) { + struct.put(VdsProperties.Custom, customProperties); + } else { + return false; + } + } + + return true; } public static void addNetworkFiltersToNic(Map<String, Object> struct, Version clusterVersion) { @@ -879,4 +940,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 89d953a..10691ba 100644 --- a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql +++ b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql @@ -383,6 +383,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/20970 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I98f621a64684deb8a19e0657c8268788b0fda275 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3 Gerrit-Owner: Moti Asayag <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
