Moti Asayag has posted comments on this change. Change subject: events: VM status update ......................................................................
Patch Set 4: Code-Review-1 (3 comments) I agree with Omer's comment. The event should trigger an action on the system, based on the new status of the vm. we should also differentiate between compatibility levels of vdsm when subsribing the VdsManager to events: this is the same flow as hosts < 3.5 which won't support it. https://gerrit.ovirt.org/#/c/37758/4/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java: Line 162: TimeUnit.MILLISECONDS); Line 163: } Line 164: Line 165: public void subscribeEvents() { Line 166: ResourceManager.getInstance().subscribe(new EventSubscriber(this.cachedVds.getHostName() + "|*|VM_status|*") { I assume that ResourceManager.getInstance().subscribe() i planned to be used from other places as well (else it could have been added directly to VdsManager) Line 167: Line 168: private VmDynamicDAO dao = DbFacade.getInstance().getVmDynamicDao(); Line 169: Line 170: @Override Line 176: @Override Line 177: public void onNext(Map<String, Object> map) { Line 178: try { Line 179: Set<String> keys = map.keySet(); Line 180: if (keys == null || keys.isEmpty()) 1. please surround with parenthesis. 2. you can replace it with CollectionUtils.isEmpty(keys) which does the same. Line 181: return; Line 182: Line 183: for (String key: keys) { Line 184: VmDynamic vmDynamic = this.dao.get(Guid.createGuidFromString(key)); Line 182: Line 183: for (String key: keys) { Line 184: VmDynamic vmDynamic = this.dao.get(Guid.createGuidFromString(key)); Line 185: vmDynamic.setStatus(convertToVmStatus((String)map.get(key))); Line 186: this.dao.update(vmDynamic); VmDynamicDAO extends the StatusAwareDao interface which has the updateStatus(ID id, S status). It should be more efficient than the update(VmDynamic). It will also reduce the need to fetch the vmDynamic, since the required parameterse (Id and status) are already in available. Line 187: } Line 188: } finally { Line 189: subscription.request(1); Line 190: } -- To view, visit https://gerrit.ovirt.org/37758 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3fb17e714390d07b215f1251d18fca9195c38565 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Roy Golan <[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
