[libvirt] [PATCH] spice: expose the disable file transfer option

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140104/71adeada/attachment-0001.sig>