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
Attachment:
pgp8FtM6j3UQ4.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list