Omer Frenkel has posted comments on this change. Change subject: events: removing vm.getStats from event processing ......................................................................
Patch Set 17: (5 comments) https://gerrit.ovirt.org/#/c/40205/17/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 79: VM dbVm = dbFacade.getVmDao().get(vmid); Line 80: Line 81: VmInternalData vdsmVm = createVmInternal(entry, dbVm); Line 82: Line 83: VmInternalData vmData = createVmInternalData(dbVm, (Map<String, Object>) map.get(vmid.toString()), notifyTime); i still think we should merge createVmInternal and createVmInternalData to a single method return a single VmInternalData the bug here still exists: vdsmVm.hash is null (because we set it only to vmData), and this is the object we send to isDevicesChanged (line 92 below) so the devicesChangedVms list will always be empty Line 84: // make sure to ignore events from other hosts during migration Line 85: // and process once the migration is done Line 86: if (dbVm != null && (dbVm.getRunOnVds() == null Line 87: || dbVm.getRunOnVds().equals(manager.getVdsId()) Line 84: // make sure to ignore events from other hosts during migration Line 85: // and process once the migration is done Line 86: if (dbVm != null && (dbVm.getRunOnVds() == null Line 87: || dbVm.getRunOnVds().equals(manager.getVdsId()) Line 88: || (!dbVm.getRunOnVds().equals(manager.getVdsId()) && vmData.getVmDynamic().getStatus() == VMStatus.Up))) { if dbVm is null we will get exception on the second part of the 'if' Line 89: if (vmData != null) { Line 90: changedVms.add(new Pair<>(dbVm, vmData)); Line 91: } Line 92: if (isDevicesChanged(dbVm, vdsmVm)) { Line 97: } Line 98: Line 99: @SuppressWarnings("unchecked") Line 100: private VmInternalData createVmInternal(Map.Entry<String, Object> entry, VM dbVm) { Line 101: VmDynamic vmdynamic = new VmDynamic(dbVm.getDynamicData()); db vm can be null (in case of external vms) please add a null check Line 102: vmdynamic.setId(new Guid((String) entry.getKey())); Line 103: Map<String, Object> stats = (Map<String, Object>) entry.getValue(); Line 104: vmdynamic.setStatus(convertToVmStatus((String) stats.get(VdsProperties.status))); Line 105: return new VmInternalData(vmdynamic, null, null, null); Line 105: return new VmInternalData(vmdynamic, null, null, null); Line 106: } Line 107: Line 108: private VmInternalData createVmInternalData(VM dbVm, Map<String, Object> xmlRpcStruct, Double notifyTime) { Line 109: VmDynamic vmDynamic = new VmDynamic(); this also should be based on dbVm because this is what we send to the monitoring (dont forget to check its not null) Line 110: if (xmlRpcStruct.containsKey(VdsProperties.status)) { Line 111: vmDynamic.setStatus(convertToVmStatus((String) xmlRpcStruct.get(VdsProperties.status))); Line 112: } Line 113: Line 131: } else { Line 132: vmDynamic.setExitReason(VmExitReason.Unknown); Line 133: } Line 134: Line 135: return new VmInternalData(vmDynamic, dbVm.getStatisticsData(), null, new HashMap<String, LUNs>()); dbVm can be null Line 136: } Line 137: Line 138: private Double parseDouble(Object value) { Line 139: if (Long.class.isInstance(value)) { -- 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: 17 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: Jenkins CI 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-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
