Roy Golan has posted comments on this change. Change subject: UI: hot set number of CPU when updating a VM ......................................................................
Patch Set 3: (5 comments) http://gerrit.ovirt.org/#/c/23252/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingVmModelBehavior.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingVmModelBehavior.java: Line 185: getModel().getIsDeleteProtected().setEntity(vm.isDeleteProtected()); Line 186: getModel().selectSsoMethod(vm.getSsoMethod()); Line 187: Line 188: getModel().getNumOfSockets().setSelectedItem(vm.getNumOfSockets()); Line 189: getModel().getNumOfSockets().setIsChangable(isHotSetCpuSupported()); > but you can still edit this if the vm is not running, right? So I would say indeed Line 190: Line 191: getModel().getCoresPerSocket().setIsChangable(!vm.isRunning()); Line 192: Line 193: getModel().getKernel_parameters().setEntity(vm.getKernelParams()); Line 273: if (isHotSetCpuSupported()) { Line 274: // cancel related events while fetching data Line 275: getModel().getTotalCPUCores().getEntityChangedEvent().removeListener(getModel()); Line 276: getModel().getCoresPerSocket().getSelectedItemChangedEvent().removeListener(getModel()); Line 277: getModel().getNumOfSockets().getSelectedItemChangedEvent().removeListener(getModel()); > 1: why remove the listeners? AFAIK the parent class just ignores the not-ye I don't want any events on them if the dialog is dealing with hot plug situation I just want to change the total cpus if the sockets change so this is the only event I register back on totalCpuCoresChanged() Line 278: Line 279: AsyncDataProvider.getHostById(new AsyncQuery(this, new INewAsyncCallback() { Line 280: @Override Line 281: public void onSuccess(Object model, Object returnValue) { Line 370: Line 371: @Override Line 372: public void numOfSocketChanged() { Line 373: if (isHotSetCpuSupported()) { Line 374: int numOfSockets = extractIntFromListModel(getModel().getNumOfSockets()); > The "numOfSockets" can be "0" but it means that it is not yet inited so has I can add it but it doesn't change much as the the last call to this method is the one that counts and its the one that being triggeretd when the callback of fetching the host is called. essentially line 398 Line 375: int coresPerSocket = vm.getCpuPerSocket(); Line 376: getModel().getTotalCPUCores().setEntity(Integer.toString(numOfSockets * coresPerSocket)); Line 377: } else { Line 378: super.numOfSocketChanged(); Line 393: Line 394: getModel().getCoresPerSocket().setSelectedItem(vm.getCpuPerSocket()); Line 395: getModel().getNumOfSockets().setSelectedItem(vm.getNumOfSockets()); Line 396: Line 397: getModel().getNumOfSockets().getSelectedItemChangedEvent().addListener(getModel()); > this method can be called more times so it will add the same listener more its being called once AFAIK - after the callback is complete and runningHost != null. unless I'm missing something? Line 398: numOfSocketChanged(); Line 399: } else { Line 400: super.totalCpuCoresChanged(); Line 401: } Line 418: return res; Line 419: } Line 420: Line 421: public boolean isHotSetCpuSupported() { Line 422: Version clusterVersion = getModel().getSelectedCluster().getcompatibility_version(); > Well, this is pretty unreadable... what about splitting it up to named bool Done Line 423: return getVm().getStatus() == VMStatus.Up && Line 424: ((Boolean) AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.HotPlugEnabled, clusterVersion.getValue())) && Line 425: Boolean.parseBoolean(((Map<String, String>) AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.HotPlugCpuSupported, Line 426: clusterVersion.getValue())).get(getModel().getSelectedCluster().getArchitecture().name())); -- To view, visit http://gerrit.ovirt.org/23252 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieac0e757c54b002eeab8f9099e6e8d151eb43340 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[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
