Mike Kolesnik has posted comments on this change. Change subject: core: Add mom policy update command ......................................................................
Patch Set 10: (3 comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateMomPolicyCommand.java Line 43: && validate(validateMinimumVersionSupport()); Line 44: } Line 45: Line 46: private ValidationResult validateMinimumVersionSupport() { Line 47: return FeatureSupported.momPolicyOnHost(Version.v3_3) What you should be sending is the cluster compatibility version, not a constant version.. You could send the host's version but that would be confusing to the users. The norm in oVirt is to restrict version specific features by either cluster or DC - since this is clearly related to cluster (and not related to storage), the restriction should be by cluster level. Line 48: ? ValidationResult.VALID Line 49: : new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VDS_VERSION); Line 50: } Line 51: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsValidator.java Line 83: } Line 84: Line 85: public ValidationResult validateStatus(VDSStatus vdsStatus, VdcBllMessages hostStatus) { Line 86: ValidationResult existsValidation = exists(); Line 87: if (!existsValidation.isValid()) { I haven't seen a response from you regarding the comment on patch set 5. I still think this check shouldn't be here.. My reasoning is in patch set 5 Line 88: return existsValidation; Line 89: } Line 90: Line 91: return vdsStatus == vds.getStatus() .................................................... File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties Line 527: ACTION_TYPE_FAILED_VDS_INTERMITENT_CONNECTIVITY=Due to intermittent connectivity to this Host, fence operations are not allowed at this time. The system is trying to reconnect, please try again in 30 seconds. Line 528: ACTION_TYPE_FAILED_PM_ENABLED_WITHOUT_AGENT=Cannot ${action} ${type}. Power Management is enabled for Host but no Agent type selected. Line 529: ACTION_TYPE_FAILED_PM_ENABLED_WITHOUT_AGENT_CREDENTIALS=Cannot ${action} ${type}. Power Management is enabled for Host but Agent credentials are missing. Line 530: ACTION_TYPE_FAILED_AGENT_NOT_SUPPORTED=Cannot ${action} ${type}. Selected Power Management Agent is not supported. Line 531: ACTION_TYPE_FAILED_VDS_VERSION=Cannot ${action} ${type}. Feature is not supported in host version. This message is not very intuitive to user - what feature? what version? how can he fix this? Besides it should refer to the cluster's version, and be named appropriately: ACTION_TYPE_FAILED_CLUSTER_VERSION_NOT_SUPPORTED A proposed alternative (if you aim at a generic message): Cannot ${action} ${type}. The cluster's compatibility version doesn't support this action. Please update the cluster's compatibility version and try again. However, usually its best to have a specific message that is more understandable and intuitive to the users, such as "Updating the host's policy is not supported for the cluster's compatibility version, etc.." Line 532: NETWORK_BOND_NOT_ATTACCH_TO_NETWORK=Bond is not attached to Network. Line 533: NETWORK_INTERFACE_NOT_ATTACCH_TO_NETWORK=Network Interface is not attached to Logical Network. Line 534: NETWORK_INTERFACE_IN_USE_BY_VLAN=Bonding cannot be applied on an Interface where VLAN is defined.\n\ Line 535: -Please remove VLAN from the interface. -- To view, visit http://gerrit.ovirt.org/17379 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64350c17f182c1dd0ba1d06130e5d56019b32fc9 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: ofri masad <oma...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-Reviewer: ofri masad <oma...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches