Omer Frenkel has posted comments on this change. Change subject: backend: spice: add support to control the user data transfer ......................................................................
Patch Set 17: (3 comments) http://gerrit.ovirt.org/#/c/26241/17/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 1730: @TypeConverterAttribute(Boolean.class) Line 1731: @DefaultValueAttribute("true") Line 1732: SpiceCopyPasteToggleSupported, Line 1733: Line 1734: Invalid; why did you add the ';' ? basically its not needed, but moreover i dont think its related to this patch http://gerrit.ovirt.org/#/c/26241/17/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 543: node = content.SelectSingleNode(OvfProperties.IS_SPICE_FILE_TRANSFER_ENABLED); Line 544: if (node != null) { Line 545: vmBase.setSpiceFileTransferEnabled(Boolean.parseBoolean(node.innerText)); Line 546: } else { Line 547: vmBase.setSpiceFileTransferEnabled(true); // for backward compatibility this made me think, we might want to have this as true by default? in vmBase constructor? for example, what do we want the value to be by default, for user creating vms in rest api and not setting any value? if we want it to be 'true' then we should set it there, and no need the explicit code here also Line 548: } Line 549: Line 550: node = content.SelectSingleNode(OvfProperties.IS_SPICE_COPY_PASTE_ENABLED); Line 551: if (node != null) { http://gerrit.ovirt.org/#/c/26241/17/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java: Line 150: Line 151: addNumaSetting(compatibilityVersion); Line 152: Line 153: if (vm.getDisplayType() == DisplayType.qxl) { Line 154: createInfo.put(VdsProperties.spiceFileTransferEnable, i guess i know the answer, but just to be safe, old vdsm versions can handle this? this value will be ignored? Line 155: Boolean.toString(vm.isSpiceFileTransferEnabled())); Line 156: createInfo.put(VdsProperties.spiceCopyPasteEnable, Line 157: Boolean.toString(vm.isSpiceCopyPasteEnabled())); Line 158: } -- 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: 17 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
