Oved Ourfali has posted comments on this change. Change subject: engine: add feature comptability check for VDS ......................................................................
Patch Set 5: (5 comments) https://gerrit.ovirt.org/#/c/39756/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsGroupCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsGroupCommand.java: Line 78: vdsGroup add some spacing here. https://gerrit.ovirt.org/#/c/39756/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HandleVdsVersionCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HandleVdsVersionCommand.java: Line 80: reportNonOperationReason(NonOperationalReason.CLUSTER_VERSION_INCOMPATIBLE_WITH_CLUSTER, Line 81: cluster.getCompatibilityVersion().toString(), Line 82: vds.getSupportedClusterLevels().toString()); Line 83: } else { Line 84: checkClusterFeaturesSupported(cluster, vds); you can just put it above the "setSucceeded" statement, right? Line 85: } Line 86: setSucceeded(true); Line 87: } Line 88: https://gerrit.ovirt.org/#/c/39756/5/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 726: can it be more than one? If so, worth changing the message. https://gerrit.ovirt.org/#/c/39756/5/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java: Line 712: // Its an work around until the supportedFeatures field is supported by VDSM Line 713: if (RpmVersionUtils.compareRpmParts(vds.getGlusterVersion().getMajor() + "." Line 714: + vds.getGlusterVersion().getMinor(), "3.7") >= 0) { Line 715: vds.setSupportedFeatures("gluster37_features"); Line 716: } so this won't be part of the patch before it is merged, right? Line 717: } Line 718: Line 719: private static void setRngSupportedSourcesToVds(VDS vds, Map<String, Object> xmlRpcStruct) { Line 720: vds.getSupportedRngSources().clear(); https://gerrit.ovirt.org/#/c/39756/5/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java: Line 368: public static final String DST_QEMU = "dstqemu"; Line 369: public static final String MIGRATION_DOWNTIME = "downtime"; Line 370: public static final String AUTO_CONVERGE = "autoConverge"; Line 371: public static final String MIGRATE_COMPRESSED = "compressed"; Line 372: public static final String GLUSTER37 = "gluster37_features"; don't you want it to be feature-wise, rather than putting all gluster37 features in one value? I'd imagine showing the user: 1. feature1, feature2, feature3 in the cluster level. 2. He can select what's important for him. 3. His selection will be validated. That way, it is a generic approach for feature compatibility. Line 373: // storage domains Line 374: public static final String code = "code"; Line 375: public static final String lastCheck = "lastCheck"; Line 376: public static final String delay = "delay"; -- To view, visit https://gerrit.ovirt.org/39756 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icba02b189a169bc676e0c5f47f7aaf394f0b49a6 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ramesh N <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
