Omer Frenkel has posted comments on this change. Change subject: engine: fetch exit reason from VDSM ......................................................................
Patch Set 8: (3 comments) http://gerrit.ovirt.org/#/c/23946/8/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDynamic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDynamic.java: Line 66: private boolean runOnce; Line 67: @UnchangeableByVdsm Line 68: private String cpuName; Line 69: private String currentCd; Line 70: @UnchangeableByVdsm why this is declared as un-changealble? if we get it from vdsm this annotation shouldnt be here Line 71: private VmExitReason exitReason; Line 72: Line 73: public static final String APPLICATIONS_LIST_FIELD_NAME = "appList"; Line 74: public static final String STATUS_FIELD_NAME = "status"; http://gerrit.ovirt.org/#/c/23946/8/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 279: * @param vm Line 280: * @param status Line 281: */ Line 282: public void InternalSetVmStatus(VM vm, final VMStatus status) { Line 283: log.infoFormat("vm {0} DOWN exit reason generic", vm.getId()); this log is not correct, this method is called also for other statuses. please also look at my comment regarding the next log here Line 284: InternalSetVmStatus(vm, status, VmExitStatus.Normal, StringUtils.EMPTY, VmExitReason.Unknown); Line 285: } Line 286: Line 287: public void InternalSetVmStatus(VM vm, final VMStatus status, final VmExitStatus exitStaus, final String exitMessage, final VmExitReason exitReason) { Line 298: vm.setRunOnVds(null); Line 299: vm.setVmPauseStatus(VmPauseStatus.NONE); Line 300: vm.setLastStopTime(new Date()); Line 301: Line 302: log.infoFormat("vm {0} DOWN exit reason : {1}", vm.getId(), vm.getExitReason()); not sure we should log here, i think the logging is the caller responsibillity, we should already have log for vm going to down today Line 303: } Line 304: } Line 305: } Line 306: -- To view, visit http://gerrit.ovirt.org/23946 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib36f06f3ba63a743d624e417fe2c96b84dda60be Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Omer Frenkel <[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
