Martin Peřina has posted comments on this change.
Change subject: core: Add proxy agent support verification
......................................................................
Patch Set 1:
(3 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceExecutor.java
Line 281: private VDS getFenceProxy(final boolean onlyUpHost, final boolean
filterSelf, final PMProxyOptions proxyOptions) {
Line 282: List<VDS> hosts = DbFacade.getInstance().getVdsDao().getAll();
Line 283: VDS proxyHost = LinqUtils.firstOrNull(hosts, new
Predicate<VDS>() {
Line 284: @Override
Line 285: public boolean eval(VDS vds) {
I wonder if moving repeated conditions wouldn't make code simpler:
public boolean eval(VDS vds) {
if (!isAgentSupported(vds)) {
return false;
}
if ((proxyOptions == PMProxyOptions.CLUSTER
&&
vds.getVdsGroupId().equals(_vds.getVdsGroupId())
|| (proxyOptions == PMProxyOptions.DC
&&
vds.getStoragePoolId().equals(_vds.getStoragePoolId()))) {
if (onlyUpHost) {
if (filterSelf) {
return !vds.getId().equals(_vds.getId())
&& vds.getStatus() == VDSStatus.Up;
} else {
return vds.getStatus() == VDSStatus.Up;
}
} else {
if (filterSelf) {
return !isHostNetworkUnreacable(vds)
&& !vds.getId().equals(_vds.getId());
} else {
return !isHostNetworkUnreacable(vds);
}
}
}
return false;
}
But please verify me, I didn't test it :-)
Line 286: if (proxyOptions == PMProxyOptions.CLUSTER) {
Line 287: if (onlyUpHost) {
Line 288: if (filterSelf) {
Line 289: return !vds.getId().equals(_vds.getId())
Line 347: private boolean isAgentSupported(VDS vds) {
Line 348: boolean ret = false;
Line 349: // Checks if the requested _vds PM agent is supported
by the candidate proxy (vds)
Line 350: VdsFenceOptions options = new
VdsFenceOptions(vds.getVdsGroupCompatibilityVersion().getValue());
Line 351: if (!StringUtils.isEmpty(_vds.getManagementIp())) {
Wouldn't it be better to use StringUtils.isNotEmpty?
Line 352: ret = options.isAgentSupported(_vds.getPmType());
Line 353: }
Line 354: // In a case that a secondary agent is defined,
require the proxy host to be
Line 355: // in a cluster that supports both Primary &
Secondary agents since in concurrent
Line 354: // In a case that a secondary agent is defined,
require the proxy host to be
Line 355: // in a cluster that supports both Primary &
Secondary agents since in concurrent
Line 356: // PM devices we need both, and in sequential PM
devices Primary might fail and then
Line 357: // Secondary PM agent should attempt to fence the Host
Line 358: if (!StringUtils.isEmpty(_vds.getPmSecondaryIp())) {
Same as above
Line 359: ret =
options.isAgentSupported(_vds.getPmSecondaryType());
Line 360: }
Line 361: return ret;
Line 362: }
--
To view, visit http://gerrit.ovirt.org/22423
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e6610e1e7be660c894c879288d9df9b0956eda5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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
