Ramesh N has posted comments on this change. Change subject: <WIP>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. Done 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? Will it not execute even though cluster version or vdsm version is not supported?. We may end of trying to put the host into non operational for two times. 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? will do that 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? Yes. Once vdsm patch is available, i will change this to check for actual features. 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 fea In general, w will have entries for each feature in the UI. But here in this case, we don't want to put each gluster37 features in the UI with check box. We have following three features and it may be confusing if we put all of them separately in the UI. Features supported: Snapshot Geo Replication Brick provisioning 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: Moti Asayag <[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
