Roy Golan has posted comments on this change. Change subject: core: VM Monitoring abstract fetching/analyzing/monitoring ......................................................................
Patch Set 19: (3 comments) http://gerrit.ovirt.org/#/c/28662/19/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java: Line 287: ).perform(); Line 288: } Line 289: } Line 290: Line 291: public VDS getVds() { > can you please explain why this is needed? i thought you are going to delet just keeping the cacheVds immutable by external usages. the intention is to hand a copy of vds to the ListFetcher because it is required by VdsBroker commands. I'm not sure those commands won't mess with it. will change the method name and add javadoc Line 292: //TODO use deep clone for defensive copying. Make VDS implement Clonable Line 293: return cachedVds.clone(); Line 294: } Line 295: public String getVdsName() { Line 305: } Line 306: Line 307: private void logFailureMessage(RuntimeException ex) { Line 308: log.warn( Line 309: "Failed to refresh VDS , cachedVds = '{}' : '{}', error = '{}', continuing.", > please change message to use "vds" and not "cachedVds" Done Line 310: cachedVds.getName(), Line 311: cachedVds.getId()); Line 312: log.error("Exception", ex); Line 313: } Line 307: private void logFailureMessage(RuntimeException ex) { Line 308: log.warn( Line 309: "Failed to refresh VDS , cachedVds = '{}' : '{}', error = '{}', continuing.", Line 310: cachedVds.getName(), Line 311: cachedVds.getId()); > you are missing the ex.getMsg Done Line 312: log.error("Exception", ex); Line 313: } Line 314: Line 315: private static void logException(final RuntimeException ex) { -- To view, visit http://gerrit.ovirt.org/28662 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1adf0a95007140e89b080b5160ba93e340ee3ba6 Gerrit-PatchSet: 19 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
