2010/12/23 Eric Blake <eblake@xxxxxxxxxx>:
> 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.
Yes, this is compile tested on Linux and Windows. I did some runtime
tests on Windows and Linux for 2.2 and 3.2. So I'm quite confident
that I didn't break it :)
>> @@ -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.
This one took quite a while digging the VirtualBox source code until I
figured out why deleting a volume didn't work :)
> ACK.
>
Thanks, pushed.
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list