Liron Ar has posted comments on this change.
Change subject: core: Move VDS to Maintenance only if StopSPM is successful
......................................................................
Patch Set 6:
(3 comments)
The fix is incomplete, take a look on SpmStopVdsCommand
if there are tasks the command will still return as true which will lead you to
the same situation.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java
Line 131:
Line 132: // find clusters for hosts that should move to maintenance
Line 133: Set<Guid> clusters = new HashSet<>();
Line 134: for (VDS vds : vdssToMaintenance.values()) {
Line 135: clusters.add(vds.getVdsGroupId());
I think that a better way here would be to iterate once over the vdss and add
the clusterid to a hashset, then check if it doesn't contain the cluster id of
the current vds before executing the logic of the clusters loop.
Line 136: }
Line 137: // set network to operational / non-operational
Line 138: for (Guid id : clusters) {
Line 139: List<Network> networks =
DbFacade.getInstance().getNetworkDao().getAllForCluster(id);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/SetVdsStatusVDSCommandParameters.java
Line 22:
Line 23: public SetVdsStatusVDSCommandParameters(Guid vdsId, VDSStatus
status, NonOperationalReason nonOperationalReason) {
Line 24: this(vdsId, status);
Line 25: this.nonOperationalReason = nonOperationalReason;
Line 26: stopSpmFailureLogged = false;
can you reply on my last comment on that?
regardless of this being removed as i prefer it as the default of boolean is
false, it's bad to have it some of the ctors and in some not, you can just
define it with this value explictly written but that's redundant
Line 27: }
Line 28:
Line 29: public VDSStatus getStatus() {
Line 30: return _status;
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 705: DISK_ALIGNMENT_SCAN_START=Starting alignment scan of disk
'${DiskAlias}'.
Line 706: DISK_ALIGNMENT_SCAN_FAILURE=Alignment scan of disk '${DiskAlias}'
failed.
Line 707: DISK_ALIGNMENT_SCAN_SUCCESS=Alignment scan of disk '${DiskAlias}' is
complete.
Line 708: VM_MEMORY_NOT_IN_RECOMMENDED_RANGE=VM ${VmName} was configured with
${VmMemInMb}mb of memory while the recommended value range is ${VmMinMemInMb}mb
- ${VmMaxMemInMb}mb
Line 709: VDS_STATUS_CHANGE_FAILED_DUE_TO_STOP_SPM_FAILURE=Failed to change
status of host '${VdsName}' due to Stop SPM failure.
the user doesn't know what is "Stop Spm" command :) should be "due to failure
to stop the spm" or something alike
--
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: 6
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
