[libvirt] [PATCH 1/2] VirtualBox support to libvirt
Hi All, I have attached a patch which when applied on the HEAD as of today would allow virtualbox support in libvirt. The patch works very well with the VirtualBox OSE version and the 2.2 Beta release. [PATCH 1/2] contains diff of files already in libvirt. [PATCH 2/2] contains new files needed for VirtualBox support. Regards, -pritesh
On Wed, Mar 18, 2009 at 06:44:31PM +0100, Pritesh Kothari wrote:
Hi All,
I have attached a patch which when applied on the HEAD as of today would allow virtualbox support in libvirt.
The patch works very well with the VirtualBox OSE version and the 2.2 Beta release.
Cool, thanks !
[PATCH 1/2] contains diff of files already in libvirt.
okay [...]
+VBOX_DRIVER_SOURCES = \ + vbox/VBoxXPCOMCGlue.c vbox/VBoxXPCOMCGlue.h \ + vbox/vbox_driver.c vbox/vbox_driver.h \ + vbox/vbox_V2_2.c
Hum, I wonder about the need of a subdirectory here, others drivers are directly in src/ ... maybe it's a good idea, but that's different from current practice. affect this too
+#ifdef WITH_VBOX +#include "vbox/vbox_driver.h" +#endif #endif
Otherwise that part of the patch looks good. Is there really no need for checkings at configure time ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi Daniel,
+VBOX_DRIVER_SOURCES = \ + vbox/VBoxXPCOMCGlue.c vbox/VBoxXPCOMCGlue.h \ + vbox/vbox_driver.c vbox/vbox_driver.h \ + vbox/vbox_V2_2.c
Hum, I wonder about the need of a subdirectory here, others drivers are directly in src/ ... maybe it's a good idea, but that's different from current practice.
Actually src/ now has 140+ files and I thought it would be better if every driver got his own place ;)
affect this too
+#ifdef WITH_VBOX +#include "vbox/vbox_driver.h" +#endif #endif
Otherwise that part of the patch looks good. Is there really no need for checkings at configure time ?
I didn't understand this, do you mean the #ifdef is not necessary? i will remove it then. Thanks, -pritesh
On Thu, Mar 19, 2009 at 02:12:21PM +0100, Pritesh Kothari wrote:
Hi Daniel,
+VBOX_DRIVER_SOURCES = \ + vbox/VBoxXPCOMCGlue.c vbox/VBoxXPCOMCGlue.h \ + vbox/vbox_driver.c vbox/vbox_driver.h \ + vbox/vbox_V2_2.c
Hum, I wonder about the need of a subdirectory here, others drivers are directly in src/ ... maybe it's a good idea, but that's different from current practice.
Actually src/ now has 140+ files and I thought it would be better if every driver got his own place ;)
affect this too
+#ifdef WITH_VBOX +#include "vbox/vbox_driver.h" +#endif #endif
that was related to the vbox subdir comment was above
Otherwise that part of the patch looks good. Is there really no need for checkings at configure time ?
I didn't understand this, do you mean the #ifdef is not necessary? i will remove it then.
nahh, I was wondering if vbox support locally should be done in configure. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Wed, Mar 18, 2009 at 06:44:31PM +0100, Pritesh Kothari wrote:
Hi All,
I have attached a patch which when applied on the HEAD as of today would allow virtualbox support in libvirt.
The patch works very well with the VirtualBox OSE version and the 2.2 Beta release.
[PATCH 1/2] contains diff of files already in libvirt.
THis first 1/2 integration patch looks fine to me. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, Mar 18, 2009 at 06:44:31PM +0100, Pritesh Kothari wrote: [...]
+VBOX_DRIVER_SOURCES = \ + vbox/VBoxXPCOMCGlue.c vbox/VBoxXPCOMCGlue.h \ + vbox/vbox_driver.c vbox/vbox_driver.h \ + vbox/vbox_V2_2.c
Another thing, it would be better to have all the driver filenames start with vbox_ in an uniform way, that's important to be able to easilly put debug filters which are based on filename substring matches. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi Daniel,
+VBOX_DRIVER_SOURCES = \ + vbox/VBoxXPCOMCGlue.c vbox/VBoxXPCOMCGlue.h \ + vbox/vbox_driver.c vbox/vbox_driver.h \ + vbox/vbox_V2_2.c
Another thing, it would be better to have all the driver filenames start with vbox_ in an uniform way, that's important to be able to easilly put debug filters which are based on filename substring matches.
Will fix it, sorry about that. Regards, Pritesh
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Pritesh Kothari