Arik Hadas 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 12: being s/being/being shown? 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 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 second method and then change the db query according to this field)? 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? 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 them? 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? 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 the cluster. I'm not sure if that's bad though. tomas, is that ok? 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 <de...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
