Re: [Libvir] [PATCH 0/2] Re-organize generation / handling of capabilities data

2008-02-26 Thread Daniel Veillard
On Tue, Feb 26, 2008 at 04:57:56AM +, Daniel P. Berrange wrote:
 Earlier today I started the 'simple' task of extending the QEMU driver to
 support Xenner for running Xen paravirt guests under KVM. This was at first
 glance easy because it basically looks like QEMU - in fact all we needed
 was to extend the capabilities XML for the QEMU driver to include data 
 about Xen guests if Xenner is available.
 
 Unfortunately this was easier said than done...
 
  - The code generating the XML in qemu_driver.c was a mix of string
formatting for the XML, and functional logic for determining runtime
capabilities (kvm, kqemu, etc).
  - More runtime logic was in the qemu_conf.c file and stuff that should
be runtime logic was static.
 
 I attempted to fix up the QEMU code for capabilities but it quickly turned
 into a horrible mess of spaghetti.
 
 Looking at the Xen driver, the situation was pretty much the same, but
 with the capabilities XML generation spread out over 3 files (xml.c, 
 xen_internal.c and xend_internal.c).
 
 So I decided that we needed to have a internal API  structure to allow
 drivers to formally register  query their capabilities , and also add a
 central method for serializing it to XML. The following 2 patches implement
 this idea, and porting the Xen, QEMU and Test drivers to use it. 
 
 This removes a lot of code duplication in XML formatting and makes the
 resulting code easier to follow, and should make it much quicker to
 adding capabilities info to new drivers too.

  Okay I looked at both patches (too bad diff can't spot blocks moved from
one file to another) they look fine to me. This really is a good cleanup,
there are some changes included which seems a bit unrelated but fine too,

  +1

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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


Re: [Libvir] [PATCH 0/2] Re-organize generation / handling of capabilities data

2008-02-26 Thread Daniel P. Berrange
On Tue, Feb 26, 2008 at 08:29:54AM -0500, Daniel Veillard wrote:
 On Tue, Feb 26, 2008 at 04:57:56AM +, Daniel P. Berrange wrote:
  Earlier today I started the 'simple' task of extending the QEMU driver to
  support Xenner for running Xen paravirt guests under KVM. This was at first
  glance easy because it basically looks like QEMU - in fact all we needed
  was to extend the capabilities XML for the QEMU driver to include data 
  about Xen guests if Xenner is available.
  
  Unfortunately this was easier said than done...
  
   - The code generating the XML in qemu_driver.c was a mix of string
 formatting for the XML, and functional logic for determining runtime
 capabilities (kvm, kqemu, etc).
   - More runtime logic was in the qemu_conf.c file and stuff that should
 be runtime logic was static.
  
  I attempted to fix up the QEMU code for capabilities but it quickly turned
  into a horrible mess of spaghetti.
  
  Looking at the Xen driver, the situation was pretty much the same, but
  with the capabilities XML generation spread out over 3 files (xml.c, 
  xen_internal.c and xend_internal.c).
  
  So I decided that we needed to have a internal API  structure to allow
  drivers to formally register  query their capabilities , and also add a
  central method for serializing it to XML. The following 2 patches implement
  this idea, and porting the Xen, QEMU and Test drivers to use it. 
  
  This removes a lot of code duplication in XML formatting and makes the
  resulting code easier to follow, and should make it much quicker to
  adding capabilities info to new drivers too.
 
   Okay I looked at both patches (too bad diff can't spot blocks moved from
 one file to another) they look fine to me. This really is a good cleanup,
 there are some changes included which seems a bit unrelated but fine too,
 
   +1

This is all committed now. 


Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-   Perl modules: http://search.cpan.org/~danberr/  -=|
|=-   Projects: http://freshmeat.net/~danielpb/   -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

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