Daniel Veillard wrote:
On Tue, Jul 24, 2007 at 10:31:10AM -0700, David Lutterkort wrote:
This patch lets the qemu/kvm drivers report features for i686 and
x86_64. The output is table-driven, like the rest of the capability
generation. That should make it easy to report additional features for
other arches (which I know zip about)

  Looks fine to me, except:

+                r = virBufferAdd(xml, "\
+      <pae/>\n", -1);

  I really can't understand what's the point of doing the indentation by
cutting in the string parameter, I would really fix those before applying.

It's like that in the current code. I did it that way so that the XML would be nicely formatted -- makes more sense on longer stretches of XML, eg:
    r = virBufferVSprintf (xml,
                        "\
<capabilities>\n\
  <host>\n\
    <cpu>\n\
      <arch>%s</arch>\n\
    </cpu>\n\
  </host>\n",
                        utsname.machine);

For single lines of code it seems like it'd be better to indent it normally. IIRC Dan Berrange was in favour of using libxml calls to build up the XML, at which point it is the job of libxml to generate nicely formatted output, but that's a lot of work to implement for only a tiny gain.

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to