Omer Frenkel has uploaded a new change for review. Change subject: core: handle hot-set and update running vm ......................................................................
core: handle hot-set and update running vm since the numOfSockets field was updateable, the infrastructure of update-running-vm didn't take it into account, when this was the only changed field, we had couple of issue: 1. the UI dialog didn't show, user cant choose if to apply now or later 2. next_run snapshot wasn't created this fix is changine the field to updateableOnStatus, to handle above issues, but since the user can choose if to use hot plug or not, a new field introduced to the annotation, to mark fields that can be hot-set. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1128443 Change-Id: I1c7f9b26ac4ddda6b446244b1fd85e2023809630 Signed-off-by: Omer Frenkel <ofren...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableOnVmStatusField.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java M backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ObjectIdentityCheckerTest.java 6 files changed, 71 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/99/32399/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java index 379dea2..24eaa6a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java @@ -128,6 +128,7 @@ // save user selected value for hotplug before overriding with db values (when updating running vm) int cpuPerSocket = newVmStatic.getCpuPerSocket(); + int numOfSockets = newVmStatic.getNumOfSockets(); if (newVmStatic.getCreationDate().equals(DateTime.getMinValue())) { newVmStatic.setCreationDate(new Date()); @@ -143,7 +144,7 @@ UpdateVmNetworks(); if (!getParameters().isApplyChangesLater()) { - hotSetCpus(cpuPerSocket); + hotSetCpus(cpuPerSocket, numOfSockets); } getVmStaticDAO().update(newVmStatic); if (getVm().isNotRunning()) { @@ -219,23 +220,22 @@ return DbFacade.getInstance().getSnapshotDao(); } - private void hotSetCpus(int cpuPerSocket) { + private void hotSetCpus(int cpuPerSocket, int newNumOfSockets) { int currentSockets = getVm().getNumOfSockets(); - int newSockets = newVmStatic.getNumOfSockets(); int currentCpuPerSocket = getVm().getCpuPerSocket(); // try hotplug only if topology (cpuPerSocket) hasn't changed - if (getVm().getStatus() == VMStatus.Up && currentSockets != newSockets + if (getVm().getStatus() == VMStatus.Up && currentSockets != newNumOfSockets && currentCpuPerSocket == cpuPerSocket) { HotSetNumerOfCpusParameters params = new HotSetNumerOfCpusParameters( newVmStatic, - currentSockets < newSockets ? PlugAction.PLUG : PlugAction.UNPLUG); + currentSockets < newNumOfSockets ? PlugAction.PLUG : PlugAction.UNPLUG); setNumberOfCpusResult = runInternalAction( VdcActionType.HotSetNumberOfCpus, params, cloneContextAndDetachFromParent()); - newVmStatic.setNumOfSockets(setNumberOfCpusResult.getSucceeded() ? newSockets : currentSockets); + newVmStatic.setNumOfSockets(setNumberOfCpusResult.getSucceeded() ? newNumOfSockets : currentSockets); auditLogHotSetCpusCandos(params); } } @@ -595,7 +595,8 @@ return getVm().isNextRunConfigurationExists() || !VmHandler.isUpdateValid(getVm().getStaticData(), getParameters().getVmStaticData(), - getVm().getStatus()) || + getVm().getStatus(), + !getParameters().isApplyChangesLater()) || !VmHandler.isUpdateValidForVmDevices(getVmId(), getVm().getStatus(), getParameters()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java index 520e780..acf6b49 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java @@ -113,6 +113,9 @@ for (Pair<EditableOnVmStatusField, Field> pair : BaseHandler.extractAnnotatedFields(EditableOnVmStatusField.class, inspectedClassNames)) { mUpdateVmsStatic.AddField(Arrays.asList(pair.getFirst().statuses()), pair.getSecond().getName()); + if (pair.getFirst().isHotsetAllowed()) { + mUpdateVmsStatic.AddHotsetFields(pair.getSecond().getName()); + } } for (Pair<EditableDeviceOnVmStatusField, Field> pair : BaseHandler.extractAnnotatedFields(EditableDeviceOnVmStatusField.class, @@ -142,6 +145,10 @@ return mUpdateVmsStatic.IsUpdateValid(source, destination, status); } + public static boolean isUpdateValid(VmStatic source, VmStatic destination, VMStatus status, boolean hotsetEnabled) { + return mUpdateVmsStatic.IsUpdateValid(source, destination, status, hotsetEnabled); + } + public static boolean isUpdateValid(VmStatic source, VmStatic destination) { return mUpdateVmsStatic.IsUpdateValid(source, destination); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableOnVmStatusField.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableOnVmStatusField.java index e08bfd2..ed7d7a7 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableOnVmStatusField.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableOnVmStatusField.java @@ -11,4 +11,5 @@ VMStatus[] statuses() default { VMStatus.Down }; + boolean isHotsetAllowed() default false; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java index f6afc3c..1a1a621 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java @@ -72,7 +72,8 @@ @EditableOnTemplate private int memSizeMb; - @EditableField + @EditableOnVmStatusField(isHotsetAllowed = true) + @EditableOnTemplate @CopyOnNewVersion private int numOfSockets; diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java index a58ff8e..d032d36 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java @@ -24,6 +24,7 @@ private Map<Enum<?>, Set<String>> dictionary = new HashMap<Enum<?>, Set<String>>(); private Set<String> permitted = new HashSet<String>(); + private Set<String> hotsetAllowedFields = new HashSet<String>(); public ObjectIdentityChecker(Class<?> type) { identities.put(type, this); @@ -83,11 +84,21 @@ } } + public final void AddHotsetFields(String... fieldNames) { + for (String fieldName : fieldNames) { + hotsetAllowedFields.add(fieldName); + } + } + public final boolean IsFieldUpdatable(String name) { return permitted.contains(name); } public boolean IsFieldUpdatable(Enum<?> status, String name, Object fieldContainer) { + return IsFieldUpdatable(status, name, fieldContainer, false); + } + + public boolean IsFieldUpdatable(Enum<?> status, String name, Object fieldContainer, boolean hotsetEnabled) { boolean returnValue = true; if (!IsFieldUpdatable(name)) { if (fieldContainer != null && container != null @@ -96,6 +107,11 @@ } else { Set<String> values = dictionary.get(status); returnValue = values != null ? values.contains(name) : false; + + // if field is not updateable in this status, check if hotset request and its an hotset allowed field + if (!returnValue && hotsetEnabled) { + returnValue = hotsetAllowedFields.contains(name); + } } if (!returnValue) { LogError(name, status); @@ -147,11 +163,15 @@ } public final boolean IsUpdateValid(Object source, Object destination, Enum<?> status) { + return IsUpdateValid(source, destination, status, false); + } + + public final boolean IsUpdateValid(Object source, Object destination, Enum<?> status, boolean hotsetEnabled) { if (source.getClass() != destination.getClass()) { return false; } for (String fieldName : GetChangedFields(source, destination)) { - if (!IsFieldUpdatable(status, fieldName, null)) { + if (!IsFieldUpdatable(status, fieldName, null, hotsetEnabled)) { log.warn(String.format("ObjectIdentityChecker.IsUpdateValid:: Not updatable field '%1$s' was updated", fieldName)); return false; diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ObjectIdentityCheckerTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ObjectIdentityCheckerTest.java index cdd01b6..0bb62a3 100644 --- a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ObjectIdentityCheckerTest.java +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ObjectIdentityCheckerTest.java @@ -7,6 +7,7 @@ import java.util.List; import org.junit.Test; +import org.ovirt.engine.core.common.businessentities.VMStatus; public class ObjectIdentityCheckerTest { @Test @@ -48,4 +49,35 @@ changed = oic.IsFieldsUpdated(jedi1, jedi2, Arrays.asList("saberColor")); assertTrue("1 Change", changed); } + + @Test + public void testHotsetUpdateableWhenHotsetRequested() { + ObjectIdentityChecker oic = new ObjectIdentityChecker(Jedi.class); + oic.AddHotsetFields("name"); + assertTrue("hot set requested for hot set fields should be true", oic.IsFieldUpdatable(null, "name", null, true)); + } + + @Test + public void testHotsetNotUpdateableWhenHotsetNotRequested() { + ObjectIdentityChecker oic = new ObjectIdentityChecker(Jedi.class); + assertFalse("Should be false by default", oic.IsFieldUpdatable("name")); + oic.AddHotsetFields("name"); + assertFalse("hot set not requested should return false even if field is hot set", oic.IsFieldUpdatable(null, "name", null, false)); + } + + @Test + public void testHotsetUpdateableWhenHotsetRequestedWithStatus() { + ObjectIdentityChecker oic = new ObjectIdentityChecker(Jedi.class); + oic.AddField(VMStatus.Down, "name"); + oic.AddHotsetFields("name"); + assertTrue("hot set requested for hot set fields should be true", oic.IsFieldUpdatable(VMStatus.Down, "name", null, true)); + } + + @Test + public void testHotsetUpdateableWhenHotsetNotRequestedWithStatus() { + ObjectIdentityChecker oic = new ObjectIdentityChecker(Jedi.class); + oic.AddField(VMStatus.Down, "name"); + oic.AddHotsetFields("name"); + assertTrue("hot set not requested field should be updateable according to status", oic.IsFieldUpdatable(VMStatus.Down, "name", null, false)); + } } -- To view, visit http://gerrit.ovirt.org/32399 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1c7f9b26ac4ddda6b446244b1fd85e2023809630 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Omer Frenkel <ofren...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
