Gilad Chaplik has posted comments on this change.

Change subject: webadmin, backend: control of hosted engine maintenance mode
......................................................................


Patch Set 3:

(6 comments)

not sure there are any master patches

http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java:

Line 202:                         clustersAsSet.add(vds.getVdsGroupId());
Line 203: 
Line 204:                         List<String> 
nonMigratableVmDescriptionsToFrontEnd = new ArrayList<String>();
Line 205:                         for (VM vm : vms) {
Line 206:                             if (vm.isHostedEngine()) {
not so familiar with the arch, but what if there is only one ha agent 
configured in the cluster?
Line 207:                                 // The Hosted Engine vm is migrated 
by the HA agent
Line 208:                                 continue;
Line 209:                             }
Line 210:                             if (vm.getMigrationSupport() != 
MigrationSupport.MIGRATABLE) {


http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetHaMaintenanceCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetHaMaintenanceCommand.java:

Line 15: import java.util.Collections;
Line 16: import java.util.List;
Line 17: 
Line 18: @NonTransactiveCommandAttribute
Line 19: public class SetHaMaintenanceCommand extends 
VdsCommand<VdsActionParameters> {
declare params generic type with SetHaMaintenanceParameters, will save the 
casting.
Line 20: 
Line 21:     public SetHaMaintenanceCommand(VdsActionParameters 
vdsActionParameters) {
Line 22:         super(vdsActionParameters);
Line 23:     }


http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/SetHaMaintenanceParameters.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/SetHaMaintenanceParameters.java:

Line 17:     public SetHaMaintenanceParameters() {
Line 18:     }
Line 19: 
Line 20:     public HaMaintenanceMode getMode() {
Line 21:         return this.mode;
minor: I personally think that 'this' is redundant here.
Line 22:     }
Line 23: 
Line 24:     public boolean getIsEnabled() {
Line 25:         return this.enabled;


http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java:

Line 361:         severities.put(AuditLogType.VDS_STORAGE_VDS_STATS_FAILED, 
AuditLogSeverity.ERROR);
Line 362:         severities.put(AuditLogType.VDS_LOW_DISK_SPACE, 
AuditLogSeverity.WARNING);
Line 363:         severities.put(AuditLogType.VDS_LOW_DISK_SPACE_ERROR, 
AuditLogSeverity.ERROR);
Line 364:         severities.put(AuditLogType.VDS_ACTIVATE_ASYNC, 
AuditLogSeverity.NORMAL);
Line 365:         severities.put(AuditLogType.VDS_ACTIVATE_MANUAL_HA, 
AuditLogSeverity.WARNING);
duplicate line
Line 366:         severities.put(AuditLogType.VDS_ACTIVATE_FAILED_ASYNC, 
AuditLogSeverity.NORMAL);
Line 367:         
severities.put(AuditLogType.VDS_SET_NON_OPERATIONAL_VM_NETWORK_IS_BRIDGELESS, 
AuditLogSeverity.WARNING);
Line 368:         
severities.put(AuditLogType.EMULATED_MACHINES_INCOMPATIBLE_WITH_CLUSTER, 
AuditLogSeverity.WARNING);
Line 369:         severities.put(AuditLogType.VDS_TIME_DRIFT_ALERT, 
AuditLogSeverity.WARNING);


http://gerrit.ovirt.org/#/c/23531/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java:

Line 1866:             return;
Line 1867:         }
Line 1868: 
Line 1869:         SetHaMaintenanceParameters params = new 
SetHaMaintenanceParameters(vm.getRunOnVds(), HaMaintenanceMode.GLOBAL, enabled);
Line 1870:         
Frontend.getInstance().runAction(VdcActionType.SetHaMaintenance, params, null, 
this);
minor: you can use runAction overload with type and params.
Line 1871:     }
Line 1872: 
Line 1873:     private void preSave()
Line 1874:     {


Line 2388:                             VDS vds = response.getReturnValue();
Line 2389:                             
setHaMaintenanceAvailability(vds.getHighlyAvailableIsConfigured());
Line 2390:                         }
Line 2391:                     }
Line 2392:                 }, true));
I'm afraid we cannot query the server for each click, especially in the vm 
screen. please come up with a different solution (you can check it when 
performing the command), or just let the server deal with it.
Line 2393:     }
Line 2394: 
Line 2395:     private void setHaMaintenanceAvailability(boolean isAvailable) {
Line 2396:         
getEnableGlobalHaMaintenanceCommand().setIsExecutionAllowed(isAvailable);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f76f7ad63bcf6d7871c362b46cfa6e928eb9c74
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Jiří Moskovčák <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to