[libvirt] [PATCH] vbox: Handle different IID representation in Version 2.2 on Windows

[Eric Blake]

On 12/23/2010 01:38 PM, Matthias Bolte wrote:
> On Windows IID's are represented as GUID by value, instead of nsID
> by reference on non-Windows platforms.
> 
> Patch the vbox_CAPI_v2_2.h header to deal with this difference.
> 
> Rewrite vboxIID abstraction that deals with the different IID
> representations. Add support for the GUID representation. Also unify
> the four context dependent free functions for vboxIIDs
> 
>   vboxIIDUnalloc, vboxIIDFree, vboxIIDUtf8Free, vboxIIDUtf16Free
> 
> into vboxIIDUnalloc that is now safe to be called (even multiple
> times) on a vboxIID independent of the source and context of the
> vboxIID.

Nice - and it cuts down on a lot of #ifdefs in the code base, as an
added bonus.

> 
> The new vboxIID is designed to be used as a stack allocated variable.
> It has a value member that represents the actual IID value.

> +++ b/src/vbox/vbox_CAPI_v2_2.h
> @@ -57,8 +57,12 @@
>  
>  #  ifdef WIN32
>  #   define PR_COM_METHOD __stdcall
> +#   define PR_IID_IN_TYPE GUID
> +#   define PR_IID_OUT_TYPE GUID *
>  #  else
>  #   define PR_COM_METHOD
> +#   define PR_IID_IN_TYPE const nsID *
> +#   define PR_IID_OUT_TYPE nsID **
>  #  endif

How annoying, but this macro looks like the right way to solve it, and
the bulk of the patch to this file is mechanical.  Also, your (three!)
wrappers in vbox_tmpl.c look sane, and everything after the hunk for
line 1044 of the original file looks pretty mechanical.  I assume you
compiled for both Windows and Linux for both 2.2 and 3.0+ versions.

> @@ -7051,7 +6951,7 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn, const char *
>       */
>  
>  #if VBOX_API_VERSION == 2002
> -    if STREQ(def->name, "vboxnet0") {
> +    if (STREQ(def->name, "vboxnet0")) {

Wow - how long has that been under-parenthesized (I guess since STREQ
parenthesizes its results, it still compiled)?

> @@ -8032,13 +7914,27 @@ static int vboxStorageVolDelete(virStorageVolPtr vol,
>              vboxArrayGet(&machineIds, hardDisk, hardDisk->vtbl->GetMachineIds);
>  #endif /* VBOX_API_VERSION >= 3001 */
>  
> +#if VBOX_API_VERSION == 2002 && defined WIN32
> +            /* VirtualBox 2.2 on Windows represents IIDs as GUIDs and the
> +             * machineIds array contains direct instances of the GUID struct
> +             * instead of pointers to the actual struct instances. But there
> +             * is no 128bit width simple item type for a SafeArray to fit a
> +             * GUID in. The largest simple type it 64bit width and VirtualBox
> +             * uses two of this 64bit items to represents one GUID. Therefore,
> +             * we devide the size of the SafeArray by two, to compensate for
> +             * this workaround in VirtualBox */
> +            machineIds.count /= 2;
> +#endif /* VBOX_API_VERSION >= 2002 */

OK, so that hunk wasn't mechanical, like most of them.  But overall, it
looks good.

ACK.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101223/88c5d8e5/attachment-0001.sig>