[libvirt] [PATCH 2/2] VirtualBox support to libvirt

[Pritesh Kothari]

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