Arik Hadas has posted comments on this change. Change subject: events: removing vm.getStats from event processing ......................................................................
Patch Set 15: (5 comments) https://gerrit.ovirt.org/#/c/40205/15/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 82: // but as part of infra code we add time when event was sent and Line 83: // we need to ignore it here Line 84: if (VdsProperties.notify_time.equals(entry.getKey())) { Line 85: continue; Line 86: } > Done where? not included in patch-set 16 Line 87: Guid vmid = new Guid((String) entry.getKey()); Line 88: VM dbVm = dbFacade.getVmDao().get(vmid); Line 89: Line 90: VmInternalData vdsmVm = createVmInternal(entry, dbVm); Line 86: } Line 87: Guid vmid = new Guid((String) entry.getKey()); Line 88: VM dbVm = dbFacade.getVmDao().get(vmid); Line 89: Line 90: VmInternalData vdsmVm = createVmInternal(entry, dbVm); > It is based on current code and we need vdsmVm for changed devices. I think that I didn't explain myself well.. vdsmVm - based on dbVm + status that was received vmData - status, exist-reason, exit-code, exist-message and status-update-time that were received. If we'll have just one VM that will be based on dbVm and all the field I mentioned above that we received from vdsm will override the existing values, we could use this VM everywhere Line 91: Line 92: VmInternalData vmData = createVmInternalData(dbVm, (Map<String, Object>) map.get(vmid.toString()), notifyTime); Line 93: // make sure to ignore events from other hosts during migration Line 94: // and process once the migration is done Line 128: vmDynamic.setHash(hash); Line 129: } catch (Exception e) { Line 130: log.error("Illegal vm hash '{}'.", hash); Line 131: } Line 132: } > This code is based on current code base. I didn't blame you :) but if you touch it.. Line 133: Line 134: if (xmlRpcStruct.containsKey(VdsProperties.exit_code)) { Line 135: String exitCodeStr = xmlRpcStruct.get(VdsProperties.exit_code).toString(); Line 136: vmDynamic.setExitStatus(VmExitStatus.forValue(Integer.parseInt(exitCodeStr))); Line 142: if (xmlRpcStruct.containsKey(VdsProperties.exit_reason)) { Line 143: String exitReasonStr = xmlRpcStruct.get(VdsProperties.exit_reason).toString(); Line 144: vmDynamic.setExitReason(VmExitReason.forValue(Integer.parseInt(exitReasonStr))); Line 145: } else { Line 146: vmDynamic.setExitReason(VmExitReason.Unknown); > copy-constructor uses value from DB and we need it from the event. Please t ok, no need for the else-clause at all - since it will not be used with old VDSMs, there is no need to keep backward compatibility here Line 147: } Line 148: Line 149: return new VmInternalData(vmDynamic, dbVm.getStatisticsData(), null, new HashMap<String, LUNs>()); Line 150: } Line 153: if (Long.class.isInstance(value)) { Line 154: return ((Long) value).doubleValue(); Line 155: } Line 156: return null; Line 157: } > I am not sure whether there is any need to use it anywhere else. probably true, it is just dirties this nice class with some generic code that is not specific for it. if you don't find more appropriate place for that, please make it static Line 158: Line 159: @Override Line 160: public void onError(Throwable t) { Line 161: } -- 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: 15 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
