Gilad Chaplik has posted comments on this change.
Change subject: engine: watchdog - DB and logic changes
......................................................................
Patch Set 5: (10 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveWatchdogAction.java
Line 5: import org.ovirt.engine.core.common.utils.VmDeviceType;
Line 6: import org.ovirt.engine.core.compat.Guid;
Line 7:
Line 8: @SuppressWarnings("serial")
Line 9: public class RemoveWatchdogAction<T extends VmOperationParameterBase>
extends VmOperationCommandBase<T> {
setActionMessageParameters?
Line 10:
Line 11: public RemoveWatchdogAction(Guid commandId) {
Line 12: super(commandId);
Line 13: }
Line 14:
Line 15: protected void Perform() {
Line 16: for (VmDevice watchdog :
getVmDeviceDao().getVmDeviceByVmIdAndType(getParameters().getVmId(),
Line 17: VmDeviceType.WATCHDOG.getName())) {
Line 18: getVmDeviceDao().remove(watchdog.getId());
Does vdsm clear these fields if watchdog is removed?
Line 19: }
Line 20:
Line 21: }
Line 22:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateWatchdogCommand.java
Line 18: }
Line 19:
Line 20: protected void executeVmCommand() {
Line 21: List<VmDevice> watchdogs =
getVmDeviceDao().getVmDeviceByVmIdAndType(getVmId(),
VmDeviceType.WATCHDOG.getName());
Line 22: if(watchdogs.isEmpty()) {
do we have a test on that? I prefer a null check that will keep me reading the
code, and not checking for getVmDeviceDao().getVmDeviceByVmIdAndType() impl
always returning a value.
Line 23: //create new watchdog
Line 24: VmDevice watchdogDevice = new VmDevice();
Line 25: watchdogDevice.setType(VmDeviceType.WATCHDOG.getName());
Line 26: watchdogDevice.setId(new VmDeviceId(Guid.NewGuid(),
getVmId()));
Line 42: specParams.put("model", getParameters().getModel());
Line 43: return specParams;
Line 44: }
Line 45:
Line 46: protected boolean canDoAction() {
yes and check whether the input params are valid:
does action='gilad' valid?
Line 47: return true;
Line 48: }
Line 49:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/UpdateWatchdogParameters.java
Line 2:
Line 3: public class UpdateWatchdogParameters extends VmOperationParameterBase {
Line 4: private static final long serialVersionUID = 8564973734004518462L;
Line 5: String action;
Line 6: String model;
you can say that on each enum we have in the system... I still think we need it.
Line 7:
Line 8: public String getAction() {
Line 9: return action;
Line 10: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmWatchdog.java
Line 17: Guid id;
Line 18: String action;
Line 19: String model;
Line 20:
Line 21: public Guid getVmId() {
you perfectly explained why you need to keep it in the device: if the vm
doesn't know which devices it has, why it needs to know what is the last_time
and last_action?
the only reason I can think of is performance... am I wrong?
Line 22: return vmId;
Line 23: }
Line 24:
Line 25: public void setVmId(Guid vmId) {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetWatchdogParameters.java
Line 1: package org.ovirt.engine.core.common.queries;
Line 2:
Line 3: import org.ovirt.engine.core.compat.Guid;
Line 4:
Line 5: public class GetWatchdogParameters extends VdcQueryParametersBase {
go for it :)
Line 6:
Line 7: private static final long serialVersionUID = -3232978226860645747L;
Line 8:
Line 9: public GetWatchdogParameters(Guid id) {
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
Line 682: .getVmDeviceDao()
Line 683: .getVmDeviceByVmIdAndType(vm.getId(),
Line 684: VmDeviceType.WATCHDOG.getName());
Line 685: for(VmDevice watchdog : watchdogs) {
Line 686: HashMap struct = new HashMap();
:-)
Line 687: struct.put(VdsProperties.Type, watchdog.getType());
Line 688: struct.put(VdsProperties.Device, watchdog.getDevice());
Line 689: Map<String, Object> specParams = watchdog.getSpecParams();
Line 690: if (specParams == null) {
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
Line 1287: auditLogable.setVmId(vmDynamic.getId());
Line 1288: auditLogable.addCustomValue("wdaction",
vmDynamic.getLastWatchdogAction());
Line 1289: //for the interpretation of vdsm's response see
http://docs.python.org/2/library/time.html
Line 1290: auditLogable.addCustomValue("wdevent",
Line 1291: ObjectUtils.toString(new
Date(vmDynamic.getLastWatchdogEvent().longValue() * 1000)));
what is the format?
Line 1292: AuditLogDirector.log(auditLogable,
AuditLogType.WATCHDOG_EVENT);
Line 1293: }
Line 1294: }
Line 1295: }
Line 1293: }
Line 1294: }
Line 1295: }
Line 1296:
Line 1297: protected static boolean isNewWatchdogEvent(VmDynamic vmDynamic,
VM vmTo) {
don't want to get it to it, if you can find an argument pro static declaration
here, please share it.
Line 1298: Double lastWatchdogEvent = vmDynamic.getLastWatchdogEvent();
Line 1299: return vmTo != null && lastWatchdogEvent != null
Line 1300: && (vmTo.getLastWatchdogEvent() == null ||
vmTo.getLastWatchdogEvent() < lastWatchdogEvent);
Line 1301: }
--
To view, visit http://gerrit.ovirt.org/13057
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I813a6f97e23008d15446285998a4e9b50b456040
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
