Arik Hadas has posted comments on this change.

Change subject: events: removing vm.getStats from event processing
......................................................................


Patch Set 15:

(5 comments)

partial review

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:                     }
you can call map.remove instead of map.get in line 78 and drop this code
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);
why to use vdsmVm if vmData contains the same status. looks like this line (90) 
and createVmInternal can be removed
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:                 }
no exception can be thrown from VmDynamic#setHash
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);
how about putting it in the copy-constructor instead?
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:             }
should move to some utility class
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

Reply via email to