Moti Asayag has posted comments on this change.

Change subject: core: remove ovirt logger from vdsbroker and friends
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.ovirt.org/#/c/34374/4/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java:

Line 174:                 // save all data to db
Line 175:                 saveDataToDb();
Line 176:             } catch (IRSErrorException ex) {
Line 177:                 
logFailureMessage("ResourceManager::refreshVdsRunTimeInfo:", ex);
Line 178:                 if (log.isDebugEnabled()) {
why does the surrounding if is required ?
could be just 
  log.debug("Exception", ex);
Line 179:                     log.error("Exception", ex);
Line 180:                 }
Line 181:             } catch (RuntimeException ex) {
Line 182:                 
logFailureMessage("ResourceManager::refreshVdsRunTimeInfo:", ex);


Line 804:             problematicNicsWithNetworks = 
NetworkMonitoringHelper.determineProblematicNics(_vds.getInterfaces(),
Line 805:                     
getDbFacade().getNetworkDao().getAllForCluster(_vds.getVdsGroupId()));
Line 806:         } catch (Exception e) {
Line 807:             log.error("Failure on checkInterfaces on update 
runtimeinfo for vds: '{}': {}", _vds.getName(), e.getMessage());
Line 808:             log.debug("Exception", e);
this situation might happen only if there is a bug in the code. No plan user 
flow should reach this catch block, hence please maintain the error severity 
level of code, or just add it as the last argument of line 807.
Line 809:         } finally {
Line 810:             if (!problematicNicsWithNetworks.isEmpty()) {
Line 811:                 // we give 1 minutes to a nic to get up in case the 
nic get the ip from DHCP server
Line 812:                 if (!hostDownTimes.containsKey(_vds.getId())) {


Line 840:                     auditLog(logable, 
AuditLogType.VDS_SET_NONOPERATIONAL_IFACE_DOWN);
Line 841:                 } catch (Exception e) {
Line 842:                     log.error("checkInterface: Failure on moving 
host: '{}' to non-operational: {}",
Line 843:                             _vds.getName(), e.getMessage());
Line 844:                     log.debug("Exception", e);
same here
Line 845:                 }
Line 846:             } else {
Line 847:                 // no nics are down, remove from list if exists
Line 848:                 hostDownTimes.remove(_vds.getId());


-- 
To view, visit http://gerrit.ovirt.org/34374
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae367e2c2fe3df862c8a345ca73575198f4c6edd
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Martin PeÅ™ina <[email protected]>
Gerrit-Reviewer: Moti Asayag <[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: Tal Nisan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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

Reply via email to