Alon Bar-Lev has posted comments on this change. Change subject: core: remove ovirt logger from vdsbroker and friends ......................................................................
Patch Set 3: (8 comments) http://gerrit.ovirt.org/#/c/34374/3/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/CreateVmVDSCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/CreateVmVDSCommand.java: Line 60: ResourceManager.getInstance().RemoveAsyncRunningVm(getParameters().getVmId()); Line 61: } Line 62: } catch (Exception e) { Line 63: log.error("Error in excuting CreateVmVDSCommand: {}", e.getMessage()); Line 64: log.debug("Exception", e); > please log exception as error Done Line 65: if (command != null && !command.getVDSReturnValue().getSucceeded()) { Line 66: ResourceManager.getInstance().RemoveAsyncRunningVm(getParameters().getVmId()); Line 67: } Line 68: throw new RuntimeException(e); Line 96: Guid guid = getParameters().getVm().getId(); Line 97: String vmName = getParameters().getVm().getName(); Line 98: VmDynamic vmDynamicFromDb = DbFacade.getInstance().getVmDynamicDao().get(guid); Line 99: if (ResourceManager.getInstance().IsVmDuringInitiating(getParameters().getVm().getId())) { Line 100: log.info("Vm Running failed - vm {}:{} already running", guid, vmName); > thats ok, so please switch above as well to be name and then guid, so it wi Done Line 101: getVDSReturnValue().setReturnValue(vmDynamicFromDb.getStatus()); Line 102: return false; Line 103: } else { Line 104: VMStatus vmStatus = vmDynamicFromDb.getStatus(); http://gerrit.ovirt.org/#/c/34374/3/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java: Line 382: } Line 383: } catch (Exception e) { Line 384: if (e.getCause() != null) { Line 385: log.error("CreateCommand failed: {}", e.getCause().getMessage()); Line 386: log.debug("Exception", e); > please log exceptions as errors here and 3 below as well Done Line 387: throw new RuntimeException(e.getCause().getMessage(), e.getCause()); Line 388: } Line 389: log.error("CreateCommand failed: {}", e.getMessage()); Line 390: log.debug("Exception", e); http://gerrit.ovirt.org/#/c/34374/3/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 264: "Failed to refresh VDS, continuing, vds ='{}' ('{}'): {}", Line 265: vds.getName(), Line 266: vds.getId(), Line 267: ex.getMessage()); Line 268: log.debug("Exception", ex); > please log exception as error Done Line 269: } Line 270: Line 271: private static void logException(final RuntimeException ex) { Line 272: log.error("ResourceManager::refreshVdsRunTimeInfo", ex); http://gerrit.ovirt.org/#/c/34374/3/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 226: new SetVdsStatusVDSCommandParameters(_vds.getId(), VDSStatus.Error)); Line 227: } Line 228: } catch (Throwable t) { Line 229: log.error("Failure to refresh Vds runtime info: {}", t.getMessage()); Line 230: log.debug("Exception", t); > i think its better to keep this in error Done Line 231: throw t; Line 232: } Line 233: moveVDSToMaintenanceIfNeeded(); Line 234: } Line 1273: logicalName = getDeviceLogicalName((Map<?, ?>) vm.get(VdsProperties.GuestDiskMapping), deviceId); Line 1274: } catch (Exception e) { Line 1275: log.error("error while getting device name when processing, vm '{}', device info '{}' with exception, skipping: {}", Line 1276: vmId, device, e.getMessage()); Line 1277: log.debug("Exception", e); > please log exception as error Done Line 1278: } Line 1279: } Line 1280: Line 1281: if (deviceId == null || vmDevice == null) { http://gerrit.ovirt.org/#/c/34374/3/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/FutureVDSCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/FutureVDSCommand.java: Line 75: httpTask.cancel(true); Line 76: VDSNetworkException ex = new VDSNetworkException("Timeout during xml-rpc call"); Line 77: ex.setVdsError(new VDSError(VdcBllErrors.VDS_NETWORK_ERROR, "Timeout during xml-rpc call")); Line 78: setVdsRuntimeError(ex); Line 79: log.error("Timeout waiting for VDSM response. " + e); > actually, here we might be good with exception message only in error, and s Done Line 80: throw e; Line 81: } catch (Exception e) { Line 82: log.error("Error: {}", e.getMessage()); Line 83: log.debug("Exception", e); Line 79: log.error("Timeout waiting for VDSM response. " + e); Line 80: throw e; Line 81: } catch (Exception e) { Line 82: log.error("Error: {}", e.getMessage()); Line 83: log.debug("Exception", e); > please use error Done Line 84: setVdsRuntimeError(e instanceof RuntimeException ? (RuntimeException) e : new RuntimeException(e)); Line 85: } Line 86: return getVDSReturnValue(); Line 87: } -- 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: 3 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
