Daniel Erez has uploaded a new change for review. Change subject: core: infrastructure for VmDevices in next run VM snapshot ......................................................................
core: infrastructure for VmDevices in next run VM snapshot Added an infrastructure for including VmDevices in a snapshot of type 'Next Run'. This should allow enabling/disabling of devices while VM is running (i.e. similarly to VM properties that are editable on a non-running VM, a warning message [1] is being shown upon editing a VM device status). * Created a new annotation interface for devices: 'EditableDeviceOnVmStatusField'. * VmHandler - initialized mUpdateVmsStatic with those fields. * UpdateVmCommand - passed a list of devices for next run when creating the snapshot with new configuration. * GetVmUpdatesOnNextRunExistsParameters now gets an instance of VmManagementParametersBase in ctr [since the new annotation (EditableDeviceOnVmStatusField) will be used in this parameters file]. * GetVmUpdatesOnNextRunExistsQuery - added a validation for updating VM devices. * Modified client invocations accordingly. [1] "Some of the changes will be applied after the NEXT RESTART" Change-Id: I4e7add4e7e09fe8d6d1f93f0d2b96795d838efa2 Bug-Url: https://bugzilla.redhat.com/1035289 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmUpdatesOnNextRunExistsQuery.java 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/VdsHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java A backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableDeviceOnVmStatusField.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmUpdatesOnNextRunExistsParameters.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java 15 files changed, 289 insertions(+), 104 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/43/30043/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmUpdatesOnNextRunExistsQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmUpdatesOnNextRunExistsQuery.java index 9e7063e..7523604 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmUpdatesOnNextRunExistsQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmUpdatesOnNextRunExistsQuery.java @@ -34,6 +34,7 @@ vmPropertiesUtils.separateCustomPropertiesToUserAndPredefined( dstVm.getVdsGroupCompatibilityVersion(), dstStatic); - setReturnValue(!VmHandler.isUpdateValid(srcStatic, dstStatic, VMStatus.Up)); + setReturnValue(!VmHandler.isUpdateValid(srcStatic, dstStatic, VMStatus.Up) || + !VmHandler.isUpdateValidForVmDevices(srcVm.getId(), getParameters().getUpdateVmParameters())); } } 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 11e8f95..df3b003 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 @@ -198,6 +198,7 @@ VM vm = new VM(); vm.setStaticData(getParameters().getVmStaticData()); + // create new snapshot with new configuration new SnapshotsManager().addSnapshot(Guid.newGuid(), "Next Run configuration snapshot", @@ -207,6 +208,7 @@ true, StringUtils.EMPTY, Collections.EMPTY_LIST, + VmDeviceUtils.getVmDevicesForNextRun(getVm(), getParameters()), getCompensationContext()); } @@ -522,12 +524,12 @@ } if (getParameters().isConsoleEnabled() != null && !getVm().isDown() - && vmDeviceChanged(VmDeviceGeneralType.CONSOLE, getParameters().isConsoleEnabled())) { + && vmDeviceChanged(getVmId(), VmDeviceGeneralType.CONSOLE, getParameters().isConsoleEnabled())) { return failCanDoAction(VdcBllMessages.VM_CANNOT_UPDATE_DEVICE_VM_NOT_DOWN, "$device console"); } if (getParameters().isSoundDeviceEnabled() != null && !getVm().isDown() - && vmDeviceChanged(VmDeviceGeneralType.SOUND, getParameters().isSoundDeviceEnabled())) { + && vmDeviceChanged(getVmId(), VmDeviceGeneralType.SOUND, getParameters().isSoundDeviceEnabled())) { return failCanDoAction(VdcBllMessages.VM_CANNOT_UPDATE_DEVICE_VM_NOT_DOWN, "$device sound"); } @@ -559,7 +561,7 @@ if (getParameters().isVirtioScsiEnabled() != null && !getVm().isDown() - && vmDeviceChanged(VmDeviceGeneralType.CONTROLLER, + && vmDeviceChanged(getVmId(), VmDeviceGeneralType.CONTROLLER, VmDeviceType.VIRTIOSCSI.getName(), getParameters().isVirtioScsiEnabled())) { return failCanDoAction(VdcBllMessages.VM_CANNOT_UPDATE_DEVICE_VM_NOT_DOWN, "$device VirtIO-SCSI"); @@ -589,27 +591,6 @@ getReturnValue().getCanDoActionMessages()); } - private boolean vmDeviceChanged(VmDeviceGeneralType deviceType, boolean deviceEnabled) { - List<VmDevice> vmDevices = getVmDeviceDao().getVmDeviceByVmIdAndType(getParameters().getVmId(), - deviceType); - - return deviceEnabled == vmDevices.isEmpty(); - } - - /** - * Determines whether a VM device change has been request by the user. - * @param deviceType VmDeviceGeneralType. - * @param device VmDeviceType name. - * @param deviceEnabled indicates whether the user asked to enable the device. - * @return true if a change has been requested; otherwise, false - */ - private boolean vmDeviceChanged(VmDeviceGeneralType deviceType, String device, boolean deviceEnabled) { - List<VmDevice> vmDevices = getVmDeviceDao().getVmDeviceByVmIdTypeAndDevice(getParameters().getVmId(), - deviceType, device); - - return deviceEnabled == vmDevices.isEmpty(); - } - private boolean isVmExist() { return getParameters().getVmStaticData() != null && getVm() != null; } @@ -628,8 +609,9 @@ private boolean isRunningConfigurationNeeded() { return getVm().isNextRunConfigurationExists() || !VmHandler.isUpdateValid(getVm().getStaticData(), - getParameters().getVmStaticData(), - getVm().getStatus()); + getParameters().getVmStaticData(), + getVm().getStatus()) || + !VmHandler.isUpdateValidForVmDevices(getVmId(), getParameters()); } @Override @@ -779,4 +761,11 @@ return getParameters().getWatchdog() != null; } + protected boolean vmDeviceChanged(Guid vmId, VmDeviceGeneralType deviceType, boolean deviceEnabled) { + return VmDeviceUtils.vmDeviceChanged(vmId, deviceType, deviceEnabled); + } + + protected boolean vmDeviceChanged(Guid vmId, VmDeviceGeneralType deviceType, String device, boolean deviceEnabled) { + return VmDeviceUtils.vmDeviceChanged(vmId, deviceType, device, deviceEnabled); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsHandler.java index 7c8c0ba..3502838 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsHandler.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.bll; +import java.lang.reflect.Field; import java.util.Arrays; import org.apache.commons.lang.StringUtils; @@ -37,12 +38,12 @@ new ObjectIdentityChecker(VdsHandler.class, Arrays.asList(inspectedClasses)); - for (Pair<EditableField, String> pair : extractAnnotatedFields(EditableField.class, inspectedClasses)) { - mUpdateVdsStatic.AddPermittedFields(pair.getSecond()); + for (Pair<EditableField, Field> pair : extractAnnotatedFields(EditableField.class, inspectedClasses)) { + mUpdateVdsStatic.AddPermittedFields(pair.getSecond().getName()); } - for (Pair<EditableOnVdsStatus, String> pair : extractAnnotatedFields(EditableOnVdsStatus.class, inspectedClasses)) { - mUpdateVdsStatic.AddField(Arrays.asList(pair.getFirst().statuses()), pair.getSecond()); + for (Pair<EditableOnVdsStatus, Field> pair : extractAnnotatedFields(EditableOnVdsStatus.class, inspectedClasses)) { + mUpdateVdsStatic.AddField(Arrays.asList(pair.getFirst().statuses()), pair.getSecond().getName()); } } 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 808e288..d560ad7 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 @@ -1,5 +1,7 @@ package org.ovirt.engine.core.bll; +import java.lang.reflect.Field; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -12,17 +14,20 @@ import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.context.CompensationContext; import org.ovirt.engine.core.bll.network.MacPoolManager; +import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.bll.validator.VmValidationUtils; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.backendinterfaces.BaseHandler; import org.ovirt.engine.core.common.businessentities.ArchitectureType; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.DisplayType; +import org.ovirt.engine.core.common.businessentities.EditableDeviceOnVmStatusField; import org.ovirt.engine.core.common.businessentities.EditableField; import org.ovirt.engine.core.common.businessentities.EditableOnVm; import org.ovirt.engine.core.common.businessentities.EditableOnVmStatusField; @@ -85,26 +90,33 @@ VmBase.class, VM.class, VmStatic.class, - VmDynamic.class }; + VmDynamic.class, + VmManagementParametersBase.class }; osRepository = SimpleDependecyInjector.getInstance().get(OsRepository.class); mUpdateVmsStatic = new ObjectIdentityChecker(VmHandler.class, Arrays.asList(inspectedClassNames)); - for (Pair<EditableField, String> pair : BaseHandler.extractAnnotatedFields(EditableField.class, + for (Pair<EditableField, Field> pair : BaseHandler.extractAnnotatedFields(EditableField.class, (inspectedClassNames))) { - mUpdateVmsStatic.AddPermittedFields(pair.getSecond()); + mUpdateVmsStatic.AddPermittedFields(pair.getSecond().getName()); } - for (Pair<EditableOnVm, String> pair : BaseHandler.extractAnnotatedFields(EditableOnVm.class, inspectedClassNames)) { - mUpdateVmsStatic.AddPermittedFields(pair.getSecond()); + for (Pair<EditableOnVm, Field> pair : BaseHandler.extractAnnotatedFields(EditableOnVm.class, inspectedClassNames)) { + mUpdateVmsStatic.AddPermittedFields(pair.getSecond().getName()); } - for (Pair<EditableOnVmStatusField, String> pair : BaseHandler.extractAnnotatedFields(EditableOnVmStatusField.class, + for (Pair<EditableOnVmStatusField, Field> pair : BaseHandler.extractAnnotatedFields(EditableOnVmStatusField.class, inspectedClassNames)) { - mUpdateVmsStatic.AddField(Arrays.asList(pair.getFirst().statuses()), pair.getSecond()); + mUpdateVmsStatic.AddField(Arrays.asList(pair.getFirst().statuses()), pair.getSecond().getName()); } + + for (Pair<EditableDeviceOnVmStatusField, Field> pair : BaseHandler.extractAnnotatedFields(EditableDeviceOnVmStatusField.class, + inspectedClassNames)) { + mUpdateVmsStatic.AddField(Arrays.asList(pair.getFirst().statuses()), pair.getSecond().getName()); + } + COMMANDS_ALLOWED_ON_EXTERNAL_VMS.add(VdcActionType.MigrateVm); COMMANDS_ALLOWED_ON_EXTERNAL_VMS.add(VdcActionType.MigrateVmToServer); COMMANDS_ALLOWED_ON_EXTERNAL_VMS.add(VdcActionType.InternalMigrateVm); @@ -129,6 +141,10 @@ public static boolean isUpdateValid(VmStatic source, VmStatic destination) { return mUpdateVmsStatic.IsUpdateValid(source, destination); + } + + public static boolean isUpdateValidForVmDevice(String fieldName, VMStatus status) { + return mUpdateVmsStatic.IsFieldUpdatable(status, fieldName, null); } public static boolean copyNonEditableFieldsToDestination(VmStatic source, VmStatic destination) { @@ -736,4 +752,44 @@ } } } + + public static boolean isUpdateValidForVmDevices(Guid vmId, Object objectWithEditableDeviceFields) { + if (objectWithEditableDeviceFields == null) { + return true; + } + return getVmDevicesFieldsToUpdateOnNextRun(vmId, objectWithEditableDeviceFields).isEmpty(); + } + + public static List<Pair<EditableDeviceOnVmStatusField, Boolean>> getVmDevicesFieldsToUpdateOnNextRun(Guid vmId, Object objectWithEditableDeviceFields) { + List<Pair<EditableDeviceOnVmStatusField, Boolean>> fieldList = new ArrayList<>(); + + List<Pair<EditableDeviceOnVmStatusField , Field>> pairList = BaseHandler.extractAnnotatedFields( + EditableDeviceOnVmStatusField.class, objectWithEditableDeviceFields.getClass()); + + for (Pair<EditableDeviceOnVmStatusField, Field> pair : pairList) { + EditableDeviceOnVmStatusField annotation = pair.getFirst(); + Field field = pair.getSecond(); + field.setAccessible(true); + + Boolean isEnabled = null; + try { + isEnabled = (Boolean) field.get(objectWithEditableDeviceFields); + } catch (IllegalAccessException | ClassCastException e) { + e.printStackTrace(); + log.warn("VmHandler:: isUpdateValidForVmDevices: Reflection error"); + } + + if (isEnabled == null || + !VmDeviceUtils.vmDeviceChanged(vmId, annotation.generalType(), + annotation.type().getName(), isEnabled)) { + continue; + } + + if (!VmHandler.isUpdateValidForVmDevice(field.getName(), VMStatus.Up)) { + fieldList.add(new Pair<>(annotation, isEnabled)); + } + } + + return fieldList; + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java index 025a5ec..3a48b3a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.bll; +import java.lang.reflect.Field; import java.util.List; import org.ovirt.engine.core.bll.context.CompensationContext; @@ -35,13 +36,13 @@ final Class<?>[] inspectedClassNames = new Class<?>[] { VmBase.class, VmTemplate.class }; mUpdateVmTemplate = new ObjectIdentityChecker(VmTemplateHandler.class); - for (Pair<EditableField, String> pair : BaseHandler.extractAnnotatedFields(EditableField.class, + for (Pair<EditableField, Field> pair : BaseHandler.extractAnnotatedFields(EditableField.class, (inspectedClassNames))) { - mUpdateVmTemplate.AddPermittedFields(pair.getSecond()); + mUpdateVmTemplate.AddPermittedFields(pair.getSecond().getName()); } - for (Pair<EditableOnTemplate, String> pair : BaseHandler.extractAnnotatedFields(EditableOnTemplate.class, inspectedClassNames)) { - mUpdateVmTemplate.AddPermittedFields(pair.getSecond()); + for (Pair<EditableOnTemplate, Field> pair : BaseHandler.extractAnnotatedFields(EditableOnTemplate.class, inspectedClassNames)) { + mUpdateVmTemplate.AddPermittedFields(pair.getSecond().getName()); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java index a775086..fbda9d0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java @@ -285,7 +285,7 @@ snapshotType, vm, true, memoryVolume, disks, compensationContext); } - /** + /**addSnapshot * Save snapshot to DB with compensation data. * * @param snapshotId @@ -313,10 +313,32 @@ String memoryVolume, List<DiskImage> disks, final CompensationContext compensationContext) { + return addSnapshot(snapshotId, + description, + snapshotStatus, + snapshotType, + vm, + saveVmConfiguration, + memoryVolume, + disks, + null, + compensationContext); + } + + public Snapshot addSnapshot(Guid snapshotId, + String description, + SnapshotStatus snapshotStatus, + SnapshotType snapshotType, + VM vm, + boolean saveVmConfiguration, + String memoryVolume, + List<DiskImage> disks, + Map<Guid, VmDevice> vmDevices, + final CompensationContext compensationContext) { final Snapshot snapshot = new Snapshot(snapshotId, snapshotStatus, vm.getId(), - saveVmConfiguration ? generateVmConfiguration(vm, disks) : null, + saveVmConfiguration ? generateVmConfiguration(vm, disks, vmDevices) : null, snapshotType, description, new Date(), @@ -335,7 +357,7 @@ * The VM to generate configuration from. * @return A String containing the VM configuration. */ - protected String generateVmConfiguration(VM vm, List<DiskImage> disks) { + protected String generateVmConfiguration(VM vm, List<DiskImage> disks, Map<Guid, VmDevice> vmDevices) { if (vm.getInterfaces() == null || vm.getInterfaces().isEmpty()) { vm.setInterfaces(getVmNetworkInterfaceDao().getAllForVm(vm.getId())); } @@ -345,7 +367,12 @@ vm.setVmtName(t.getName()); } - VmDeviceUtils.setVmDevices(vm.getStaticData()); + if (vmDevices == null) { + VmDeviceUtils.setVmDevices(vm.getStaticData()); + } else { + vm.getStaticData().setManagedDeviceMap(vmDevices); + } + if (disks == null) { disks = ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(vm.getId()), false, true, true); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java index f7e6cd4..fe8eba7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java @@ -1,14 +1,5 @@ package org.ovirt.engine.core.bll.utils; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.VmHandler; import org.ovirt.engine.core.bll.network.VmInterfaceManager; @@ -18,6 +9,7 @@ import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DisplayType; +import org.ovirt.engine.core.common.businessentities.EditableDeviceOnVmStatusField; import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VM; @@ -31,6 +23,7 @@ import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.osinfo.OsRepository; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.utils.SimpleDependecyInjector; import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils; import org.ovirt.engine.core.common.utils.VmDeviceType; @@ -39,6 +32,15 @@ import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.VmDeviceDAO; import org.ovirt.engine.core.vdsbroker.vdsbroker.VdsProperties; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; public class VmDeviceUtils { private static VmDeviceDAO dao = DbFacade.getInstance().getVmDeviceDao(); @@ -1080,4 +1082,78 @@ return dao.getVmDeviceByVmIdTypeAndDevice( vmId, VmDeviceGeneralType.CONTROLLER, VmDeviceType.VIRTIOSCSI.getName(), userID, isFiltered); } + + /** + * Determines whether a VM device change has been request by the user. + * @param deviceType VmDeviceGeneralType. + * @param device VmDeviceType name. + * @param deviceEnabled indicates whether the user asked to enable the device. + * @return true if a change has been requested; otherwise, false + */ + public static boolean vmDeviceChanged(Guid vmId, VmDeviceGeneralType deviceType, String device, boolean deviceEnabled) { + List<VmDevice> vmDevices = device != null ? + dao.getVmDeviceByVmIdTypeAndDevice(vmId, deviceType, device): + dao.getVmDeviceByVmIdAndType(vmId, deviceType); + + return deviceEnabled == vmDevices.isEmpty(); + } + + public static boolean vmDeviceChanged(Guid vmId, VmDeviceGeneralType deviceType, boolean deviceEnabled) { + return vmDeviceChanged(vmId, deviceType, null, deviceEnabled); + } + + /** + * Returns a map (device ID to VmDevice) of devices that are relevant for next run by examining + * properties that are annotated by EditableDeviceOnVmStatusField. + * @param vm the relevant VM + * @param objectWithEditableDeviceFields object that contains properties which are annotated with + * EditableDeviceField (e.g. parameters file) + * @return a map of device ID to VmDevice object of relevant devices for next run + */ + public static Map<Guid, VmDevice> getVmDevicesForNextRun(VM vm, Object objectWithEditableDeviceFields) { + VmDeviceUtils.setVmDevices(vm.getStaticData()); + Map<Guid, VmDevice> vmManagedDeviceMap = vm.getManagedVmDeviceMap(); + + List<Pair<EditableDeviceOnVmStatusField, Boolean>> fieldList = + VmHandler.getVmDevicesFieldsToUpdateOnNextRun(vm.getId(), objectWithEditableDeviceFields); + + // Add the enabled devices and remove the disabled ones + for (Pair<EditableDeviceOnVmStatusField, Boolean> pair : fieldList) { + final EditableDeviceOnVmStatusField field = pair.getFirst(); + Boolean isEnabled = pair.getSecond(); + + if (Boolean.TRUE.equals(isEnabled)) { + VmDevice device = + new VmDevice(new VmDeviceId(Guid.newGuid(), vm.getId()), + field.generalType(), + field.type().getName(), + "", + 0, + new HashMap<String, Object>(), + true, + true, + field.isReadOnly(), + "", + null, + null); + + vmManagedDeviceMap.put(device.getDeviceId(), device); + } + else { + vmManagedDeviceMap.remove(getVmDeviceIdByName(vmManagedDeviceMap, field.type().getName())); + } + } + + return vmManagedDeviceMap; + } + + private static Guid getVmDeviceIdByName(Map<Guid, VmDevice> vmManagedDeviceMap, String name) { + for (Map.Entry<Guid, VmDevice> entry : vmManagedDeviceMap.entrySet()) { + if (entry.getValue().getDevice().equals(name)) { + return entry.getKey(); + } + } + + return null; + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java index 9abde8d..dff5db1 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java @@ -35,7 +35,6 @@ import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; -import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.config.ConfigValues; @@ -300,10 +299,10 @@ doReturn(vmDeviceDAO).when(command).getVmDeviceDao(); doReturn(true).when(command).areUpdatedFieldsLegal(); - doReturn(Collections.emptyList()).when(vmDeviceDAO).getVmDeviceByVmIdAndType( - any(Guid.class), any(VmDeviceGeneralType.class)); - doReturn(Collections.singletonList(new VmDevice())).when(vmDeviceDAO).getVmDeviceByVmIdTypeAndDevice( - vm.getId(), VmDeviceGeneralType.CONTROLLER, VmDeviceType.VIRTIOSCSI.getName()); + doReturn(false).when(command).vmDeviceChanged( + any(Guid.class), any(VmDeviceGeneralType.class), any(Boolean.class)); + doReturn(true).when(command).vmDeviceChanged( + vm.getId(), VmDeviceGeneralType.CONTROLLER, VmDeviceType.VIRTIOSCSI.getName(), false); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VM_CANNOT_UPDATE_DEVICE_VM_NOT_DOWN); @@ -318,10 +317,10 @@ doReturn(vmDeviceDAO).when(command).getVmDeviceDao(); doReturn(true).when(command).areUpdatedFieldsLegal(); - doReturn(Collections.emptyList()).when(vmDeviceDAO).getVmDeviceByVmIdAndType( - any(Guid.class), any(VmDeviceGeneralType.class)); - doReturn(Collections.emptyList()).when(vmDeviceDAO).getVmDeviceByVmIdTypeAndDevice( - any(Guid.class), any(VmDeviceGeneralType.class), any(String.class)); + doReturn(false).when(command).vmDeviceChanged( + any(Guid.class), any(VmDeviceGeneralType.class), any(Boolean.class)); + doReturn(false).when(command).vmDeviceChanged( + any(Guid.class), any(VmDeviceGeneralType.class), any(String.class), any(Boolean.class)); CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java index 222f967..1a37630 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java @@ -39,13 +39,13 @@ * class array of the scanned types. * @return array of pairs of a the annotation instance -> field name */ - public static <A extends Annotation> List<Pair<A , String>> extractAnnotatedFields(Class<A> annotation, Class<?>... clz) { - List<Pair<A, String>> pairList = new ArrayList<Pair<A, String>>(); + public static <A extends Annotation> List<Pair<A , Field>> extractAnnotatedFields(Class<A> annotation, Class<?>... clz) { + List<Pair<A, Field>> pairList = new ArrayList<Pair<A, Field>>(); for (Class<?> clazz : clz) { for (Field field : clazz.getDeclaredFields()) { A fieldAnnotation = field.getAnnotation(annotation); if (fieldAnnotation != null) { - pairList.add(new Pair<A, String>(fieldAnnotation, field.getName())); + pairList.add(new Pair<A, Field>(fieldAnnotation, field)); } } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableDeviceOnVmStatusField.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableDeviceOnVmStatusField.java new file mode 100644 index 0000000..34cdc1f --- /dev/null +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EditableDeviceOnVmStatusField.java @@ -0,0 +1,21 @@ +package org.ovirt.engine.core.common.businessentities; + +import org.ovirt.engine.core.common.utils.VmDeviceType; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target(ElementType.FIELD) +@Retention(RetentionPolicy.RUNTIME) +public @interface EditableDeviceOnVmStatusField { + + VMStatus[] statuses() default { VMStatus.Down }; + + VmDeviceGeneralType generalType(); + + VmDeviceType type(); + + boolean isReadOnly() default false; +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmUpdatesOnNextRunExistsParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmUpdatesOnNextRunExistsParameters.java index 11c5f93..0b52b83 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmUpdatesOnNextRunExistsParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmUpdatesOnNextRunExistsParameters.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.common.queries; +import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.businessentities.VM; public class GetVmUpdatesOnNextRunExistsParameters extends VdcQueryParametersBase { @@ -8,12 +9,15 @@ private VM updated; + private VmManagementParametersBase updateVmParameters; + public GetVmUpdatesOnNextRunExistsParameters() { } - public GetVmUpdatesOnNextRunExistsParameters(VM original, VM updated) { + public GetVmUpdatesOnNextRunExistsParameters(VM original, VM updated, VmManagementParametersBase updateVmParameters) { this.original = original; this.updated = updated; + this.updateVmParameters = updateVmParameters; } public VM getOriginal() { @@ -31,4 +35,12 @@ public void setUpdated(VM updated) { this.updated = updated; } + + public VmManagementParametersBase getUpdateVmParameters() { + return updateVmParameters; + } + + public void setUpdateVmParameters(VmManagementParametersBase updateVmParameters) { + this.updateVmParameters = updateVmParameters; + } } 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 39ed429..a58ff8e 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 @@ -87,7 +87,7 @@ return permitted.contains(name); } - private boolean IsFieldUpdatable(Enum<?> status, String name, Object fieldContainer) { + public boolean IsFieldUpdatable(Enum<?> status, String name, Object fieldContainer) { boolean returnValue = true; if (!IsFieldUpdatable(name)) { if (fieldContainer != null && container != null diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java index 902fe62..452999f 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java @@ -22,6 +22,7 @@ import org.ovirt.engine.core.common.VdcEventNotificationUtils; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.action.gluster.GlusterVolumeRemoveBricksQueriesParameters; import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.ArchitectureType; @@ -670,9 +671,9 @@ Frontend.getInstance().runQuery(VdcQueryType.GetVmNextRunConfiguration, new IdQueryParameters(vmId), aQuery); } - public static void isNextRunConfigurationChanged(VM original, VM updated, AsyncQuery aQuery) { + public static void isNextRunConfigurationChanged(VM original, VM updated, VmManagementParametersBase updateVmParameters, AsyncQuery aQuery) { Frontend.getInstance().runQuery(VdcQueryType.GetVmUpdatesOnNextRunExists, - new GetVmUpdatesOnNextRunExistsParameters(original, updated), aQuery); + new GetVmUpdatesOnNextRunExistsParameters(original, updated, updateVmParameters), aQuery); } public static void getDataCenterList(AsyncQuery aQuery) { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java index 8eac616..13540a5 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java @@ -1158,7 +1158,7 @@ gettempVm().setUseLatestVersion(constants.latestTemplateVersionName().equals(model.getTemplate().getSelectedItem().getTemplateVersionName())); if (selectedItem.isRunningOrPaused()) { - AsyncDataProvider.isNextRunConfigurationChanged(editedVm, gettempVm(), new AsyncQuery(this, + AsyncDataProvider.isNextRunConfigurationChanged(editedVm, gettempVm(), getUpdateVmParameters(false), new AsyncQuery(this, new INewAsyncCallback() { @Override public void onSuccess(Object thisModel, Object returnValue) { @@ -1204,11 +1204,7 @@ new IFrontendActionAsyncCallback() { @Override public void executed(FrontendActionAsyncResult result) { - VmManagementParametersBase param = new VmManagementParametersBase(gettempVm()); - param.setSoundDeviceEnabled(model.getIsSoundcardEnabled().getEntity()); - param.setConsoleEnabled(model.getIsConsoleDeviceEnabled().getEntity()); - param.setApplyChangesLater(applyCpuChangesLater); - setRngDeviceToParams(model, param); + VmManagementParametersBase param = getUpdateVmParameters(applyCpuChangesLater); Frontend.getInstance().runAction(VdcActionType.UpdateVm, param, new UnitVmModelNetworkAsyncCallback(model, defaultNetworkCreatingManager, gettempVm().getId()), this); } @@ -1216,17 +1212,24 @@ } else { - VmManagementParametersBase param = new VmManagementParametersBase(gettempVm()); - param.setSoundDeviceEnabled(model.getIsSoundcardEnabled().getEntity()); - param.setConsoleEnabled(model.getIsConsoleDeviceEnabled().getEntity()); - setRngDeviceToParams(model, param); - param.setApplyChangesLater(applyCpuChangesLater); - setRngDeviceToParams(model, param); - + VmManagementParametersBase param = getUpdateVmParameters(applyCpuChangesLater); Frontend.getInstance().runAction(VdcActionType.UpdateVm, param, new UnitVmModelNetworkAsyncCallback(model, defaultNetworkCreatingManager, gettempVm().getId()), this); } } + private VmManagementParametersBase getUpdateVmParameters(boolean applyCpuChangesLater) { + UnitVmModel model = (UnitVmModel) getWindow(); + VmManagementParametersBase params = new VmManagementParametersBase(gettempVm()); + + params.setSoundDeviceEnabled(model.getIsSoundcardEnabled().getEntity()); + params.setConsoleEnabled(model.getIsConsoleDeviceEnabled().getEntity()); + setRngDeviceToParams(model, params); + params.setApplyChangesLater(applyCpuChangesLater); + setRngDeviceToParams(model, params); + + return params; + } + private void setRngDeviceToParams(UnitVmModel model, VmManagementParametersBase parameters) { parameters.setUpdateRngDevice(true); parameters.setRngDevice(model.getIsRngEnabled().getEntity() ? model.generateRngDevice() : null); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java index e68e0de..2903d4d 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java @@ -2021,7 +2021,7 @@ getcurrentVm().setUseLatestVersion(constants.latestTemplateVersionName().equals(model.getTemplate().getSelectedItem().getTemplateVersionName())); if (selectedItem.isRunningOrPaused()) { - AsyncDataProvider.isNextRunConfigurationChanged(editedVm, getcurrentVm(), new AsyncQuery(this, + AsyncDataProvider.isNextRunConfigurationChanged(editedVm, getcurrentVm(), getUpdateVmParameters(false), new AsyncQuery(this, new INewAsyncCallback() { @Override public void onSuccess(Object thisModel, Object returnValue) { @@ -2086,14 +2086,7 @@ { VM vm = vmListModel.getcurrentVm(); - VmManagementParametersBase updateVmParams = new VmManagementParametersBase(vm); - setVmWatchdogToParams(model, updateVmParams); - updateVmParams.setSoundDeviceEnabled(model.getIsSoundcardEnabled() - .getEntity()); - updateVmParams.setBalloonEnabled(balloonEnabled(model)); - updateVmParams.setVirtioScsiEnabled(model.getIsVirtioScsiEnabled().getEntity()); - updateVmParams.setApplyChangesLater(applyCpuChangesLater); - setRngDeviceToParams(model, updateVmParams); + VmManagementParametersBase updateVmParams = vmListModel.getUpdateVmParameters(applyCpuChangesLater); Frontend.getInstance().runAction(VdcActionType.UpdateVm, updateVmParams, new UnitVmModelNetworkAsyncCallback(model, defaultNetworkCreatingManager, vm.getId()), vmListModel); @@ -2111,22 +2104,27 @@ { model.startProgress(null); - VM vm = getcurrentVm(); - - VmManagementParametersBase updateVmParams = new VmManagementParametersBase(vm); - - setVmWatchdogToParams(model, updateVmParams); - updateVmParams.setSoundDeviceEnabled(model.getIsSoundcardEnabled().getEntity()); - updateVmParams.setConsoleEnabled(model.getIsConsoleDeviceEnabled().getEntity()); - updateVmParams.setBalloonEnabled(balloonEnabled(model)); - updateVmParams.setVirtioScsiEnabled(model.getIsVirtioScsiEnabled().getEntity()); - updateVmParams.setApplyChangesLater(applyCpuChangesLater); - setRngDeviceToParams(model, updateVmParams); + VmManagementParametersBase updateVmParams = getUpdateVmParameters(applyCpuChangesLater); Frontend.getInstance().runAction(VdcActionType.UpdateVm, updateVmParams, new UnitVmModelNetworkAsyncCallback(model, defaultNetworkCreatingManager, getcurrentVm().getId()), this); } } + public VmManagementParametersBase getUpdateVmParameters(boolean applyCpuChangesLater) { + UnitVmModel model = (UnitVmModel) getWindow(); + VmManagementParametersBase updateVmParams = new VmManagementParametersBase(getcurrentVm()); + + setVmWatchdogToParams(model, updateVmParams); + updateVmParams.setSoundDeviceEnabled(model.getIsSoundcardEnabled().getEntity()); + updateVmParams.setConsoleEnabled(model.getIsConsoleDeviceEnabled().getEntity()); + updateVmParams.setBalloonEnabled(balloonEnabled(model)); + updateVmParams.setVirtioScsiEnabled(model.getIsVirtioScsiEnabled().getEntity()); + updateVmParams.setApplyChangesLater(applyCpuChangesLater); + setRngDeviceToParams(model, updateVmParams); + + return updateVmParams; + } + private void saveNewVm(final UnitVmModel model) { if (getcurrentVm().getVmtGuid().equals(Guid.Empty)) { -- To view, visit http://gerrit.ovirt.org/30043 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4e7add4e7e09fe8d6d1f93f0d2b96795d838efa2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches