Gilad Chaplik has posted comments on this change.

Change subject: engine: watchdog - frontend
......................................................................


Patch Set 5: (7 inline comments)

nice patch, some comments

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingVmModelBehavior.java
Line 82:             public void OnSuccess(Object target, Object returnValue) {
Line 83:                 UnitVmModel model = (UnitVmModel) target;
Line 84:                 VdcQueryReturnValue val = (VdcQueryReturnValue) 
returnValue;
Line 85:                 @SuppressWarnings("unchecked")
Line 86:                 Collection<VmWatchdog> watchdogs = 
(Collection<VmWatchdog>) val.getReturnValue();
null check or move the casting logic to converter within AsyncDataProvider
Line 87:                 for(VmWatchdog watchdog: watchdogs) {
Line 88:                     
model.getWatchdogAction().setEntity(watchdog.getAction());
Line 89:                     
model.getWatchdogModel().setEntity(watchdog.getModel());
Line 90:                 }


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/TemplateVmModelBehavior.java
Line 61:             public void OnSuccess(Object target, Object returnValue) {
Line 62:                 UnitVmModel model = (UnitVmModel) target;
Line 63:                 VdcQueryReturnValue val = (VdcQueryReturnValue) 
returnValue;
Line 64:                 @SuppressWarnings("unchecked")
Line 65:                 Collection<VmWatchdog> watchdogs = 
(Collection<VmWatchdog>) val.getReturnValue();
same.
Line 66:                 for(VmWatchdog watchdog: watchdogs) {
Line 67:                     
model.getWatchdogAction().setEntity(watchdog.getAction());
Line 68:                     
model.getWatchdogModel().setEntity(watchdog.getModel());
Line 69:                 }


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
Line 1157: 
Line 1158:         setWatchdogAction(new NotChangableForVmInPoolListModel());
Line 1159:         
getWatchdogAction().getEntityChangedEvent().addListener(this);
Line 1160:         ArrayList<String> watchDogActions = new ArrayList<String>();
Line 1161:         watchDogActions.add("none"); //$NON-NLS-1$
see comment in previous patch in patch-set, consider using enums (localization)
Line 1162:         watchDogActions.add("reset"); //$NON-NLS-1$
Line 1163:         watchDogActions.add("poweroff"); //$NON-NLS-1$
Line 1164:         watchDogActions.add("dump"); //$NON-NLS-1$
Line 1165:         watchDogActions.add("pause"); //$NON-NLS-1$


Line 1169:         getWatchdogModel().getEntityChangedEvent().addListener(this);
Line 1170:         ArrayList<String> watchDogModels = new ArrayList<String>();
Line 1171:         watchDogModels.add(""); //$NON-NLS-1$
Line 1172:         watchDogModels.add("i6300esb"); //$NON-NLS-1$
Line 1173:         watchDogModels.add("ib700"); //$NON-NLS-1$
same.
Line 1174:         getWatchdogModel().setItems(watchDogModels);
Line 1175: 
Line 1176:         setIsAutoAssign(new NotChangableForVmInPoolEntityModel());
Line 1177:         getIsAutoAssign().getEntityChangedEvent().addListener(this);


Line 1217:         setCpuPinning(new NotChangableForVmInPoolEntityModel());
Line 1218:         getCpuPinning().setEntity("");
Line 1219:         getCpuPinning().setIsChangable(false);
Line 1220: 
Line 1221:         Frontend.RunQuery(VdcQueryType.GetWatchdog, new 
GetWatchdogParameters(null), new AsyncQuery(this, new INewAsyncCallback() {
vm id = null?
Line 1222: 
Line 1223:             public void OnSuccess(Object model, Object returnValue) {
Line 1224:                 VmWatchdog vmWatchdog = (VmWatchdog) model;
Line 1225:                 
getWatchdogAction().setEntity(vmWatchdog.getAction());


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
Line 2153:                                                     
VdcReturnValueBase retVal = result1.getReturnValue();
Line 2154:                                                     if (retVal != 
null && retVal.getSucceeded())
Line 2155:                                                     {
Line 2156:                                                         
vmListModel1.Cancel();
Line 2157:                                                     }
we should not create watchdog if model is null.
Line 2158:                                                     
UpdateWatchdogParameters watchdogParameters = new UpdateWatchdogParameters();
Line 2159:                                                     
watchdogParameters.setVmId(getcurrentVm().getId());
Line 2160:                                                     
watchdogParameters.setAction((String) model.getWatchdogAction().getEntity());
Line 2161:                                                     
watchdogParameters.setModel((String) model.getWatchdogModel().getEntity());


Line 2192:                                 if (returnValueBase != null && 
returnValueBase.getSucceeded())
Line 2193:                                 {
Line 2194:                                     vmListModel.Cancel();
Line 2195:                                 }
Line 2196: 
we should not update watch dog if model/action didn't change.
my concerns are in updating a vm in userportal and overriding watchdog in case 
it's not set.
Line 2197:                                 final UpdateWatchdogParameters 
watchdogParameters = new UpdateWatchdogParameters();
Line 2198:                                 
watchdogParameters.setVmId(getcurrentVm().getId());
Line 2199:                                 
watchdogParameters.setAction((String) model.getWatchdogAction().getEntity());
Line 2200:                                 watchdogParameters.setModel((String) 
model.getWatchdogModel().getEntity());


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3f72fabf35d32a05a75652ced362c5bee32b8d7
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to