Tomas Jelinek has posted comments on this change.

Change subject: webadmin: support Memory Balloon
......................................................................


Patch Set 2: (4 inline comments)

I see you have added this check box to vm/pool/template dialogs but you 
actually use it only for VM (and also for it  only partially). If it is 
supposed to be on all the mentioned dialogs you miss the logic for 
PoolListModel and TemplateListModel and partially for VmListModel (e.g. for 
onNewTemplate).

Where is it supposed to be? If not on all the mentioned places please describe 
the intended places in commit message.

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
Line 1981: 
Line 1982:         
getcurrentVm().setCustomProperties(model.getCustomPropertySheet().getEntity());
Line 1983:         getcurrentVm().setBalloonEnabled((Boolean) 
model.getMemoryBalloonDeviceEnabled().getEntity()
Line 1984:             && 
model.getSelectedCluster().getcompatibility_version().compareTo(BALLOON_DEVICE_MIN_VERSION)
 >= 0);
Line 1985: 
why do you need to set the balloon property of VM if you set it to the params?
Line 1986:         EntityModel displayProtocolSelectedItem = (EntityModel) 
model.getDisplayProtocol().getSelectedItem();
Line 1987:         getcurrentVm().setDefaultDisplayType((DisplayType) 
displayProtocolSelectedItem.getEntity());
Line 1988: 
Line 1989:         EntityModel prioritySelectedItem = (EntityModel) 
model.getPriority().getSelectedItem();


Line 2037: 
Line 2038:                 AddVmFromScratchParameters parameters = new 
AddVmFromScratchParameters(getcurrentVm(),
Line 2039:                         new ArrayList<DiskImage>(),
Line 2040:                         Guid.Empty);
Line 2041:                 parameters.setSoundDeviceEnabled((Boolean) 
model.getIsSoundcardEnabled().getEntity());
don't you need to set the balloon also here?
Line 2042:                 parameters.setConsoleEnabled((Boolean) 
model.getIsConsoleDeviceEnabled().getEntity());
Line 2043: 
Line 2044:                 setVmWatchdogToParams(model, parameters);
Line 2045: 


Line 2068:                             AddVmFromTemplateParameters param = new 
AddVmFromTemplateParameters(
Line 2069:                                     vmListModel.getcurrentVm(),
Line 2070:                                     
unitVmModel.getDisksAllocationModel().getImageToDestinationDomainMap(),
Line 2071:                                     Guid.Empty);
Line 2072:                             param.setSoundDeviceEnabled((Boolean) 
model.getIsSoundcardEnabled().getEntity());
don't you need to set the balloon also here?
Line 2073:                             param.setConsoleEnabled((Boolean) 
model.getIsConsoleDeviceEnabled().getEntity());
Line 2074: 
Line 2075:                             
Frontend.RunAction(VdcActionType.AddVmFromTemplate, param, new 
NetworkCreateOrUpdateFrontendActionAsyncCallback(model, 
defaultNetworkCreatingManager), vmListModel);
Line 2076:                             
param.setCopyTemplatePermissions((Boolean) 
model.getCopyPermissions().getEntity());


Line 2146:                                 {
Line 2147:                                     VmManagementParametersBase 
updateVmParams =
Line 2148:                                             new 
VmManagementParametersBase(vmListModel.getcurrentVm());
Line 2149:                                     setVmWatchdogToParams(model, 
updateVmParams);
Line 2150:                                     
updateVmParams.setSoundDeviceEnabled((Boolean) model.getIsSoundcardEnabled()
don't you need to set the balloon also here?
Line 2151:                                             .getEntity());
Line 2152: 
Line 2153:                                     
Frontend.RunAction(VdcActionType.UpdateVm,
Line 2154:                                             updateVmParams, new 
NetworkUpdateFrontendAsyncCallback(model, defaultNetworkCreatingManager, 
vmListModel.getcurrentVm().getId()), vmListModel);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ce357b592a825e0ab96290848aa4f4cb3e20ace
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to