Laszlo Hornyak has posted comments on this change.
Change subject: engine: watchdog - DB and logic changes
......................................................................
Patch Set 5: (17 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetWatchdogQuery.java
Line 8: import org.ovirt.engine.core.common.queries.GetWatchdogParameters;
Line 9: import org.ovirt.engine.core.common.utils.VmDeviceType;
Line 10: import org.ovirt.engine.core.dao.VmDeviceDAO;
Line 11:
Line 12: public class GetWatchdogQuery<P extends GetWatchdogParameters> extends
QueriesCommandBase<P> {
there is GetDevices, but in that case I have to process the device data both in
restapi and GWT
Line 13:
Line 14: public GetWatchdogQuery(P parameters) {
Line 15: super(parameters);
Line 16: }
....................................................
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> {
what monitoring is needed here?
Line 10:
Line 11: public RemoveWatchdogAction(Guid commandId) {
Line 12: super(commandId);
Line 13: }
Line 10:
Line 11: public RemoveWatchdogAction(Guid commandId) {
Line 12: super(commandId);
Line 13: }
Line 14:
Done
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());
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());
that should not lead to an incorrect trigger event notification in case the the
watchdog device is re-introduced, do you still want that cleanup procedure take
place here?
Line 19: }
Line 20:
Line 21: }
Line 22:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateWatchdogCommand.java
Line 15:
Line 16: public UpdateWatchdogCommand(T parameters) {
Line 17: super(parameters);
Line 18: }
Line 19:
Done
Line 20: protected void executeVmCommand() {
Line 21: List<VmDevice> watchdogs =
getVmDeviceDao().getVmDeviceByVmIdAndType(getVmId(),
VmDeviceType.WATCHDOG.getName());
Line 22: if(watchdogs.isEmpty()) {
Line 23: //create new watchdog
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()) {
the list returned should not be null, only empty
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()));
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetWatchdogQueryTest.java
Line 36: Assert.assertNull(query.getReturnValue());
Line 37: }
Line 38:
Line 39: @Test
Line 40: @Ignore
yes remove.
Line 41: public void executeQueryCommandWithWatchdog() {
Line 42: final Guid vmId = new
Guid("ee655a4d-effc-4aab-be2b-2f80ff40cd1c");
Line 43: VmDeviceDAO vmDeviceDaoMock = Mockito.mock(VmDeviceDAO.class);
Line 44: HashMap<String, Object> watchdogSpecParams = new
HashMap<String, Object>();
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateWatchdogCommandTest.java
Line 19: Assert.assertEquals("reset", specParams.get("action"));
Line 20: }
Line 21:
Line 22: @Test
Line 23: public void Perform() {
Done
Line 24:
Line 25: }
....................................................
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;
that's true, but it only introduces constraints here what values do the engine
accept, while what engine really does is that is passes over the values to
libvirt through vdsm.
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/VmDynamic.java
Line 84: result = prime * result + ((vmHost == null) ? 0 :
vmHost.hashCode());
Line 85: result = prime * result + ((vmIp == null) ? 0 :
vmIp.hashCode());
Line 86: result = prime * result + ((lastStartTime == null) ? 0 :
lastStartTime.hashCode());
Line 87: result = prime * result + ((vmPid == null) ? 0 :
vmPid.hashCode());
Line 88: result = prime * result + (lastWatchdogEvent == null ? 0 :
lastWatchdogEvent.hashCode());
well all the above is quite redundant, but let's add that too :)
Line 89: return result;
Line 90: }
Line 91:
Line 92: @Override
....................................................
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() {
The VmWatchdog is constructed from a Device and therefore considered as a more
or less static entity modified only by user interactions and not by Vm state
and events.
This is kind of a confusing part of the engine, but some point of the engine is
written as if there could be any number of watchdogs (especially the rest-api).
libvirt accepts only one watchdog device and similarly vdsm does not tell witch
watchdog was triggered (since there is only one or none at all), so the event
belongs to the VM not to the watchdog card.
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 {
ok, I will look into this, but in this case we have a big pile of parameter
classes to kill
Line 6:
Line 7: private static final long serialVersionUID = -3232978226860645747L;
Line 8:
Line 9: public GetWatchdogParameters(Guid id) {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java
Line 237: GetGlusterHooks,
Line 238:
Line 239: GetDefaultConfigurationVersion(VdcQueryAuthType.User),
Line 240:
Line 241: GetWatchdog,
Done
Line 242:
Line 243: // Default type instead of having to null check
Line 244: Unknown(VdcQueryAuthType.User);
Line 245:
....................................................
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();
ah yes, when this patch started it was XmlRpcStruct, not much difference
but if you look around, still all the code calls that HashMap a 'struct'
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/vdsbroker/VmOldInfoBuilder.java
Line 214: // Not supported in old code
Line 215: }
Line 216:
Line 217: protected void buildVmWatchdog() {
Line 218: throw new NotImplementedException("not implemented");
Done. removed.
Line 219: // Not supported in old code
Line 220: }
....................................................
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)));
true, that was because of the funny format the vdsm sends back
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) {
why not static?
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 <lhorn...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
