Tomas Jelinek has posted comments on this change.
Change subject: Engine: update video device save wrong values
......................................................................
Patch Set 1:
(3 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
Line 63: updateBootOrderInVmDeviceAndStoreToDB(entity);
Line 64: }
Line 65:
Line 66: // if the console type has changed, recreate Video devices
Line 67: if (oldVmBase.getDefaultDisplayType() !=
entity.getDefaultDisplayType() || (entity.getDefaultDisplayType() ==
DisplayType.qxl && oldVmBase.getNumOfMonitors() != entity
this expression is quite complex and unreadable. What about extracting it's
subparts to named variables like this?
...
boolean displayTypeChanged = oldVmBase.getDefaultDisplayType() !=
entity.getDefaultDisplayType();
boolean numOfMonitorsChanged = entity.getDefaultDisplayType() ==
DisplayType.qxl && oldVmBase.getNumOfMonitors() != entity.getNumOfMonitors();
boolean singleQxlChanged = oldVmBase.getSingleQxlPci() !=
entity.getSingleQxlPci();
if(displayTypeChanged || numOfMonitorsChanged || singleQxlChanged) {
...
Line 68: .getNumOfMonitors()) ||
oldVmBase.getSingleQxlPci() != entity.getSingleQxlPci()) {
Line 69: // delete all video device
Line 70: for (VmDevice device :
dao.getVmDeviceByVmIdAndType(oldVmBase.getId(), VmDeviceGeneralType.VIDEO)) {
Line 71: dao.remove(device.getId());
Line 585: /**
Line 586: * updates new/existing VM video cards in vm device
Line 587: * @param oldVmBase
Line 588: * @param newStatic
Line 589: */
this method is not used anymore so can be removed
Line 590: private static void updateNumOfMonitorsInVmDevice(VmBase
oldVmBase,
Line 591: VmBase newStatic) {
Line 592: int prevNumOfMonitors=0;
Line 593: if (oldVmBase != null) {
Line 831: String mem;
Line 832: if (singleQxlPci) {
Line 833: mem = String.valueOf(VmDeviceCommonUtils.LOW_VIDEO_MEM *
heads);
Line 834: } else {
Line 835: mem = (numOfMonitors < 2 ?
String.valueOf(VmDeviceCommonUtils.LOW_VIDEO_MEM) :
Why have you changed the "> 2" to "< 2"?
Line 836: String.valueOf(VmDeviceCommonUtils.HIGH_VIDEO_MEM));
Line 837: }
Line 838: Map<String, Object> specParams = new HashMap<String,
Object>();
Line 839: specParams.put(VRAM, mem);
--
To view, visit http://gerrit.ovirt.org/18786
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6cc48d5eb3fc60cba022d8f6b4a8dee3760be5e5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shav...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
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
