[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/4] VirtualBox support



On Thu, Apr 16, 2009 at 04:18:58PM +0200, 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. It takes cares of all the stuff mentioned on 
> the list earlier. Still if I have missed anything, please do tell me.

I actually just tried out your previous patch from 2 days ago and it
worked without trouble, so I reckon we can plan to get this driver
in the forthcoming  release next week.


> +static virCapsPtr vboxCapsInit(void) {
> +    struct utsname utsname;
> +    virCapsPtr caps;
> +    virCapsGuestPtr guest;
> +
> +    uname(&utsname);
> +
> +    if ((caps = virCapabilitiesNew(utsname.machine,
> +                                   0, 0)) == NULL)
> +        goto no_memory;
> +
> +    if (virCapsInitNUMA(caps) < 0)
> +        goto no_memory;
> +
> +    virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x08, 0x00, 0x27 });
> +
> +    if ((guest = virCapabilitiesAddGuest(caps,
> +                                         "hvm",
> +                                         utsname.machine,
> +                                         sizeof(int) == 4 ? 32 : 64,

I was wondering why the capabilities said '32' as wordsize
even on x86_64, and of course this is because 'int' is still
32 bits on x86_64.   I'd switch to 'sizeof(size_t)' instead
unless someone has better suggestions for determining the
native arch wordsize in a portable manner.


> +static const char *vboxGetType(virConnectPtr conn ATTRIBUTE_UNUSED) {
> +    DEBUG("%s: in vboxGetType",conn->driver->name);
> +    return strdup("VBox");
> +}

This shouldn't strdup the type - the returned string is const,
not to be free'd by caller. 

Even better just remove this method entirely. I don't know why
we have this as a driver method at all. The default impl in
src/libvirt.c already does the correct thing, returning the
conn->driver->name string.

We should remove getType from all our driver impls.



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]