Liron Aravot has posted comments on this change.

Change subject: core: Push ioTune QoS info when hotplugging disk
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.ovirt.org/#/c/33907/3/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java:

Line 67:             drive.put(VdsProperties.VolumeId, 
diskImage.getImageId().toString());
Line 68:             drive.put(VdsProperties.ImageId, 
diskImage.getId().toString());
Line 69:             drive.put(VdsProperties.PropagateErrors, 
disk.getPropagateErrors().toString().toLowerCase());
Line 70: 
Line 71:             // maps to avoid fetching qos object for same disk profile 
id
the comment is unneeded here, we fetch here always only once (plugging one disk)
Line 72:             Map<Guid, Guid> diskProfileStorageQosMap = new HashMap<>();
Line 73:             Map<Guid, Map<String, Long>> storageQosIoTuneMap = new 
HashMap<>();
Line 74:             Map<String, Long> ioTune =
Line 75:                     VmInfoBuilder.buildIoTune(diskImage, 
diskProfileStorageQosMap, storageQosIoTuneMap);


Line 69:             drive.put(VdsProperties.PropagateErrors, 
disk.getPropagateErrors().toString().toLowerCase());
Line 70: 
Line 71:             // maps to avoid fetching qos object for same disk profile 
id
Line 72:             Map<Guid, Guid> diskProfileStorageQosMap = new HashMap<>();
Line 73:             Map<Guid, Map<String, Long>> storageQosIoTuneMap = new 
HashMap<>();
maybe it'll be better to tweak the code a bit to avoid creating those maps each 
time..as we don't need the caching here
Line 74:             Map<String, Long> ioTune =
Line 75:                     VmInfoBuilder.buildIoTune(diskImage, 
diskProfileStorageQosMap, storageQosIoTuneMap);
Line 76:             if (ioTune != null) {
Line 77:                 if (vmDevice.getSpecParams() == null) {


Line 72:             Map<Guid, Guid> diskProfileStorageQosMap = new HashMap<>();
Line 73:             Map<Guid, Map<String, Long>> storageQosIoTuneMap = new 
HashMap<>();
Line 74:             Map<String, Long> ioTune =
Line 75:                     VmInfoBuilder.buildIoTune(diskImage, 
diskProfileStorageQosMap, storageQosIoTuneMap);
Line 76:             if (ioTune != null) {
this code should be shared with VmInfoBuilder somewhere, otherwise it's error 
prone (no one will remember to change both places..)
Line 77:                 if (vmDevice.getSpecParams() == null) {
Line 78:                     vmDevice.setSpecParams(new HashMap<String, 
Object>());
Line 79:                 }
Line 80:                 vmDevice.getSpecParams().put(VdsProperties.Iotune, 
ioTune);


-- 
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: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tomer Saban <[email protected]>
Gerrit-Reviewer: [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

Reply via email to