Martin Peřina has posted comments on this change. Change subject: PendingResourceManager for tracking resources in WaitForLaunch ......................................................................
Patch Set 6: (3 comments) https://gerrit.ovirt.org/#/c/40136/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/pending/AbstractPendingResource.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/pending/AbstractPendingResource.java: Line 15: public abstract class AbstractPendingResource { Line 16: protected VDS host; Line 17: protected VM vm; Line 18: Line 19: public AbstractPendingResource(Guid host, VM vm) { Is this really necessary? Wouldn't it be cleaner to have only AbstractPendingResource(VDS, VM) constructor and take care of either creating new VDS instance and setting its id or fetching VDS by its id from db on caller? Line 20: this.host = new VDS(); Line 21: this.host.setId(host); Line 22: this.vm = vm; Line 23: } https://gerrit.ovirt.org/#/c/40136/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java: Line 1183: public void setFenceAgents(List<FenceAgent> fenceAgents) { Line 1184: this.fenceAgents = fenceAgents; Line 1185: } Line 1186: Line 1187: public void calculateFreeVirtualMemory() { IMO this method doesn't belong to entity object. It should be moved into some helper/manager class which would do the calculation using entity attributes and PendingResourceManager Line 1188: if (getMemCommited() != null && getPhysicalMemMb() != null && getReservedMem() != null) { Line 1189: maxSchedulingMemory = (getMaxVdsMemoryOverCommit() * getPhysicalMemMb() / 100.0f) Line 1190: - getMemCommited() Line 1191: - getReservedMem() https://gerrit.ovirt.org/#/c/40136/6/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAO.java: Line 27: * @param pendingVmemSize - a new value will be GREATEST(pending_vmem_size + pendingVmemSize, 0) Line 28: * @param memCommited - will decrease or increase value of mem_commited by ABS(memCommited) + guest_overhead Line 29: * @param vmsCoresCount - a new value will be GREATEST(vms_cores_count + v_vmsCoresCount, 0) Line 30: */ Line 31: void updatePartialVdsDynamicCalc(Guid id, int vmCount, int pendingVcpusCount, int pendingVmemSize, int memCommited, int vmsCoresCount); If the method is deprecated then please mark it with @Deprecated Line 32: Line 33: /** Line 34: * This method will update the controlled_by_pm_policy flag in DB. Line 35: * @param id - id or record to be updated -- To view, visit https://gerrit.ovirt.org/40136 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie741b8e244acb64e470af83252d90ec134ba7f8e Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@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
