Liron Ar has posted comments on this change.

Change subject: core: Move VDS to Maintenance only if StopSPM is successful
......................................................................


Patch Set 4:

(2 comments)

....................................................
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}
about 1 - in case that we didn't execute anything (there are tasks), what vds 
error will we have? seems to me that we won't have any.


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/SetVdsStatusVDSCommand.java
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);
You didn't understand my meaning, i've meant to have the if condition only for 
the status change - 
if (getVDSReturnValue().getSucceeded()) {
_vdsManager.setStatus(parameters.getStatus(), vds);
}

*other db updates*


if you feel comfortable to remove the other updates (for example, what if the 
non operational reason which is updated in updateVdsFromParameters(parameters, 
vds); has changed? where will it be updated now?)..please do, i'd check 
carefully.
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

Reply via email to