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