[libvirt] [PATCH 0/2] v2: spice: expose the disable file transfer option
Here it is the second version of the patch which export the spice agent disable file transfer. I followed Eric's advice and added detection of the support in QEMU in the first patch; the second builds upon the first and exports the support to libvirt. The XML format is intentionally similar to the 'clipboard' element, I thought the share enough similiarities; however, suggestions are more than welcome here. Please also feel free to suggest how to extend the testing, and if what was added here is enough. As everyone probably already noticed it's my first libvirt contribution; I tried to follow HACKING's advice caredully but I can have missed something (more). Best regards, Francesco Romani (2): spice: detect if qemu can disable file transfer spice: expose the QEMU disable file transfer option docs/formatdomain.html.in | 8 +++++ docs/schemas/domaincommon.rng | 11 ++++++ src/conf/domain_conf.c | 31 ++++++++++++++++- src/conf/domain_conf.h | 10 ++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 5 ++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 9 +++++ tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + ...emuxml2argv-graphics-spice-agent-file-xfer.args | 9 +++++ ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 ++++ 13 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml -- 1.8.4.2
spice-server offers an API to disable file transfer messages on the agent channel between the client and the guest. This is supported in qemu through the disable-agent-file-xfer option. This detects if QEMU supports this option, and add a capability if does. --- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0538115..c3f2e65 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -247,6 +247,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "boot-strict", /* 160 */ "pvpanic", "enable-fips", + "spice-file-xfer-disable" ); struct _virQEMUCaps { @@ -2565,8 +2566,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); - if (qemuCaps->version >= 1006000) + if (qemuCaps->version >= 1006000) { virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + } if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index efb3f43..23dccce 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -201,6 +201,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_BOOT_STRICT = 160, /* -boot strict */ QEMU_CAPS_DEVICE_PANIC = 161, /* -device pvpanic */ QEMU_CAPS_ENABLE_FIPS = 162, /* -enable-fips */ + QEMU_CAPS_SPICE_FILE_XFER_DISABLE = 163, /* -spice disable-agent-file-xfer */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 2d50cf9..61542a8 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -139,4 +139,5 @@ <flag name='pvpanic'/> <flag name='reboot-timeout'/> <flag name='enable-fips'/> + <flag name='spice-file-xfer-disable'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index ba64177..8ce17aa 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -137,4 +137,5 @@ <flag name='boot-strict'/> <flag name='pvpanic'/> <flag name='reboot-timeout'/> + <flag name='spice-file-xfer-disable'/> </qemuCaps> -- 1.8.4.2
On Tue, Jan 07, 2014 at 08:04:32PM +0100, Francesco Romani wrote:
spice-server offers an API to disable file transfer messages on the agent channel between the client and the guest. This is supported in qemu through the disable-agent-file-xfer option.
Looks good to me, ACK. Christophe
This detects if QEMU supports this option, and add a capability if does. --- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + 4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0538115..c3f2e65 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -247,6 +247,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "boot-strict", /* 160 */ "pvpanic", "enable-fips", + "spice-file-xfer-disable" );
struct _virQEMUCaps { @@ -2565,8 +2566,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET);
- if (qemuCaps->version >= 1006000) + if (qemuCaps->version >= 1006000) { virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + }
if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index efb3f43..23dccce 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -201,6 +201,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_BOOT_STRICT = 160, /* -boot strict */ QEMU_CAPS_DEVICE_PANIC = 161, /* -device pvpanic */ QEMU_CAPS_ENABLE_FIPS = 162, /* -enable-fips */ + QEMU_CAPS_SPICE_FILE_XFER_DISABLE = 163, /* -spice disable-agent-file-xfer */
QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 2d50cf9..61542a8 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -139,4 +139,5 @@ <flag name='pvpanic'/> <flag name='reboot-timeout'/> <flag name='enable-fips'/> + <flag name='spice-file-xfer-disable'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index ba64177..8ce17aa 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -137,4 +137,5 @@ <flag name='boot-strict'/> <flag name='pvpanic'/> <flag name='reboot-timeout'/> + <flag name='spice-file-xfer-disable'/> </qemuCaps> -- 1.8.4.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
spice-server offers an API to disable file transfer messages on the agent channel between the client and the guest. This is supported in qemu through the disable-agent-file-xfer option. This patch exposes this option to libvirt. Adds a new element 'filetransfer', with one property, 'filetransfer', which accepts a boolean setting. Default is enabled. Depends on the capability exported in the first patch of the series. --- docs/formatdomain.html.in | 8 +++++ docs/schemas/domaincommon.rng | 11 ++++++ src/conf/domain_conf.c | 31 ++++++++++++++++- src/conf/domain_conf.h | 10 ++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 9 +++++ ...emuxml2argv-graphics-spice-agent-file-xfer.args | 9 +++++ ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 ++++ 9 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 68860ef..f3186e8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4042,6 +4042,7 @@ qemu-kvm -net nic,model=? /dev/null <streaming mode='filter'/> <clipboard copypaste='no'/> <mouse mode='client'/> + <filetransfer enable='no'/> </graphics></pre> <p> Spice supports variable compression settings for audio, @@ -4081,6 +4082,13 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">since 0.9.11</span>. If no mode is specified, the qemu default will be used (client mode). </p> + <p> + File transfer on the agent channel between the client and + the guest is supported through spice-server and enabled + by default. It can be disabled by setting the <code>enable</code> + property to <code>no</code> in the <code>filetransfer</code> + element, <span class="since">since 1.2.1</span>. + </p> </dd> <dt><code>"rdp"</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 86a60c9..cd2c499 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2468,6 +2468,17 @@ <empty/> </element> </optional> + <optional> + <element name="filetransfer"> + <attribute name="enable"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + <empty/> + </element> + </optional> </interleave> </group> <group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e65f3e3..18b7759 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -604,6 +604,12 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste, "yes", "no"); +VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer, + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST, + "default", + "yes", + "no"); + VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST, "subsystem", "capabilities") @@ -8519,6 +8525,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, VIR_FREE(copypaste); def->data.spice.copypaste = copypasteVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "filetransfer")) { + char *enable = virXMLPropString(cur, "enable"); + int enableVal; + + if (!enable) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("spice filetransfer missing enable")); + goto error; + } + + if ((enableVal = + virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown enable value '%s'"), enable); + VIR_FREE(enable); + goto error; + } + VIR_FREE(enable); + + def->data.spice.filetransfer = enableVal; } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")) { char *mode = virXMLPropString(cur, "mode"); int modeVal; @@ -16423,7 +16449,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (!children && (def->data.spice.image || def->data.spice.jpeg || def->data.spice.zlib || def->data.spice.playback || def->data.spice.streaming || def->data.spice.copypaste || - def->data.spice.mousemode)) { + def->data.spice.mousemode || def->data.spice.filetransfer)) { virBufferAddLit(buf, ">\n"); children = true; } @@ -16448,6 +16474,9 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.spice.copypaste) virBufferAsprintf(buf, " <clipboard copypaste='%s'/>\n", virDomainGraphicsSpiceClipboardCopypasteTypeToString(def->data.spice.copypaste)); + if (def->data.spice.filetransfer) + virBufferAsprintf(buf, " <filetransfer enable='%s'/>\n", + virDomainGraphicsSpiceAgentFileTransferTypeToString(def->data.spice.copypaste)); } if (children) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 647d115..ce877fc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1461,6 +1461,14 @@ enum virDomainGraphicsSpiceClipboardCopypaste { VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_LAST }; +enum virDomainGraphicsSpiceAgentFileTransfer { + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_DEFAULT = 0, + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_YES, + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_NO, + + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST +}; + enum virDomainGraphicsListenType { VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE = 0, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS, @@ -1531,6 +1539,7 @@ struct _virDomainGraphicsDef { int playback; int streaming; int copypaste; + int filetransfer; } spice; } data; /* nListens, listens, and *port are only useful if type is vnc, @@ -2693,6 +2702,7 @@ VIR_ENUM_DECL(virDomainInputBus) VIR_ENUM_DECL(virDomainGraphics) VIR_ENUM_DECL(virDomainGraphicsListen) VIR_ENUM_DECL(virDomainGraphicsAuthConnected) +VIR_ENUM_DECL(virDomainGraphicsSpiceAgentFileTransfer) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelName) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelMode) VIR_ENUM_DECL(virDomainGraphicsSpiceImageCompression) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3b3de15..2a9b0b1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -235,6 +235,8 @@ virDomainGraphicsListenGetType; virDomainGraphicsListenSetAddress; virDomainGraphicsListenSetNetwork; virDomainGraphicsListenSetType; +virDomainGraphicsSpiceAgentFileTransferTypeFromString; +virDomainGraphicsSpiceAgentFileTransferTypeToString; virDomainGraphicsSpiceChannelModeTypeFromString; virDomainGraphicsSpiceChannelModeTypeToString; virDomainGraphicsSpiceChannelNameTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 35b7c67..0398675 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7390,6 +7390,15 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming)); if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) virBufferAddLit(&opt, ",disable-copy-paste"); + if (graphics->data.spice.filetransfer == VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_NO) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU can't disable the spice file transfer")); + goto error; + } else { + virBufferAddLit(&opt, ",disable-agent-file-xfer"); + } + } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEAMLESS_MIGRATION)) { /* If qemu supports seamless migration turn it diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args new file mode 100644 index 0000000..66f22bc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +/dev/HostVG/QEMUGuest1 -spice port=5903,tls-port=5904,addr=127.0.0.1,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs,\ +disable-agent-file-xfer -vga qxl -global qxl-vga.ram_size=67108864 \ +-global qxl-vga.vram_size=33554432 \ +-device qxl,id=video1,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml new file mode 100644 index 0000000..3a3e366 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + <channel name='main' mode='secure'/> + <channel name='inputs' mode='insecure'/> + <filetransfer enable='no'/> + </graphics> + <video> + <model type='qxl' ram='65536' vram='32768' heads='1'/> + </video> + <video> + <model type='qxl' ram='65536' vram='65536' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b90f0e7..64e04df 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -885,6 +885,12 @@ mymain(void) QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_USB_HUB, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_USB_REDIR, QEMU_CAPS_CHARDEV_SPICEVMC); + DO_TEST("graphics-spice-agent-file-xfer", + QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_QXL_VGA, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE_FILE_XFER_DISABLE); DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); -- 1.8.4.2
On Tue, Jan 07, 2014 at 08:04:33PM +0100, Francesco Romani wrote:
spice-server offers an API to disable file transfer messages on the agent channel between the client and the guest. This is supported in qemu through the disable-agent-file-xfer option.
This patch exposes this option to libvirt. Adds a new element 'filetransfer', with one property, 'filetransfer', which accepts a boolean setting. Default is enabled.
Depends on the capability exported in the first patch of the series. --- docs/formatdomain.html.in | 8 +++++ docs/schemas/domaincommon.rng | 11 ++++++ src/conf/domain_conf.c | 31 ++++++++++++++++- src/conf/domain_conf.h | 10 ++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 9 +++++ ...emuxml2argv-graphics-spice-agent-file-xfer.args | 9 +++++ ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 ++++ 9 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 68860ef..f3186e8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4042,6 +4042,7 @@ qemu-kvm -net nic,model=? /dev/null <streaming mode='filter'/> <clipboard copypaste='no'/> <mouse mode='client'/> + <filetransfer enable='no'/> </graphics></pre> <p> Spice supports variable compression settings for audio, @@ -4081,6 +4082,13 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">since 0.9.11</span>. If no mode is specified, the qemu default will be used (client mode). </p> + <p> + File transfer on the agent channel between the client and + the guest is supported through spice-server and enabled + by default. It can be disabled by setting the <code>enable</code> + property to <code>no</code> in the <code>filetransfer</code> + element, <span class="since">since 1.2.1</span>.
I'd use a wording similar to what is used for copy&paste: "File transfer functionality (via Spice agent) is set using the <code>filetransfer</code> element. It is enabled by default, and can be disabled by setting the <code>copypaste</code> property to <code>no</code>", or at least I'd reword the first sentence which is a bit confusing imo, and is mostly implementation details. At this point, I think it will be "since 1.2.2"
+ </p> </dd> <dt><code>"rdp"</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 86a60c9..cd2c499 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2468,6 +2468,17 @@ <empty/> </element> </optional> + <optional> + <element name="filetransfer"> + <attribute name="enable"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + <empty/> + </element> + </optional> </interleave> </group> <group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e65f3e3..18b7759 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -604,6 +604,12 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste, "yes", "no");
+VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer, + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST, + "default", + "yes", + "no"); + VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST, "subsystem", "capabilities") @@ -8519,6 +8525,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, VIR_FREE(copypaste);
def->data.spice.copypaste = copypasteVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "filetransfer")) { + char *enable = virXMLPropString(cur, "enable"); + int enableVal; + + if (!enable) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("spice filetransfer missing enable")); + goto error; + } + + if ((enableVal = + virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown enable value '%s'"), enable);
the code to disable copy&paste uses VIR_ERR_INTERNAL_ERROR when it's not able to parse the enum value, but the compression code uses VIR_ERR_CONFIG_UNSUPPORTED, and the mouse code uses VIR_ERR_XML_ERROR :( The rest of domain_conf.c is not very consistent in what error is reported on unknown enum values :( I'd personnally go with either VIR_ERR_CONFIG_UNSUPPORTED or VIR_ERR_XML_ERROR as they are more specific than VIR_ERR_INTERNAL_ERROR. However, given that the rest of the code is inconsistent, this can stay this way for now, and be fixed up at a later time.
+ VIR_FREE(enable); + goto error; + } + VIR_FREE(enable); + + def->data.spice.filetransfer = enableVal; } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")) { char *mode = virXMLPropString(cur, "mode"); int modeVal; @@ -16423,7 +16449,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (!children && (def->data.spice.image || def->data.spice.jpeg || def->data.spice.zlib || def->data.spice.playback || def->data.spice.streaming || def->data.spice.copypaste || - def->data.spice.mousemode)) { + def->data.spice.mousemode || def->data.spice.filetransfer)) { virBufferAddLit(buf, ">\n"); children = true; } @@ -16448,6 +16474,9 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.spice.copypaste) virBufferAsprintf(buf, " <clipboard copypaste='%s'/>\n", virDomainGraphicsSpiceClipboardCopypasteTypeToString(def->data.spice.copypaste)); + if (def->data.spice.filetransfer) + virBufferAsprintf(buf, " <filetransfer enable='%s'/>\n", + virDomainGraphicsSpiceAgentFileTransferTypeToString(def->data.spice.copypaste));
s/copypaste/filetransfer/
}
if (children) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 647d115..ce877fc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1461,6 +1461,14 @@ enum virDomainGraphicsSpiceClipboardCopypaste { VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_LAST };
+enum virDomainGraphicsSpiceAgentFileTransfer { + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_DEFAULT = 0, + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_YES, + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_NO, + + VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST +}; + enum virDomainGraphicsListenType { VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE = 0, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS, @@ -1531,6 +1539,7 @@ struct _virDomainGraphicsDef { int playback; int streaming; int copypaste; + int filetransfer; } spice; } data; /* nListens, listens, and *port are only useful if type is vnc, @@ -2693,6 +2702,7 @@ VIR_ENUM_DECL(virDomainInputBus) VIR_ENUM_DECL(virDomainGraphics) VIR_ENUM_DECL(virDomainGraphicsListen) VIR_ENUM_DECL(virDomainGraphicsAuthConnected) +VIR_ENUM_DECL(virDomainGraphicsSpiceAgentFileTransfer) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelName) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelMode) VIR_ENUM_DECL(virDomainGraphicsSpiceImageCompression) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3b3de15..2a9b0b1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -235,6 +235,8 @@ virDomainGraphicsListenGetType; virDomainGraphicsListenSetAddress; virDomainGraphicsListenSetNetwork; virDomainGraphicsListenSetType; +virDomainGraphicsSpiceAgentFileTransferTypeFromString; +virDomainGraphicsSpiceAgentFileTransferTypeToString; virDomainGraphicsSpiceChannelModeTypeFromString; virDomainGraphicsSpiceChannelModeTypeToString; virDomainGraphicsSpiceChannelNameTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 35b7c67..0398675 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7390,6 +7390,15 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming)); if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) virBufferAddLit(&opt, ",disable-copy-paste"); + if (graphics->data.spice.filetransfer == VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_NO) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU can't disable the spice file transfer"));
I'd say "spice file transfers" or "file transfers through spice" rather than "the spice file transfer" Christophe
Hi, thanks for the review
I'd use a wording similar to what is used for copy&paste: "File transfer functionality (via Spice agent) is set using the <code>filetransfer</code> element. It is enabled by default, and can be disabled by setting the <code>copypaste</code> property to <code>no</code>", or at least I'd reword the first sentence which is a bit confusing imo, and is mostly implementation details. At this point, I think it will be "since 1.2.2"
fine for me, I will fix both.
virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown enable value '%s'"), enable);
the code to disable copy&paste uses VIR_ERR_INTERNAL_ERROR when it's not able to parse the enum value, but the compression code uses VIR_ERR_CONFIG_UNSUPPORTED, and the mouse code uses VIR_ERR_XML_ERROR :( The rest of domain_conf.c is not very consistent in what error is reported on unknown enum values :( I'd personnally go with either VIR_ERR_CONFIG_UNSUPPORTED or VIR_ERR_XML_ERROR as they are more specific than VIR_ERR_INTERNAL_ERROR. However, given that the rest of the code is inconsistent, this can stay this way for now, and be fixed up at a later time.
I already have to resubmit, and I personally like errors to be specific, so I will change to VIR_ERR_CONFIG_UNSUPPORTED, because it seems the most relevant.
virDomainGraphicsSpiceAgentFileTransferTypeToString(def->data.spice.copypaste));
s/copypaste/filetransfer/
Ops :) will fix.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU can't disable the spice file transfer"));
I'd say "spice file transfers" or "file transfers through spice" rather than "the spice file transfer"
Ok for "file transfers through spice". New version of the patch coming soon. Thanks and best regards, -- Francesco Romani
Hey, On Thu, Jan 09, 2014 at 07:21:25AM -0500, Francesco Romani wrote:
the code to disable copy&paste uses VIR_ERR_INTERNAL_ERROR when it's not able to parse the enum value, but the compression code uses VIR_ERR_CONFIG_UNSUPPORTED, and the mouse code uses VIR_ERR_XML_ERROR :( The rest of domain_conf.c is not very consistent in what error is reported on unknown enum values :( I'd personnally go with either VIR_ERR_CONFIG_UNSUPPORTED or VIR_ERR_XML_ERROR as they are more specific than VIR_ERR_INTERNAL_ERROR. However, given that the rest of the code is inconsistent, this can stay this way for now, and be fixed up at a later time.
I already have to resubmit, and I personally like errors to be specific, so I will change to VIR_ERR_CONFIG_UNSUPPORTED, because it seems the most relevant.
Fwiw, I was thinking a bit more about this, and I'd tend to use VIR_ERR_CONFIG_UNSUPPORTED as meaning "libvirt knows about this enum value, but cannot handle it for some reason" vs VIR_ERR_XML_ERROR meaning "this document cannot be parsed because this enum value is invalid/unknown". So my preference here would go to VIR_ERR_XML_ERROR. Christophe
Hi,
On Thu, Jan 09, 2014 at 07:21:25AM -0500, Francesco Romani wrote:
the code to disable copy&paste uses VIR_ERR_INTERNAL_ERROR when it's not able to parse the enum value, but the compression code uses VIR_ERR_CONFIG_UNSUPPORTED, and the mouse code uses VIR_ERR_XML_ERROR :( The rest of domain_conf.c is not very consistent in what error is reported on unknown enum values :( I'd personnally go with either VIR_ERR_CONFIG_UNSUPPORTED or VIR_ERR_XML_ERROR as they are more specific than VIR_ERR_INTERNAL_ERROR. However, given that the rest of the code is inconsistent, this can stay this way for now, and be fixed up at a later time.
I already have to resubmit, and I personally like errors to be specific, so I will change to VIR_ERR_CONFIG_UNSUPPORTED, because it seems the most relevant.
Fwiw, I was thinking a bit more about this, and I'd tend to use VIR_ERR_CONFIG_UNSUPPORTED as meaning "libvirt knows about this enum value, but cannot handle it for some reason" vs VIR_ERR_XML_ERROR meaning "this document cannot be parsed because this enum value is invalid/unknown". So my preference here would go to VIR_ERR_XML_ERROR.
OK then, I'll change to VIR_ERR_XML_ERROR. Thanks for the explanation, -- Francesco Romani
participants (2)
-
Christophe Fergeau -
Francesco Romani