Omer Frenkel has posted comments on this change.

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


Patch Set 3:

(5 comments)

partially reviewed

http://gerrit.ovirt.org/#/c/34374/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetConfigurationValueQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetConfigurationValueQuery.java:

Line 25:                 }
Line 26:                 returnValue = Config.<Object> getValue(value, version);
Line 27:             } catch (Exception e) {
Line 28:                 log.error("Unable to return config parameter {}: {}", 
getParameters(), e.getMessage());
Line 29:                 log.debug("Exception", e);
why did you move the exception to debug?
Line 30:             }
Line 31:         }
Line 32: 
Line 33:         getQueryReturnValue().setReturnValue(returnValue);


http://gerrit.ovirt.org/#/c/34374/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java:

Line 413:             log.error("Sql Injection in search: {}", 
getParameters().getSearchPattern());
Line 414:             data = null;
Line 415:         } catch (RuntimeException ex) {
Line 416:             log.warn("Illegal search: {}: {}", 
getParameters().getSearchPattern(), ex.getMessage());
Line 417:             log.debug("Exception", ex);
also here
Line 418:             data = null;
Line 419:         }
Line 420:         return data;
Line 421:     }


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);
also here
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);
you missed to change the order of guid:name to name:guid here (as was done 
below (why is it better?))
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);
same here and the 3 below
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);


-- 
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: 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