Arik Hadas has uploaded a new change for review.
Change subject: core: mark fields that vdsm cannot change with annotation
......................................................................
core: mark fields that vdsm cannot change with annotation
We used to have the names of the fields in VmDynamic that cannot be
changed by VDSM hard-coded in VdsUpdateRunTimeInfo (so that if we detect
when we compare the VmDynamic we hold in the DB and the VmDynamic we get
from VDSM that only such kind of fields are different, we will not
update the DB).
But holding the field names hard-coded in VdsUpdateRunTimeInfo is
problematic because:
* It means that the fields are renamed, we need to remember to rename
them in VdsUpdateRunTimeInfo as well
* When new fields that should not be change by VDSM are added to
VmDynamic, we tend to forget to add them to the list that resides in
VdsUpdateRunTimeInfo and it cause the number of unneeded writes to the
DB to increase.
The solution is to mark those fields in VmDynamic with an annotation
which indicates that they cannot be changed by VDSM, and then retrieve
all the fields with that annotation in VdsUpdateRunTimeInfo instead of
using hard-coded list of field names.
In addition, the block of code that maps pause status to audit log type
in VdsUpdateRunTimeInfo#updateRepository is extracted to separate method
so make it a bit shorter and more readable.
Change-Id: I8c1981a7adaa6b4ff3ba4012fbe7318f6488e2ab
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
A
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/UnchangeableByVdsm.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDynamic.java
M
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
3 files changed, 77 insertions(+), 40 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/44/22644/15
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/UnchangeableByVdsm.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/UnchangeableByVdsm.java
new file mode 100644
index 0000000..1d0d763
--- /dev/null
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/UnchangeableByVdsm.java
@@ -0,0 +1,16 @@
+package org.ovirt.engine.core.common.businessentities;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation for fields in {@link VmDynamic} that are not expected
+ * to be changed by VDSM.
+ */
+@Target(ElementType.FIELD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface UnchangeableByVdsm {
+
+}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDynamic.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDynamic.java
index 111a545..4ce3603 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDynamic.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDynamic.java
@@ -14,17 +14,22 @@
private VMStatus status;
private String vmIp;
private String vmFQDN;
+ @UnchangeableByVdsm
private String vmHost;
private Integer vmPid;
+ @UnchangeableByVdsm
private Date lastStartTime;
private Date lastStopTime;
private String guestCurUserName;
- private String consoleCurUserName;
+ @UnchangeableByVdsm
+ private String consoleCurrentUserName;
+ @UnchangeableByVdsm
private Guid consoleUserId;
private Date guestLastLoginTime;
private Date guestLastLogoutTime;
private String guestOs;
private Guid migratingToVds;
+ @UnchangeableByVdsm
private Guid runOnVds;
private String appList;
private Integer display;
@@ -35,21 +40,28 @@
private Boolean kvmEnable;
private Integer displaySecurePort;
private Integer utcDiff;
+ @UnchangeableByVdsm
private Guid lastVdsRunOn;
private String clientIp;
private Integer guestRequestedMemory;
+ @UnchangeableByVdsm
private String hibernationVolHandle;
+ @UnchangeableByVdsm
private BootSequence bootSequence;
private VmExitStatus exitStatus;
private VmPauseStatus pauseStatus;
private String hash;
private int guestAgentNicsHash;
- private String mExitMessage;
+ @UnchangeableByVdsm
+ private String exitMessage;
+ @UnchangeableByVdsm
private ArrayList<DiskImageDynamic> disks;
private boolean win2kHackEnabled;
private Long lastWatchdogEvent;
private String lastWatchdogAction;
+ @UnchangeableByVdsm
private boolean runOnce;
+ @UnchangeableByVdsm
private String cpuName;
@Override
@@ -65,7 +77,7 @@
result = prime * result + ((displayIp == null) ? 0 :
displayIp.hashCode());
result = prime * result + ((displaySecurePort == null) ? 0 :
displaySecurePort.hashCode());
result = prime * result + displayType.hashCode();
- result = prime * result + ((consoleCurUserName == null) ? 0 :
consoleCurUserName.hashCode());
+ result = prime * result + ((consoleCurrentUserName == null) ? 0 :
consoleCurrentUserName.hashCode());
result = prime * result + ((guestCurUserName == null) ? 0 :
guestCurUserName.hashCode());
result = prime * result + ((consoleUserId == null) ? 0 :
consoleUserId.hashCode());
result = prime * result + ((guestLastLoginTime == null) ? 0 :
guestLastLoginTime.hashCode());
@@ -76,7 +88,7 @@
result = prime * result + ((kvmEnable == null) ? 0 :
kvmEnable.hashCode());
result = prime * result + ((lastVdsRunOn == null) ? 0 :
lastVdsRunOn.hashCode());
result = prime * result + ((disks == null) ? 0 : disks.hashCode());
- result = prime * result + ((mExitMessage == null) ? 0 :
mExitMessage.hashCode());
+ result = prime * result + ((exitMessage == null) ? 0 :
exitMessage.hashCode());
result = prime * result + exitStatus.hashCode();
result = prime * result + (win2kHackEnabled ? 1231 : 1237);
result = prime * result + ((migratingToVds == null) ? 0 :
migratingToVds.hashCode());
@@ -119,7 +131,7 @@
&& ObjectUtils.objectsEqual(displayIp, other.displayIp)
&& ObjectUtils.objectsEqual(displaySecurePort,
other.displaySecurePort)
&& displayType == other.displayType
- && ObjectUtils.objectsEqual(consoleCurUserName,
other.consoleCurUserName)
+ && ObjectUtils.objectsEqual(consoleCurrentUserName,
other.consoleCurrentUserName)
&& ObjectUtils.objectsEqual(guestCurUserName,
other.guestCurUserName)
&& ObjectUtils.objectsEqual(consoleUserId, other.consoleUserId)
&& ObjectUtils.objectsEqual(guestLastLoginTime,
other.guestLastLoginTime)
@@ -130,7 +142,7 @@
&& ObjectUtils.objectsEqual(kvmEnable, other.kvmEnable)
&& ObjectUtils.objectsEqual(lastVdsRunOn, other.lastVdsRunOn)
&& ObjectUtils.objectsEqual(disks, other.disks)
- && ObjectUtils.objectsEqual(mExitMessage, other.mExitMessage)
+ && ObjectUtils.objectsEqual(exitMessage, other.exitMessage)
&& exitStatus == other.exitStatus
&& win2kHackEnabled == other.win2kHackEnabled
&& ObjectUtils.objectsEqual(migratingToVds,
other.migratingToVds)
@@ -152,11 +164,11 @@
}
public String getExitMessage() {
- return mExitMessage;
+ return exitMessage;
}
public void setExitMessage(String value) {
- mExitMessage = value;
+ exitMessage = value;
}
public VmExitStatus getExitStatus() {
@@ -221,11 +233,11 @@
}
public String getConsoleCurrentUserName() {
- return consoleCurUserName;
+ return consoleCurrentUserName;
}
public void setConsoleCurrentUserName(String consoleCurUserName) {
- this.consoleCurUserName = consoleCurUserName;
+ this.consoleCurrentUserName = consoleCurUserName;
}
public String getGuestCurrentUserName() {
diff --git
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
index 272b516..8284f85 100644
---
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
+++
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
@@ -1,5 +1,6 @@
package org.ovirt.engine.core.vdsbroker;
+import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
@@ -22,8 +23,9 @@
import org.ovirt.engine.core.common.businessentities.DiskImage;
import org.ovirt.engine.core.common.businessentities.DiskImageDynamic;
import org.ovirt.engine.core.common.businessentities.Entities;
-import org.ovirt.engine.core.common.businessentities.MigrationSupport;
import org.ovirt.engine.core.common.businessentities.IVdsEventListener;
+import org.ovirt.engine.core.common.businessentities.MigrationSupport;
+import org.ovirt.engine.core.common.businessentities.UnchangeableByVdsm;
import org.ovirt.engine.core.common.businessentities.OriginType;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VDSGroup;
@@ -123,6 +125,19 @@
private static final int TO_MEGA_BYTES = 1024;
private static final String HOSTED_ENGINE_VM_NAME = "HostedEngine";
private static final String EXTERNAL_VM_NAME_FORMAT = "external-%1$s";
+
+ /** names of fields in {@link VmDynamic} that cannot be changed by VDSM */
+ private static final List<String> UNCHANGEABLE_FIELDS_BY_VDSM;
+
+ static {
+ List<String> tmpList = new ArrayList<String>();
+ for (Field field : VmDynamic.class.getDeclaredFields()) {
+ if (field.isAnnotationPresent(UnchangeableByVdsm.class)) {
+ tmpList.add(field.getName());
+ }
+ }
+ UNCHANGEABLE_FIELDS_BY_VDSM = Collections.unmodifiableList(tmpList);
+ }
private void saveDataToDb() {
if (_saveVdsDynamic) {
@@ -1643,23 +1658,10 @@
else if (runningVm.getStatus() == VMStatus.Paused) {
_vmsToRemoveFromAsync.add(vmToUpdate.getId());
if (vmToUpdate.getStatus() != VMStatus.Paused) {
- // check exit message to determine wht the vm has
- // paused
- AuditLogType logType = AuditLogType.UNASSIGNED;
- AuditLogableBase logable = new
AuditLogableBase(_vds.getId(), vmToUpdate.getId());
- VmPauseStatus pauseStatus =
runningVm.getPauseStatus();
- if (pauseStatus.equals(VmPauseStatus.NOERR) ||
pauseStatus.equals(VmPauseStatus.NONE)) {
- // user requested pause, no log needed
- } else if (pauseStatus == VmPauseStatus.ENOSPC) {
- logType = AuditLogType.VM_PAUSED_ENOSPC;
- } else if (pauseStatus == VmPauseStatus.EIO) {
- logType = AuditLogType.VM_PAUSED_EIO;
- } else if (pauseStatus == VmPauseStatus.EPERM) {
- logType = AuditLogType.VM_PAUSED_EPERM;
- } else {
- logType = AuditLogType.VM_PAUSED_ERROR;
- }
+ // check exit message to determine why the VM is
paused
+ AuditLogType logType =
vmPauseStatusToAuditLogType(runningVm.getPauseStatus());
if (logType != AuditLogType.UNASSIGNED) {
+ AuditLogableBase logable = new
AuditLogableBase(_vds.getId(), vmToUpdate.getId());
auditLog(logable, logType);
}
}
@@ -1696,6 +1698,23 @@
}
// compare between vm in cache and vm from vdsm
removeVmsFromCache(running);
+ }
+
+ private AuditLogType vmPauseStatusToAuditLogType(VmPauseStatus
pauseStatus) {
+ switch (pauseStatus) {
+ case NOERR:
+ case NONE:
+ // user requested pause, no log needed
+ return AuditLogType.UNASSIGNED;
+ case ENOSPC:
+ return AuditLogType.VM_PAUSED_ENOSPC;
+ case EIO:
+ return AuditLogType.VM_PAUSED_EIO;
+ case EPERM:
+ return AuditLogType.VM_PAUSED_EPERM;
+ default:
+ return AuditLogType.VM_PAUSED_ERROR;
+ }
}
private static void logVmStatusTransition(VM vmToUpdate, VmDynamic
runningVm) {
@@ -1882,19 +1901,9 @@
// check if dynamic data changed - update cache and DB
List<String> props = ObjectIdentityChecker.GetChangedFields(
vmToUpdate.argvalue.getDynamicData(), vmNewDynamicData);
- // dont check fields:
- props.remove("vmHost");
- props.remove("runOnVds");
- props.remove("disks");
- props.remove("bootSequence");
- props.remove("lastVdsRunOn");
- props.remove("hibernationVolHandle");
- props.remove("exitMessage");
- props.remove("lastStartTime");
- props.remove("consoleUserId");
- props.remove("consoleCurrentUserName");
- props.remove("runOnce");
- props.remove("cpuName");
+ // remove all fields that should not be checked:
+ props.removeAll(UNCHANGEABLE_FIELDS_BY_VDSM);
+
if (vmNewDynamicData.getStatus() != VMStatus.Up) {
props.remove("appList");
vmNewDynamicData.setAppList(vmToUpdate.argvalue.getAppList());
--
To view, visit http://gerrit.ovirt.org/22644
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c1981a7adaa6b4ff3ba4012fbe7318f6488e2ab
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
