Omer Frenkel has posted comments on this change. Change subject: backend: spice: add support to control the user data transfer ......................................................................
Patch Set 15: (4 comments) http://gerrit.ovirt.org/#/c/26241/15/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java: Line 622: getVm().setRunAndPause(getParameters().getRunAndPause() == null ? getVm().isRunAndPause() : getParameters().getRunAndPause()); Line 623: getVm().setAcpiEnable(getParameters().getAcpiEnable()); Line 624: getVm().setBootMenuEnabled(getParameters().getBootMenuEnabled() == null ? getVm().isBootMenuEnabled() : getParameters().isBootMenuEnabled()); Line 625: Line 626: getVm().setSpiceFileTransferEnabled(getParameters().getSpiceFileTransferEnabled() == null ?getVm().isSpiceFileTransferEnabled() :getParameters().isSpiceFileTransferEnabled()); missing space after '?' and after ':' (above and below) Line 627: getVm().setSpiceCopyPasteEnabled(getParameters().getSpiceCopyPasteEnabled() == null ?getVm().isSpiceCopyPasteEnabled() :getParameters().isSpiceCopyPasteEnabled()); Line 628: Line 629: // Clear the first user: Line 630: getVm().setConsoleUserId(null); http://gerrit.ovirt.org/#/c/26241/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java: Line 308: private boolean bootMenuEnabled; Line 309: Line 310: @CopyOnNewVersion Line 311: @EditableOnVmStatusField Line 312: @EditableField it doesnt make sense to put both @EditableOnVmStatusField and @EditableField it should be @EditableOnVmStatusField and @EditableOnTemplate (also for the second field below) Line 313: private boolean spiceFileTransferEnabled; Line 314: Line 315: @CopyOnNewVersion Line 316: @EditableOnVmStatusField http://gerrit.ovirt.org/#/c/26241/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java: Line 1699: @DefaultValueAttribute("true") Line 1700: BootMenuSupported, Line 1701: Line 1702: @TypeConverterAttribute(Boolean.class) Line 1703: @DefaultValueAttribute("false") if you set the value to false here, and dont set explicitly true for the right version in the config.sql, it will always be false. the way it is usually done is to set here true, assuming this is supported from the current version and ahead, and false for 'old' non-supported versions in the config.sql (as you did) Line 1704: SpiceFileTransferToggleSupported, Line 1705: Line 1706: @TypeConverterAttribute(Boolean.class) Line 1707: @DefaultValueAttribute("false") http://gerrit.ovirt.org/#/c/26241/15/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java: Line 539: if (node != null) { Line 540: vmBase.setBootMenuEnabled(Boolean.parseBoolean(node.innerText)); Line 541: } Line 542: Line 543: node = content.SelectSingleNode(OvfProperties.IS_SPICE_FILE_TRANSFER_ENABLED); since old exported vms will not have this value, maybe its better to set as true for backward compatibility, if the value is missing. Line 544: if (node != null) { Line 545: vmBase.setSpiceFileTransferEnabled(Boolean.parseBoolean(node.innerText)); Line 546: } Line 547: -- To view, visit http://gerrit.ovirt.org/26241 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I945e6b23df2b9d0f7c18a4f1e73c64e12e9e1a78 Gerrit-PatchSet: 15 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@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
