Michael Kublin has posted comments on this change.
Change subject: engine-core: Prestarted Vm (2 - Review Only)
......................................................................
Patch Set 6: (9 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachUserToVmFromPoolAndRunCommand.java
Line 111: boolean isPrestartedVm = false;
After introducing getPrestartedVmToAttach, we should move a GLOBAL LOCK, just
please go inside an internal methods and calculate a number of db queries.
First of all it is too many , but inside a CS it will be a disaster.
Line 112: synchronized (_lockObject) {
Maybe better to get all vmIds, and after check prestarted and notPrestarted.
It is look like that the same queries will be run in the future, but with
different filtering in BLL. Not good as for me
Line 113:
setVmId(getPrestartedVmToAttach(getParameters().getVmPoolId()));
Please write CONST.equals(something)
Line 164: // (since 'Vm' is dependant on 'mVmId', which is not set
here).
Check, I think that line can be removed
Line 171:
You don't need to check that, tje check is done inside RunVm, if it is not
successes
EndSuccessfully should not be called.
Line 172: if
(DbFacade.getInstance().getDiskImageDAO().getAllStatelessVmImageMapsForVm(getVm().getvm_guid()).size()
> 0) {
Those code will cause for Backend.getInstance().endAction() run twice
Line 202: // (since 'Vm' is dependant on 'mVmId', which is not set
here).
why do these? (Not your code, but you already here)
Line 206: // the DB so we won't get a log-deadlock because of the
transaction.
same here
Line 208:
why endAction of RunVm will not be called automatically ?
--
To view, visit http://gerrit.ovirt.org/1798
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3c0b0d69c0da82d3ba2dcbcf045cd92a6db7bef
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Muli Salem <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Muli Salem <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
