Liron Ar has posted comments on this change. Change subject: core: Move VDS to Maintenance only if StopSPM is successful ......................................................................
Patch Set 2: (2 comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java Line 59: } Line 60: } Line 61: Line 62: private void MoveVdssToGoingToMaintenanceMode() { Line 63: List<VDS> nonSpms = new LinkedList<>(); Martin, perhaps you missed my point. The previous code already changes to non spms status first and then pass the spms to the status change, the logic that you introduced does the same, therefore unless i'm missing something this refactor is unneeded. Line 64: List<VDS> spms = new LinkedList<>(); Line 65: for(VDS vds : vdssToMaintenance.values()) { Line 66: if (vds.getSpmStatus() == VdsSpmStatus.SPM) { Line 67: spms.add(0, vds); Line 94: if (vds.getStatus() != VDSStatus.PreparingForMaintenance && vds.getStatus() != VDSStatus.NonResponsive Line 95: && vds.getStatus() != VDSStatus.Down) { Line 96: VDSReturnValue result = runVdsCommand(VDSCommandType.SetVdsStatus, Line 97: new SetVdsStatusVDSCommandParameters(vds.getId(), VDSStatus.PreparingForMaintenance)); Line 98: if (!result.getSucceeded()) { "Do you have any idea how to test this case without blocking the host with iptables?" - I suggest to test the bug scenario, have a spm with tasks and see that it's status doesn't change..as it seems to me, it will change. Logging - can't you just move this logging code to there? you don't log "this" (the object) but creating a new one, so u should be fine :) Line 99: // remove VDS from base list, because of an error during SetVdsStatus Line 100: vdssToMaintenance.remove(vds.getId()); Line 101: Line 102: AuditLogableBase base = new AuditLogableBase(); -- To view, visit http://gerrit.ovirt.org/21231 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5c58f9a9629d2e7a496f02c4dececeb842d44543 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@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