[libvirt] [PATCH 2/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:50PM +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.
I've not got a VirtualBox installation to test with yet, but I've hit one small problem this patch. If you don't have any vbox install present, then the vboxRegister() method returns -1, causing entire of virInitialize() method to retturn -1 error, and thus non of libvirt works at all.
diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c new file mode 100644 index 0000000..b484abc --- /dev/null +++ b/src/vbox/vbox_driver.c @@ -0,0 +1,53 @@ +/** @file vbox_driver.c + * Core driver methods for managing VirtualBox VM's + */ + +#include <config.h> + +#include "internal.h" + +#include "datatypes.h" +#include "logging.h" +#include "vbox_driver.h" +#include "VBoxXPCOMCGlue.h" + +#define VIR_FROM_THIS VIR_FROM_VBOX + + +extern virDriver vbox22Driver; +extern virDriver vbox25Driver; + + +int vboxRegister(void) { + virDriver *driver; + uint32_t uVersion = 0; + uint32_t major = 0; + uint32_t minor = 0; + uint32_t intVer = 0; + uint32_t micro = 0; + + if (VBoxCGlueInit() != 0) + return -1; + + if (g_pVBoxFuncs == NULL) + return -1;
These two should just return 0, and then the 'open' method should return VIR_ERR_DECLINED if called with a virtualbox URI.
+ + uVersion = g_pVBoxFuncs->pfnGetVersion(); + major = uVersion / 1000000; + intVer = uVersion % 1000000; + minor = intVer / 1000; + micro = intVer % 1000; + + DEBUG("VBoxCGlueInit worked for version: %d.%d.%d", major, minor, micro); + + /* select driver implementation based on version. */ + if ((major == 2) && ((minor == 1)||(minor == 2)) ) + driver = &vbox22Driver; + else + return -1;
Likewise, this should just return 0;
+ + if (virRegisterDriver(driver) < 0) + return -1;
This one is OK, because it is indicating a real fatal error in the virRegisterDriver() method.
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c new file mode 100644 index 0000000..bb5a812 --- /dev/null +++ b/src/vbox/vbox_tmpl.c @@ -0,0 +1,2337 @@ +/** @file vbox_tmpl.c + * Template File to support multiple versions of VirtualBox + * at runtime :).
Can you explain a little about this idea... & how it works
+ * + * IMPORTANT: + * Please dont include this file in the src/Makefile.am, it + * is automatically include by other files. + */ + +#include <config.h> + +#include <dlfcn.h> +#include <sys/utsname.h> + +#include "internal.h" + +#include "datatypes.h" +#include "domain_conf.h" +#include "virterror_internal.h" +#include "uuid.h" +#include "memory.h" +#include "nodeinfo.h" +#include "logging.h" +#include "vbox_driver.h" +#include "VBoxXPCOMCGlue.h" + +/* This one changes from version to version. */ +#if VBOX_API_VERSION == 2002 +# include "VBoxCAPI_v2_2.h" +/* Commented for now, v2.5 is far far away */ +/* +#elif VBOX_API_VERSION == 2005 +# include "VBoxCAPI_v2_5.h" +*/ +#endif + +#define VIR_FROM_THIS VIR_FROM_VBOX + +#define vboxError(conn, code, fmt...) \ + virReportErrorHelper(conn, VIR_FROM_VBOX, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) + +struct vbox_driver { + virMutex lock; + int version; + virDomainObjList domains; + virCapsPtr caps; +}; + + +/******************************************************************************* +* Global VirtualBox and ISession Objects * +*******************************************************************************/ +static IVirtualBox *g_vboxObj = NULL; +static ISession *g_vboxSession = NULL;
What are the thread-safety requirements/guarrentees of these objects ? Are they fully thread safe, or do they require caller to maintain exclusive locking before using them. I'm hoping the later, since the driver code appears to call them without taking out locks. Ideally I'd prefer to see these two objects in the general driver state 'struct vbox_driver' - in the perfect world the only static variable is the instance of the 'struct vbox_driver' object.
+virDriver NAME(Driver) = { + VIR_DRV_VBOX, + "VBOX", + .open = vboxOpen, + .close = vboxClose, + .supports_feature = NULL, + .type = vboxGetType, + .version = vboxGetVersion, + .getHostname = vboxGetHostname, + .getURI = NULL, + .getMaxVcpus = NULL, + .nodeGetInfo = NULL, + .getCapabilities = vboxGetCapabilities, + .listDomains = vboxListDomains, + .numOfDomains = vboxNumOfDomains, + .domainCreateXML = NULL, + .domainLookupByID = vboxDomainLookupByID, + .domainLookupByUUID = vboxDomainLookupByUUID, + .domainLookupByName = vboxDomainLookupByName, + .domainSuspend = vboxDomainSuspend, + .domainResume = vboxDomainResume, + .domainShutdown = vboxDomainShutdown, + .domainReboot = vboxDomainReboot, + .domainDestroy = vboxDomainDestroy, + .domainGetOSType = vboxDomainGetOSType, + .domainGetMaxMemory = NULL, + .domainSetMaxMemory = NULL, + .domainSetMemory = NULL, + .domainGetInfo = vboxDomainGetInfo, + .domainSave = NULL, + .domainRestore = NULL, + .domainCoreDump = NULL, + .domainSetVcpus = NULL, + .domainPinVcpu = NULL, + .domainGetVcpus = NULL, + .domainGetMaxVcpus = NULL, + .domainDumpXML = NULL, + .listDefinedDomains = vboxListDefinedDomains, + .numOfDefinedDomains = vboxNumOfDefinedDomains, + .domainCreate = vboxDomainCreate, + .domainDefineXML = vboxDomainDefineXML, + .domainUndefine = vboxDomainUndefine, + .domainAttachDevice = NULL, + .domainDetachDevice = NULL, + .domainGetAutostart = NULL, + .domainSetAutostart = NULL, + .domainGetSchedulerType = NULL, + .domainGetSchedulerParameters = NULL, + .domainSetSchedulerParameters = NULL, + .domainMigratePrepare = NULL, + .domainMigratePerform = NULL, + .domainMigrateFinish = NULL, + .domainBlockStats = NULL, + .domainInterfaceStats = NULL, + .domainBlockPeek = NULL, + .domainMemoryPeek = NULL, + .nodeGetCellsFreeMemory = NULL, + .getFreeMemory = NULL, + .domainEventRegister = NULL, + .domainEventDeregister = NULL, + .domainMigratePrepare2 = NULL, + .domainMigrateFinish2 = NULL, +};
If you're planning next steps, my recommendation would be to try and get the 'domainDumpXML', 'domainCreateXML' and 'nodeGetInfo' APIs working. With those done, I think you'd be pretty close to being able to try running this driver in virt-manager / virt-install for provisioning new guests 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 :|
Hi Daniel,
These two should just return 0, and then the 'open' method should return VIR_ERR_DECLINED if called with a virtualbox URI.
+ else + return -1; Likewise, this should just return 0;
oops sorry, will change that.
+/** @file vbox_tmpl.c + * Template File to support multiple versions of VirtualBox + * at runtime :).
Can you explain a little about this idea... & how it works
First I will give the problem being faced and then how I tried to solve it. Problem: I want to support "All" VirtualBox versions after and including version 2.2 at "Runtime". Now since VirtualBox adds Multiple new features for each release, it is but natural that the C API which I use is volatile across versions and thus I need a good mechanism to handle multiple versions during runtime. My solution was something like this: I have a single template file called vbox_tmpl.c which is included multiple times during compilation using some pre-processor magic, for example: /* #define NAME(name) vbox22##name */ vbox_V2_2.c includes vbox_tmpl.c but renames the driver as vbox22Driver, similarly when i add support for 2.5 i would have /* #define NAME(name) vbox25##name */ vbox_V2_5.c which would include vbox_tmpl.c while renaming the driver as vbox25Driver. This would make sure that it would include the appropriate header files namely VBoxCAPI_v2_2.h and VBoxCAPI_v2_5.h respectively. The Objects which I depend on, basically the IVirtualBox and ISession are defined in these headers and change for each version depending on the functionality. These Objects are nothing but structs with function pointers in them (vtables equivalent of C++) and it would be disaster if i try to access them if I don't have the right version and thus the crazy stuff above to circumvent it. Hope it is clear now, or else I would try to explain it in more detail again.
+static IVirtualBox *g_vboxObj = NULL; +static ISession *g_vboxSession = NULL;
What are the thread-safety requirements/guarrentees of these objects ?
Are they fully thread safe, or do they require caller to maintain exclusive locking before using them. I'm hoping the later, since the driver code appears to call them without taking out locks.
These two objects are fully thread safe, the code which you see was put there by me to check few other things in the initial stage of the development, but I guess it is not required any more.
Ideally I'd prefer to see these two objects in the general driver state 'struct vbox_driver' - in the perfect world the only static variable is the instance of the 'struct vbox_driver' object.
Yes, I agree with you, but just for making life simpler in initial stages I had put them there, will put them back in their rightful places. :)
If you're planning next steps, my recommendation would be to try and get the 'domainDumpXML', 'domainCreateXML' and 'nodeGetInfo' APIs working. With those done, I think you'd be pretty close to being able to try running this driver in virt-manager / virt-install for provisioning new guests
I am currently working on 'domainCreateXML' and then it would be 'domainDumpXML' and 'nodeGetInfo', so would submit another patch when i complete it with above changes in it. Thanks. Regards, Pritesh
On Tue, Mar 24, 2009 at 09:51:20AM +0100, Pritesh Kothari wrote:
+/** @file vbox_tmpl.c + * Template File to support multiple versions of VirtualBox + * at runtime :).
Can you explain a little about this idea... & how it works
First I will give the problem being faced and then how I tried to solve it.
Problem: I want to support "All" VirtualBox versions after and including version 2.2 at "Runtime".
Now since VirtualBox adds Multiple new features for each release, it is but natural that the C API which I use is volatile across versions and thus I need a good mechanism to handle multiple versions during runtime. My solution was something like this:
I have a single template file called vbox_tmpl.c which is included multiple times during compilation using some pre-processor magic, for example:
I hit that last Friday while starting to review the patch an found that a bit strange.
/* #define NAME(name) vbox22##name */ vbox_V2_2.c includes vbox_tmpl.c but renames the driver as vbox22Driver, similarly when i add support for 2.5 i would have /* #define NAME(name) vbox25##name */ vbox_V2_5.c which would include vbox_tmpl.c while renaming the driver as vbox25Driver. This would make sure that it would include the appropriate header files namely VBoxCAPI_v2_2.h and VBoxCAPI_v2_5.h respectively.
The Objects which I depend on, basically the IVirtualBox and ISession are defined in these headers and change for each version depending on the functionality. These Objects are nothing but structs with function pointers in them (vtables equivalent of C++) and it would be disaster if i try to access them if I don't have the right version and thus the crazy stuff above to circumvent it.
Hope it is clear now, or else I would try to explain it in more detail again.
That's probably worth a README in the vbox/ directory IMHO I guess Dan did a more complete review of the code than mine. One of the issues I had was many pieces of code licenced under the MIT Licence, which is compatible with the LGPL, but I must admit that if you are the authors of that code I would prefer just LGPL. I also saw parts which were MIT but allowed to relicence under LGPL, for the sake of uniformity I would prefer all files provided under LGPLv2 like the other parts of libvirt code. In a sense it's equivalent for you, but for people doing the legal review when trying to embbed libvirt this makes things more complicated. Any chance you could fix all those headers ? thanks, 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,
I have a single template file called vbox_tmpl.c which is included multiple times during compilation using some pre-processor magic, for example:
I hit that last Friday while starting to review the patch an found that a bit strange.
I couldn't think of any other cleaner solution, if there is any suggestion regarding it, I can check that out.
That's probably worth a README in the vbox/ directory IMHO
Will do that.
I guess Dan did a more complete review of the code than mine. One of the issues I had was many pieces of code licenced under the MIT Licence, which is compatible with the LGPL, but I must admit that if you are the authors of that code I would prefer just LGPL. I also saw parts which were MIT but allowed to relicence under LGPL, for the sake of uniformity I would prefer all files provided under LGPLv2 like the other parts of libvirt code. In a sense it's equivalent for you, but for people doing the legal review when trying to embbed libvirt this makes things more complicated. Any chance you could fix all those headers ?
I have fixed all headers to be LGPLv2.1 :) Thanks Pritesh
On Tue, Mar 24, 2009 at 05:33:21PM +0100, Pritesh Kothari wrote:
I guess Dan did a more complete review of the code than mine. One of the issues I had was many pieces of code licenced under the MIT Licence, which is compatible with the LGPL, but I must admit that if you are the authors of that code I would prefer just LGPL. I also saw parts which were MIT but allowed to relicence under LGPL, for the sake of uniformity I would prefer all files provided under LGPLv2 like the other parts of libvirt code. In a sense it's equivalent for you, but for people doing the legal review when trying to embbed libvirt this makes things more complicated. Any chance you could fix all those headers ?
I have fixed all headers to be LGPLv2.1 :)
Ah, that's good news :-) 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/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Pritesh Kothari