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

On 01/02/2014 02:59 AM, Francesco Romani wrote: 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? 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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org