Omer Frenkel has posted comments on this change. Change subject: events: sending stats with vm status change event ......................................................................
Patch Set 2: (3 comments) we are still missing a solution for concurrency of event and getAllStats thread https://gerrit.ovirt.org/#/c/40205/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/EventVMStatsRefresher.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/EventVMStatsRefresher.java: Line 72: for (Map.Entry<String, Object> entry : map.entrySet()) { Line 73: VmDynamic vmdynamic = new VmDynamic(); Line 74: vmdynamic.setId(new Guid((String) entry.getKey())); Line 75: Map<String, Object> attributes = (Map<String, Object>) map.get(entry.getKey()); Line 76: vmdynamic.setStatus(convertToVmStatus((String) attributes.get("status"))); why this is needed, isnt it part of the stats? i think the code in prepareChanges() below that gets the data from stats should be here instead this (the vmInternalData is created twice, here and in prepareChanges, i dont see why) Line 77: Line 78: VmInternalData vmData = new VmInternalData(vmdynamic, null, null, null); Line 79: returnVMs.add(vmData); Line 80: } Line 88: for (VmInternalData vdsmVm : vms) { Line 89: Guid vmid = vdsmVm.getVmDynamic().getId(); Line 90: VM dbVm = dbVms.get(vmid); Line 91: Line 92: VDS host = dbFacade.getVdsDao().get(manager.getVdsId()); you can use manager.getCopyVds() and save a db call Line 93: Map<String, Object> attributes = (Map<String, Object>) map.get(vmid.toString()); Line 94: VmInternalData vmData = Line 95: VmStatsVdsBrokerCommand.createVmInternalData((Map<String, Object>) attributes.get("stats"), Line 96: host); https://gerrit.ovirt.org/#/c/40205/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmStatsVdsBrokerCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmStatsVdsBrokerCommand.java: Line 25: protected Object getReturnValueFromBroker() { Line 26: return mVmListReturn; Line 27: } Line 28: Line 29: public static VmInternalData createVmInternalData(Map<String, Object> xmlRpcStruct, VDS host) { it would be nicer to add an overload here instead changing the callers Line 30: VmDynamic vmDynamic = new VmDynamic(); Line 31: VdsBrokerObjectsBuilder.updateVMDynamicData(vmDynamic, xmlRpcStruct); Line 32: adjustDisplayIp(vmDynamic.getGraphicsInfos(), host); Line 33: return new VmInternalData(vmDynamic, -- To view, visit https://gerrit.ovirt.org/40205 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib15df9ce78bbb86c9284bdd6b772d0e6801db765 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Roy Golan <rgo...@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
