Mike Kolesnik has posted comments on this change.
Change subject: engine: Block unsupported profiles from vNICs
......................................................................
Patch Set 1:
(4 comments)
Please notice this fix is not enough, as when changing a VM cluster it will not
be executed.
I'm pretty sure the same can also happen on import and go back to snapshot of a
VM where an incompatible profile will be allowed since it exists on the DC
level but not supported for the cluster level that you import the VM into or go
back to snapshot in.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java
Line 88: return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_IS_NOT_SUPPORTED,
Line 89: clusterVersion());
Line 90:
Line 91: // Check if the profile contains custom properties is
supported in the current cluster's version
Line 92: if (!FeatureSupported.deviceCustomProperties(version)
The logic is backwards, it makes more sense to check first if there are even
custom properties and only then if the feature is even supported..
I know it has the same end result, but reading it would be easier.
Line 93: && !(vnicProfile.getCustomProperties() == null ||
vnicProfile.getCustomProperties().isEmpty())) {
Line 94: return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_CUSTOM_PROPERTIES_NOT_SUPPORTED,
Line 95: clusterVersion());
Line 96: }
Line 89: clusterVersion());
Line 90:
Line 91: // Check if the profile contains custom properties is
supported in the current cluster's version
Line 92: if (!FeatureSupported.deviceCustomProperties(version)
Line 93: && !(vnicProfile.getCustomProperties() == null ||
vnicProfile.getCustomProperties().isEmpty())) {
Would you mind extracting this to a method such as profileHasCustomProperties()?
This would make the code much easier to read.
Line 94: return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_CUSTOM_PROPERTIES_NOT_SUPPORTED,
Line 95: clusterVersion());
Line 96: }
Line 97:
Line 95: clusterVersion());
Line 96: }
Line 97:
Line 98: // Check that if the profile marked for port mirroring is
supported in the current cluster's version
Line 99: if (!FeatureSupported.portMirroring(version) &&
vnicProfile.isPortMirroring()) {
Same here about the reversed logic..
Line 100: return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_PORT_MIRRORING_NOT_SUPPORTED,
Line 101: clusterVersion());
Line 102: }
Line 103: }
....................................................
File packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 383: select fn_db_add_config_value('MTUOverrideSupported','true','3.1');
Line 384: select fn_db_add_config_value('MTUOverrideSupported','true','3.2');
Line 385: select fn_db_add_config_value('MTUOverrideSupported','true','3.3');
Line 386: select fn_db_add_config_value('PortMirroringSupported','false','3.0');
Line 387: select fn_db_add_config_value('PortMirroringSupported','false','3.1');
Why not supported in 3.1?
>From my understanding it was supported already in that version.
Line 388: --Handling Organization Name
Line 389: select fn_db_add_config_value('OrganizationName','oVirt','general');
Line 390: select fn_db_add_config_value('OriginType','OVIRT','general');
Line 391: select
fn_db_add_config_value('OvfVirtualSystemType','ENGINE','general');
--
To view, visit http://gerrit.ovirt.org/20671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I320d583437eac59fef62270aa7699afd8e6bbd6e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
