Sergey Gotliv has posted comments on this change.

Change subject: core: possible NPE in UpdateVmDisk
......................................................................


Patch Set 1:

(1 comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 54:     private List<PermissionSubject> listPermissionSubjects;
Line 55:     private Map<Guid, List<Disk>> otherVmDisks = new HashMap<Guid, 
List<Disk>>();
Line 56:     private List<VM> vmsDiskPluggedTo, vmsDiskSnapshotPluggedTo, 
vmsDiskOrSnapshotPluggedTo,
Line 57:             vmsDiskOrSnapshotAttachedTo = Collections.emptyList();
Line 58: 
Wow!!!
Do we have performance issues I am not aware of!

Can you show me a profile results?

What exactly faster here and memory consuming? 
In 99.9% of all cases (success scenarios) you will need to instantiate it in 
the "load" method anyway! In 0.01% of cases where getOldDisk() is null you have 
bigger problems to take care of than this instantiating.
Line 59:     /**
Line 60:      * vm device for the given vm and disk
Line 61:      */
Line 62:     private VmDevice vmDeviceForVm;


-- 
To view, visit http://gerrit.ovirt.org/20639
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I787b4a9e723e3096b5d22a5a925cd976940497ff
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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