Liron Ar has posted comments on this change.
Change subject: core: Move VDS to Maintenance only if StopSPM is successful
......................................................................
Patch Set 4:
(8 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java
Line 59: List<VDS> spms = new ArrayList<>();
Line 60:
Line 61: // copy VDSs to new list since we will remove those on which
Stop SPM command fails
Line 62: List<VDS> vdss = new ArrayList<>(vdssToMaintenance.size());
Line 63: vdss.addAll(vdssToMaintenance.values());
I'm in favor of just using the iterator and remove during the iteration..that's
unneeded.
Line 64:
Line 65: for (VDS vds : vdss) {
Line 66: // SPMs will move to Prepare For Maintenance later after
standard hosts
Line 67: if (vds.getSpmStatus() != VdsSpmStatus.SPM) {
Line 129: @Override
Line 130: protected void executeCommand() {
Line 131: MoveVdssToGoingToMaintenanceMode();
Line 132: MigrateAllVdss();
Line 133: // set network to operational / non-operational
what about this part? if we didn't do anything on that cluster (because there
are tasks) - seems like it shouldn't be done.
Line 134: for (Guid id : _vdsGroupIds) {
Line 135: List<Network> networks =
DbFacade.getInstance().getNetworkDao().getAllForCluster(id);
Line 136: for (Network net : networks) {
Line 137: NetworkClusterHelper.setStatus(id, net);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/SetVdsStatusVDSCommandParameters.java
Line 16: public SetVdsStatusVDSCommandParameters(Guid vdsId, VDSStatus
status) {
Line 17: super(vdsId);
Line 18: _status = status;
Line 19: nonOperationalReason = NonOperationalReason.NONE;
Line 20: logStopSpmFailure = false;
that's unneeded..it will be false by default.
Line 21: }
Line 22:
Line 23: public SetVdsStatusVDSCommandParameters(Guid vdsId, VDSStatus
status, NonOperationalReason nonOperationalReason) {
Line 24: this(vdsId, status);
Line 22:
Line 23: public SetVdsStatusVDSCommandParameters(Guid vdsId, VDSStatus
status, NonOperationalReason nonOperationalReason) {
Line 24: this(vdsId, status);
Line 25: this.nonOperationalReason = nonOperationalReason;
Line 26: logStopSpmFailure = false;
same
Line 27: }
Line 28:
Line 29: public VDSStatus getStatus() {
Line 30: return _status;
Line 42: public void setNonOperationalReason(NonOperationalReason
nonOperationalReason) {
Line 43: this.nonOperationalReason = (nonOperationalReason == null ?
NonOperationalReason.NONE : nonOperationalReason);
Line 44: }
Line 45:
Line 46: public boolean shouldLogStopSpmFailure() {
please change to "is" or "get" prefix to meet getter/setter naming convention
Line 47: return logStopSpmFailure;
Line 48: }
Line 49:
Line 50: public void setLogStopSpmFailure(boolean logStopSpmFailure) {
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 703: DISK_ALIGNMENT_SCAN_START=Starting alignment scan of disk
'${DiskAlias}'.
Line 704: DISK_ALIGNMENT_SCAN_FAILURE=Alignment scan of disk '${DiskAlias}'
failed.
Line 705: DISK_ALIGNMENT_SCAN_SUCCESS=Alignment scan of disk '${DiskAlias}' is
complete.
Line 706: 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 707: FAILED_TO_STOP_SPM_ON_HOST=Failed to stop SPM running on host
${VdsName}
the audit log here is wrong IMO, we didn't get to try to stop (because there
were tasks)
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/SetVdsStatusVDSCommand.java
Line 40: // log to audit log if SpmStop command failed and
SpmStop result was not marked as ignored
Line 41: if (!getVDSReturnValue().getSucceeded() &&
getParameters().shouldLogStopSpmFailure()) {
Line 42: AuditLogableBase base = new AuditLogableBase();
Line 43: base.setVds(vds);
Line 44: AuditLogDirector.log(base,
AuditLogType.FAILED_TO_STOP_SPM_ON_HOST);
IMO we should distinguish between situation that spm stop failed and situation
that it wasn't performed
Line 45: }
Line 46: }
Line 47:
Line 48: // set status to Prepare for Maintenance for SPM only if
ResetIrs command was successful
Line 46: }
Line 47:
Line 48: // set status to Prepare for Maintenance for SPM only if
ResetIrs command was successful
Line 49: if (getVDSReturnValue().getSucceeded()) {
Line 50: updateVdsFromParameters(parameters, vds);
this currently updates the non operational reason, now if we didn't perform
spmStop that won't occur..which might cause behavior change in the different
flows - seems to me like all the updates should occur besides the status update.
Line 51: TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
Line 52:
Line 53: @Override
Line 54: public Void runInTransaction() {
--
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: 4
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
