2013/12/31 Jean-Baptiste Rouault <jean-baptiste.rouault@diateam.net>:
On Monday 30 December 2013 11:26:08 Ryota Ozaki wrote:
On Mon, Dec 30, 2013 at 5:55 PM, Jean-Baptiste Rouault
<jean-baptiste.rouault@diateam.net> wrote:
On Sunday 29 December 2013 14:44:10 Ryota Ozaki wrote:
On Wed, Dec 25, 2013 at 12:47 AM, Jean-Baptiste Rouault
<jean-baptiste.rouault@diateam.net> wrote:
While working on adding virDomain*Stats support to the vbox driver, we found bugs in the VirtualBox API C bindings. These bugs have been fixed in versions 4.2.20 and 4.3.4. However, the changes in the C bindings are incompatible with the vbox_CAPI_v4_2.h and vbox_CAPI_v4_3.h files which are bundled in libvirt source code. This is why the following patch adds vbox_CAPI_v4_2_20.h and vbox_CAPI_v4_3_4.h.
We tried to keep compatibility with older VirtualBox 4.2.x and 4.3.x releases so we added a "SPECIAL_VERSION" identifier to conditionnaly include the right header. I'm not really pleased with this "SPECIAL_VERSION" identifier, maybe we could instead increase the precision of "VBOX_API_VERSION", for example 4002 would become 4002000. This would permit us to select the right header based on the VBOX_API_VERSION only, what do you think ?
Can we use VBOX_XPCOMC_VERSION instead of adding a new flag? The version has been bumped up when the incompatibility is introduced.
ozaki-r
The problem is that VBOX_XPCOMC_VERSION is defined in the vbox_CAPI_v*.h headers and we need a flag to choose which header we have to include.
Oops. You're right.
Well, one other idea is to include each vbox_CAPI_X_Y.h in the corresponding vbox_VX_Y.c. That's rather straightforward for me than including vbox_CAPI_*.h in vbox_tmpl.c according to VBOX_API_VERSION.
ozaki-r
This would indeed solve the problem for header inclusion. But what about future code using the new API ? Will it have to check both VBOX_API_VERSION and VBOX_XPCOMC_VERSION ? Wouldn't it be simpler if VBOX_API_VERSION was more precise ? e.g 4003004
Okay, so the actual underlying problem here is that libvirt assumes that VirtualBox API can only change between minor versions (4.2 -> 4.3), but we have a case here where it changed (or got fixed) between release version (4.2.19 -> 4.2.20). Using this new SPECIAL_VERSION define is the least invasive fix for the problem. The more correct solution would be to make VBOX_API_VERSION represent the full API version number, instead of just the major and minor part. This would fix the incorrect assumption in libvirt. -- Matthias Bolte http://photron.blogspot.com