Eli Mesika has posted comments on this change. Change subject: core: FenceProxyLocator* code clean up ......................................................................
Patch Set 1: (3 comments) https://gerrit.ovirt.org/#/c/39763/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/FenceProxyLocator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/FenceProxyLocator.java: Line 71: break; Line 72: } Line 73: } Line 74: if (proxyHost == null) { Line 75: log.error("Can not run fence action on host '{}', no suitable proxy host was found.", we have a mixed terminology all around of "Power Management" and fence , IIRC , for messages that are targeted to users the term "Power Management" is considered more friendly. I have no objection to change this if it is OK from product management side but we should make it consistent Line 76: fencedHost.getName()); Line 77: return null; Line 78: } Line 79: return proxyHost; Line 145: return proxyCandidate.getSupportedClusterVersionsSet().contains(minimalSupportedVersion); Line 146: } Line 147: Line 148: private boolean isHostNetworkUnreachable(VDS proxyCandidate) { Line 149: return proxyCandidate.getStatus() == VDSStatus.Down why this code was changed? for example : previously vdsDynamic.getStatus() now : proxyCandidate.getStatus() where we get the data from the dynamic information? please explain ... Line 150: || proxyCandidate.getStatus() == VDSStatus.Reboot Line 151: || proxyCandidate.getStatus() == VDSStatus.Kdumping Line 152: || proxyCandidate.getStatus() == VDSStatus.NonResponsive Line 153: || proxyCandidate.getStatus() == VDSStatus.PendingApproval https://gerrit.ovirt.org/#/c/39763/1/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/pm/FenceProxyLocatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/pm/FenceProxyLocatorTest.java: Line 13: import org.junit.Test; Line 14: import org.junit.runner.RunWith; Line 15: import org.mockito.Mock; Line 16: import org.mockito.runners.MockitoJUnitRunner; Line 17: import org.ovirt.engine.core.bll.DbDependentTestBase; why do we need this, I see no usage of it in this code ... Line 18: import org.ovirt.engine.core.common.businessentities.FencingPolicy; Line 19: import org.ovirt.engine.core.common.businessentities.NonOperationalReason; Line 20: import org.ovirt.engine.core.common.businessentities.VDS; Line 21: import org.ovirt.engine.core.common.businessentities.VDSStatus; -- To view, visit https://gerrit.ovirt.org/39763 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9424a1afefde58b6e687eb4a0325a59ce9573950 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
