Daniel Erez has posted comments on this change. Change subject: core: infrastructure for VmDevices in next run VM snapshot ......................................................................
Patch Set 2: (7 comments) http://gerrit.ovirt.org/#/c/29384/2//COMMIT_MSG Commit Message: Line 8: Line 9: Added an infrastructure for including VmDevices in a snapshot Line 10: of type 'Next Run'. This should allow enabling/disabling of Line 11: devices while VM is running (i.e. similarly to VM properties that Line 12: are editable on a non-running VM, a warning message [1] is being > s/being/being shown? Done Line 13: when editing a VM device status). Line 14: Line 15: * Created a new annotation interfce for devices: Line 16: 'EditableDeviceOnVmStatusField'. Line 11: devices while VM is running (i.e. similarly to VM properties that Line 12: are editable on a non-running VM, a warning message [1] is being Line 13: when editing a VM device status). Line 14: Line 15: * Created a new annotation interfce for devices: > s/interfce/interface Done Line 16: 'EditableDeviceOnVmStatusField'. Line 17: * VmHandler - initialized mUpdateVmsStatic with those fields. Line 18: * UpdateVmCommand - passed a list of devices for next run Line 19: when creating the snapshot with new configuration. http://gerrit.ovirt.org/#/c/29384/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java: Line 1086: public static boolean vmDeviceChanged(Guid vmId, VmDeviceGeneralType deviceType, boolean deviceEnabled) { Line 1087: List<VmDevice> vmDevices = dao.getVmDeviceByVmIdAndType(vmId, deviceType); Line 1088: Line 1089: return deviceEnabled == vmDevices.isEmpty(); Line 1090: } > how about to combine the two methods above (pass null device from the secon Good idea. Done. Line 1091: Line 1092: public static Map<Guid, VmDevice> getVmDevicesForNextRun(VM vm, Object objectWithEditableDeviceFields) { Line 1093: VmDeviceUtils.setVmDevices(vm.getStaticData()); Line 1094: Map<Guid, VmDevice> vmManagedDeviceMap = vm.getManagedVmDeviceMap(); Line 1088: Line 1089: return deviceEnabled == vmDevices.isEmpty(); Line 1090: } Line 1091: Line 1092: public static Map<Guid, VmDevice> getVmDevicesForNextRun(VM vm, Object objectWithEditableDeviceFields) { > javadoc? Done Line 1093: VmDeviceUtils.setVmDevices(vm.getStaticData()); Line 1094: Map<Guid, VmDevice> vmManagedDeviceMap = vm.getManagedVmDeviceMap(); Line 1095: Line 1096: List<Pair<EditableDeviceOnVmStatusField, Boolean>> fieldList = http://gerrit.ovirt.org/#/c/29384/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/backendinterfaces/BaseHandler.java: Line 60: * @param clz Line 61: * class array of the scanned types. Line 62: * @return array of pairs of a the annotation instance -> field Line 63: */ Line 64: public static <A extends Annotation> List<Pair<A , Field>> extractAnnotatedFieldObjects(Class<A> annotation, Class<?>... clz) { > It looks the same as the method above except the name, can we remove one of Sure, forgot to remove it :) Line 65: List<Pair<A, Field>> pairList = new ArrayList<Pair<A, Field>>(); Line 66: for (Class<?> clazz : clz) { Line 67: for (Field field : clazz.getDeclaredFields()) { Line 68: A fieldAnnotation = field.getAnnotation(annotation); http://gerrit.ovirt.org/#/c/29384/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java: Line 67: import java.util.HashMap; Line 68: import java.util.HashSet; Line 69: import java.util.List; Line 70: import java.util.Map; Line 71: import java.util.Set; > no major change in this file, can you remove it from the patch? Done Line 72: Line 73: public class UnitVmModel extends Model { Line 74: Line 75: public static final int VM_TEMPLATE_NAME_MAX_LIMIT = 40; http://gerrit.ovirt.org/#/c/29384/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java: Line 2078: { Line 2079: VM vm = vmListModel.getcurrentVm(); Line 2080: Line 2081: VmManagementParametersBase updateVmParams = vmListModel.getUpdateVmParameters(applyCpuChangesLater); Line 2082: > There is a side effect that now the console might be enabled after changing Hmm, actually I think it should be allowed; i.e. the previous behavior seems like a bug... Line 2083: Frontend.getInstance().runAction(VdcActionType.UpdateVm, Line 2084: updateVmParams, new UnitVmModelNetworkAsyncCallback(model, defaultNetworkCreatingManager, vm.getId()), vmListModel); Line 2085: } Line 2086: else -- To view, visit http://gerrit.ovirt.org/29384 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4e7add4e7e09fe8d6d1f93f0d2b96795d838efa2 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
