[libvirt] [PATCH] spice: expose the disable file transfer option
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. --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 2 ++ 4 files changed, 40 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e65f3e3..5483a91 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, "enabled"); + 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; 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 2dbb8f8..cafcb84 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 d723dc8..e370853 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7394,6 +7394,8 @@ 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) + virBufferAddLit(&opt, ",disable-agent-file-xfer"); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEAMLESS_MIGRATION)) { /* If qemu supports seamless migration turn it -- 1.8.4.2
On 01/02/2014 02:59 AM, 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. --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 2 ++ 4 files changed, 40 insertions(+)
Thanks for starting this patch. Since this adds new XML, you also need to patch docs/formatdomain.html.in to document it, docs/schemas/domaincommon.rng to allow 'virt-xml-validate' to recognize it, and tests/qemuxml2* to add a new testsuite that proves we can parse the new XML as valid, round-trip it through our internal representation, and generate the correct command line option. Also, it helps if the commit message describes the actual XML you added to expose the option. Are you able to help add those pieces to get this patch incorporated?
@@ -8519,6 +8525,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, VIR_FREE(copypaste);
You added a parser for the new function, but did not modify the output function, which means your action is one-way; it won't survive a round trip to 'virsh dumpxml' and would be lost the next time we use the domain. Again, that's why we insist on testsuite additions for new XML features.
+++ b/src/qemu/qemu_command.c @@ -7394,6 +7394,8 @@ 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) + virBufferAddLit(&opt, ",disable-agent-file-xfer");
It is probably also wise to split this into two patches. Not all versions of qemu support the new option, so we probably want to add a feature bit into qemu_capabilities.[hc] that probes when we can safely use the feature, so that we can issue a graceful error for qemu versions that lack it. In which case you want two patches - one for the XML addition, and one for the qemu capability addition. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Hello, ----- Original Message -----
From: "Eric Blake" <eblake@redhat.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com Sent: Saturday, January 4, 2014 3:33:09 PM Subject: Re: [libvirt] [PATCH] spice: expose the disable file transfer option
On 01/02/2014 02:59 AM, 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. --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 2 ++ 4 files changed, 40 insertions(+)
Thanks for starting this patch. Since this adds new XML, you also need to patch docs/formatdomain.html.in to document it, docs/schemas/domaincommon.rng to allow 'virt-xml-validate' to recognize it, and tests/qemuxml2* to add a new testsuite that proves we can parse the new XML as valid, round-trip it through our internal representation, and generate the correct command line option. Also, it helps if the commit message describes the actual XML you added to expose the option.
Are you able to help add those pieces to get this patch incorporated?
Yes, I'll go ahead. I'm not sure the XML I added is the best way to express the option (I have no strong preference on this topic, however), but I think we can discuss this after the bits you outlined are in place.
@@ -8519,6 +8525,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, VIR_FREE(copypaste);
You added a parser for the new function, but did not modify the output function, which means your action is one-way; it won't survive a round trip to 'virsh dumpxml' and would be lost the next time we use the domain. Again, that's why we insist on testsuite additions for new XML features.
Ugh, this was unintentional, thanks for pointing this out, I'll update the patch. [...]
It is probably also wise to split this into two patches. Not all versions of qemu support the new option, so we probably want to add a feature bit into qemu_capabilities.[hc] that probes when we can safely use the feature, so that we can issue a graceful error for qemu versions that lack it. In which case you want two patches - one for the XML addition, and one for the qemu capability addition.
Fine for me, I'll update the patch(set) accordingly and repost once ready. Thanks for the review and happy new year 2014, -- Francesco Romani
participants (2)
-
Eric Blake -
Francesco Romani