Alon Bar-Lev has posted comments on this change. Change subject: core: Replace oVirt logger with slf4j in bll ......................................................................
Patch Set 3: (12 comments) partial http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java: Line 435 Line 436 Line 437 Line 438 Line 439 please dump Line 433: return new String(out.toByteArray(), Charset.forName("UTF-8")); Line 434: } Line 435: catch (Exception e) { Line 436: log.warn( Line 437: "Failed to initiate vdsm-id request on host with message {}", Failed to initiate vdsm-id request on host: {} Line 438: e.getMessage() Line 439: ); Line 440: return null; Line 441: } http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoRecoveryManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoRecoveryManager.java: Line 134: if (fails.size() > 0) { Line 135: final BackendInternal backend = getBackend(); Line 136: log.info("Autorecovering {} {}", fails.size(), logMsg); Line 137: for (final T fail : fails) { Line 138: log.info("Autorecovering {} id: {} {}", logMsg, fail.getId(), getHostName(fail)); I would have switched so message will be at end Line 139: final VdcActionParametersBase actionParams = paramsCallback.doWith(fail); Line 140: actionParams.setShouldBeLogged(true); Line 141: backend.runInternalAction(actionType, actionParams); Line 142: } http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java: Line 154: try { Line 155: dbUp = DbFacade.getInstance().checkDBConnection(); Line 156: } catch (RuntimeException ex) { Line 157: log.error("Error in getting DB connection. The database is inaccessible. " + Line 158: "Original exception is: {}", ExceptionUtils.getMessage(ex)); we should decide if we use ExceptionUtils or not... so far we removed most, if we need to add, then we need another large patch, although trivial. Line 159: try { Line 160: Thread.sleep(waitBetweenInterval); Line 161: } catch (InterruptedException e) { Line 162: log.warn("Failed to wait between connection polling attempts. " + Line 342: try { Line 343: cmd.compensate(); Line 344: } catch (RuntimeException e) { Line 345: log.error( Line 346: "Failed to run compensation on startup for Command '{}', Command Id '{}', due to: {}", no need ", due to" Line 347: commandSnapshot.getValue(), Line 348: commandSnapshot.getKey(), Line 349: ExceptionUtils.getMessage(e)); Line 350: log.debug("Exception", e); http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java: Line 931 Line 932 Line 933 Line 934 Line 935 groupIds no need fancy format in debug Line 1268: } Line 1269: } Line 1270: Line 1271: // Log the final appended message to the log. Line 1272: log.info(logInfo.toString()); "{}", logInfo Line 1273: } Line 1274: Line 1275: private StringBuilder getPermissionSubjectsAsStringBuilder(List<PermissionSubject> permissionSubjects) { Line 1276: StringBuilder builder = new StringBuilder(); http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandEntityCleanupManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandEntityCleanupManager.java: Line 30: Line 31: String cronExpression = String.format("%d %d %d * * ?", calendar.get(Calendar.SECOND), Line 32: calendar.get(Calendar.MINUTE), calendar.get(Calendar.HOUR_OF_DAY)); Line 33: Line 34: log.info("Setting command entity cleanup manager to run at: {}" + cronExpression); , Line 35: SchedulerUtilQuartzImpl.getInstance().scheduleACronJob(this, "onTimer", new Class[] {}, new Object[] {}, Line 36: cronExpression); Line 37: log.info("Finished initializing {}", getClass().getSimpleName()); Line 38: } http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExtendImageSizeCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExtendImageSizeCommand.java: Line 90: if (!ret.getSucceeded()) { Line 91: updateAuditLogFailedToUpdateVM(vm.getName()); Line 92: } Line 93: } catch (VdcBLLException e) { Line 94: log.warn("Failed to update VM '{}' with the new volume size due to error: {}." + , Line 95: "VM should be restarted to detect the new size.", Line 96: vm.getName(), ExceptionUtils.getMessage(e)); Line 97: updateAuditLogFailedToUpdateVM(vm.getName()); Line 98: } http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceExecutor.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceExecutor.java: Line 133 Line 134 Line 135 Line 136 Line 137 "Exception" http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportRepoImageCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportRepoImageCommand.java: Line 230 Line 231 Line 232 Line 233 Line 234 switch order? http://gerrit.ovirt.org/#/c/34359/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java: Line 101: vmFromConfiguration.setDiskMap(getParameters().getDiskMap()); Line 102: vmFromConfiguration.setImages(getDiskImageListFromDiskMap(getParameters().getDiskMap())); Line 103: } Line 104: } catch (OvfReaderException e) { Line 105: log.error("failed to parse a given ovf configuration: \n{}", ovfEntityData.getOvfData()); no need space before new line can we avoid new line in this case? Line 106: log.error("Exception", e); Line 107: } Line 108: } Line 109: } -- To view, visit http://gerrit.ovirt.org/34359 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82070b252197458422792c49b2135fcb9b9b1430 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Roy Golan <[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
