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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
