Liron Aravot has posted comments on this change. Change subject: core: Push ioTune QoS info when hotplugging disk ......................................................................
Patch Set 10: (2 comments) http://gerrit.ovirt.org/#/c/33907/10/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java: Line 332: if (FeatureSupported.storageQoS(vm.getVdsGroupCompatibilityVersion())) { Line 333: ioTune = buildIoTune(diskImage, diskProfileStorageQosMap, storageQosIoTuneMap); Line 334: } Line 335: Line 336: addIoTuneParams(vm, vmDevice, ioTune); 1. please move this to be after line 333, you can also remove the defintion from line 331 (define within the if clause) 2. after you change that,consider the following- we can have a new method - handleIoTune that this if condition will move to ..something like that public void handleIoTune(.._) { if (FeatureSUpported..) { ioTune = buildIoTune addIoTuneParams(...) } now we can call this method from here and from HotPlugVdsCommand and avoid the if condition in line 361. what do you think? Line 337: } else { Line 338: LunDisk lunDisk = (LunDisk) disk; Line 339: struct.put(VdsProperties.Guid, lunDisk.getLun().getLUN_id()); Line 340: struct.put(VdsProperties.Format, VolumeFormat.RAW.toString().toLowerCase()); Line 358: ArchStrategyFactory.getStrategy(vm.getClusterArch()).run(new CreateAdditionalControllers(devices)); Line 359: } Line 360: Line 361: static void addIoTuneParams(VM vm, VmDevice vmDevice, Map<String, Long> ioTune) { Line 362: if (FeatureSupported.storageQoS(vm.getVdsGroupCompatibilityVersion())) { this if can be removed, see my previous comment..if you leave it, it should be merged with the if on 363- if (ioTUne != null && FeatureSupported...) Line 363: if (ioTune != null) { Line 364: if (vmDevice.getSpecParams() == null) { Line 365: vmDevice.setSpecParams(new HashMap<String, Object>()); Line 366: } -- To view, visit http://gerrit.ovirt.org/33907 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic4bb85cd307089088be77cff28adbc783ebcaedd Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Tomer Saban <tsa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
