Omer Frenkel has posted comments on this change.

Change subject: events: VM Status based on an event
......................................................................


Patch Set 19:

(3 comments)

https://gerrit.ovirt.org/#/c/37488/19/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/RefresherFactory.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/RefresherFactory.java:

Line 12: public class RefresherFactory {
Line 13: 
Line 14:     public static VMStatsRefresher create(final VdsManager manager, 
final AuditLogDirector auditLogDirector, final SchedulerUtil scheduler) {
Line 15:         Version version = manager.getCompatibilityVersion();
Line 16:         if (FeatureSupported.jsonProtocol(version) && 
FeatureSupported.vmStatsEvents(version)) {
can it be that the current host has json disabled?
(i see that i can disable this in the edit host dialog)
should we check for that as well? (or instead if json is enabled in the cluster)
Line 17:             return new EventVMStatsRefresher(manager, 
auditLogDirector, scheduler);
Line 18:         }
Line 19:         return new PollVMStatsRefresher(manager, auditLogDirector, 
scheduler);
Line 20:     }


https://gerrit.ovirt.org/#/c/37488/19/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 58:                 try {
Line 59:                     List<Pair<VM, VmInternalData>> changedVms = new 
ArrayList<>();
Line 60:                     List<Pair<VM, VmInternalData>> devicesChangedVms = 
new ArrayList<>();
Line 61: 
Line 62:                     List<VmInternalData> vms = convertEvent(map);
please rename to vdsmVms
Line 63:                     prepareChanges(vms, changedVms, devicesChangedVms);
Line 64: 
Line 65:                     new VmsMonitoring(manager, changedVms, 
devicesChangedVms, auditLogDirector).perform();
Line 66:                 } finally {


Line 89:                     VmInternalData vmData = fetchStats(dbVm, vdsmVm);
Line 90:                     if (vmData != null) {
Line 91:                         changedVms.add(new Pair<>(dbVm, vmData));
Line 92:                     }
Line 93:                     if (isDevicesChanged(dbVm, vdsmVm)) {
i guess this should be part of the upper 'if' and should get vmData which has 
the hash from vdsm..
Line 94:                         devicesChangedVms.add(new Pair<>(dbVm, 
vdsmVm));
Line 95:                     }
Line 96:                 }
Line 97:             }


-- 
To view, visit https://gerrit.ovirt.org/37488
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b35877ccd63372759ad6989280e9417c259b21
Gerrit-PatchSet: 19
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: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@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-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to