Tomas Jelinek 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 something like: getModel().getNumOfSockets().setIsChangable(isHotSetCpuSupported() || !vm.isRunning()); 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-yet-inited values (e.g. if something == 0 than return). I would do it the same also here. 2: but if you decide to go this way, you need to register the listeners back (I see only the numOfSockets.selectedItemChangedEvent to be registered back). 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 to be ignored (like in the parent method) and not that the "totalCpuCores" should be set to "0". 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 times 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 booleans? Something like: boolean vmUp = getVm().getStatus() == VMStatus.Up; boolean hotplugGloballyEnabled = ((Boolean) AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.HotPlugEnabled, clusterVersion.getValue())); boolean hotplugSupported = Boolean.parseBoolean(((Map<String, String>) AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.HotPlugCpuSupported, clusterVersion.getValue())).get(getModel().getSelectedCluster().getArchitecture().name())); return vmUp && hotplugGloballyEnabled && hotplugSupported; 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
