On 03/13/2014 04:06 PM, Eric Blake wrote: > On 03/12/2014 10:04 PM, Laine Stump wrote: >> This should be squashed into the first patch of the previous >> "eliminate hardcoded indent" series (which was ACKed but hasn't yet >> been pushed): >> >> https://www.redhat.com/archives/libvir-list/2014-March/msg00361.html >> >> The initial patch neglected to change the functions that format >> private metadata in the qemu and lxc domain xml. >> >> I have verified that make check still passes when these changes are >> squashed into the original. >> --- >> src/conf/domain_conf.c | 2 +- >> src/lxc/lxc_domain.c | 4 ++-- >> src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++------------------ >> 3 files changed, 30 insertions(+), 21 deletions(-) >> > ACK. > > Hmm, your comment about dropping all the virBufferAdjustIndent() calls, > free-form output of the text without regards to indentation, then > running it through util/virxml.c to parse and then pretty-print the xml, > may be more maintainable, even if it eats more CPU cycles. But if we do > that, it could be a followup to this series. Okay, I made the changes you suggested and pushed this series along with the previous one. It is now illegal to call a virBuffer function with a string that starts with spaces followed by <. I did some experimenting with a pretty-printing virBuffer (as you mentioned above) tonight. After making a virBufferXMLContentAndReset() function that calls virXMLParseString() followed by virXMLNodeToString() (which would magically indent xml that started out unindented), and modifying virDomainDefFormat() to use it, I found the following: 1) if you modify qemuxml2xmltest to run the parse/format operation 300 times for each document, the time to run the test is ~30 seconds when using virBufferContentAndReset() (manual indentation), and 35 seconds when using virBufferXMLContentAndReset() (pretty print). So it adds about 17% to the time for a parse/format roundtrip. 2) virXMLNodeToString() uses double quotes for strings, while we use single quotes everywhere. So all of the tests end up failing (this may have slightly skewed the timing above, since the strcmp would have exited sooner each time). (DV pointed out xmlTextWriter*() functions in libxml as a possible way to overcome the double vs. single quotes problem, but I couldn't immediately figure out how to use it.) 3) I noticed in the code that many of the places where we call virBufferContentAndReset(), we are assuming that it will be successful, just returning the result to the caller, and the caller assumes that if it gets a NULL, an error will have already been reported. But if there is an error in the XML, the pretty-printing version could fail, so every caller will need to be modified to handle an error in this case. 4) libvirt-setuid-rpc-client.la (used by virt-login-shell) needs to have every util .c file it uses added explicitly because we want to give the setuid process access to as little as possible. But it already uses virbuffer.c, and if virbuffer.c calls virXMLParseString(), it will also need to include virxml.c. *and* if virxml.c is needed, then it will also need to link with libxml2. That may be beyond what we want in virt-login-shell. So for the moment I'm going to just let this idea simmer - I think the extra overhead is probably acceptable in return for the easier maintenance, but I'm not sure what to do about the double vs. single quotes, nor whether or not it is acceptable to add so much new code to the footprint of virt-login-shell. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list