[libvirt] [PATCH] vbox: add support for v4.2.20+ and v4.3.4+

[Matthias Bolte]

2013/12/31 Jean-Baptiste Rouault <jean-baptiste.rouault at 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 at 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 at 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