2010/12/23 Eric Blake <ebl...@redhat.com>: > 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list