Itamar Heim has posted comments on this change.
Change subject: Verify vdsm cluster level support before send getHardwareInfo
request
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
Line 515: if (vdsBrokerCommand.getVDSReturnValue().getSucceeded()) {
Line 516: // Verify version capabilities
Line 517: String clusterCompatibility =
vds.getvds_group_compatibility_version().getValue();
Line 518: List<String> hostVersions =
Arrays.asList(vds.getsupported_cluster_levels().split("[,]", -1));
Line 519: if (hostVersions.contains(clusterCompatibility) &&
I'd put a comment on such a line of code to explain why you are doing this
check here, since there are other places in the code checking for host matching
the cluster version. (personally, i'd also first do the check on the
config/cluster to check HardwareInfoEnable, which is the logical check, then
the filtering check on host matching the cluster version / supporting this
feature (with a comment).
I agree these 3 lines of code are probably repeating themselves and can later
be reviewed for refactoring across the project.
Line 520: Config.<Boolean>
GetValue(ConfigValues.HardwareInfoEnabled, clusterCompatibility)) {
Line 521: VDSReturnValue ret =
ResourceManager.getInstance().runVdsCommand(VDSCommandType.GetHardwareInfo,
Line 522: new VdsIdAndVdsVDSCommandParametersBase(vds));
Line 523: if (!ret.getSucceeded()) {
--
To view, visit http://gerrit.ovirt.org/11027
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I240786444bdf393f6df4e53daf94d0ee197ecb73
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
