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

From: "Eric Blake" <eblake(a)redhat.com&gt; To: "Francesco Romani" <fromani(a)redhat.com&gt;, libvir-list(a)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?

> @@ -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.

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.