Liron Ar has posted comments on this change.

Change subject: engine: Speed up SPM re-selection process.
......................................................................


Patch Set 6:

(3 comments)

....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SpmStopVDSCommand.java
Line 30:                             
.runVdsCommand(VDSCommandType.HSMGetAllTasksStatuses,
Line 31:                                     new 
VdsIdVDSCommandParametersBase(getVds().getId()));
Line 32: 
Line 33:                     if (isNotSPM(vdsReturnValue)) {
Line 34:                         getVDSReturnValue().setSucceeded(true);
this could be omitted, seems to me like it's set to true in 
VDSCommandBase.executeCommand before this method is called.
Line 35:                         return;
Line 36:                     }  else {
Line 37:                         
getVDSReturnValue().setSucceeded(vdsReturnValue.getSucceeded());
Line 38:                         
getVDSReturnValue().setVdsError(vdsReturnValue.getVdsError());


Line 32: 
Line 33:                     if (isNotSPM(vdsReturnValue)) {
Line 34:                         getVDSReturnValue().setSucceeded(true);
Line 35:                         return;
Line 36:                     }  else {
+1
Line 37:                         
getVDSReturnValue().setSucceeded(vdsReturnValue.getSucceeded());
Line 38:                         
getVDSReturnValue().setVdsError(vdsReturnValue.getVdsError());
Line 39: 
Line 40:                         if (vdsReturnValue.getReturnValue() != null) {


Line 89:         return vdsStatus != VDSStatus.NonResponsive && 
getVds().getStatus() != VDSStatus.Connecting;
Line 90:     }
Line 91: 
Line 92:     private boolean isNotSPM(VDSReturnValue returnValue) {
Line 93:         return returnValue != null && returnValue.getVdsError() != 
null &&
the two last checks can and should be squashed into one check - 
returnValue.getVdsError() != VdcBllErrors.SpmStatusError, I actually prefer
!VdcBllErrors.SpmStatusError.equals(returnValue.getVdsError())

the first check can be removed, but that's up to you..if it's being changed 
into one check, the whole method call can be replaced with the check :)
Line 94:                 returnValue.getVdsError().getCode() == 
VdcBllErrors.SpmStatusError;
Line 95:     }
Line 96: 
Line 97:     @Override


-- 
To view, visit http://gerrit.ovirt.org/18013
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c191ad8d4ec0fb4f5f8ff68b84fa5189ecc7114
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@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

Reply via email to