[libvirt] [PATCH] vbox: add support for v4.2.20+ and v4.3.4+
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 ? Jean-Baptiste Rouault (1): vbox: add support for v4.2.20+ and v4.3.4+ src/Makefile.am | 4 +- src/vbox/vbox_CAPI_v4_2_20.h | 9001 +++++++++++++++++++++++++++++++++++ src/vbox/vbox_CAPI_v4_3_4.h | 10321 +++++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_V2_2.c | 1 + src/vbox/vbox_V3_0.c | 1 + src/vbox/vbox_V3_1.c | 1 + src/vbox/vbox_V3_2.c | 1 + src/vbox/vbox_V4_0.c | 1 + src/vbox/vbox_V4_1.c | 1 + src/vbox/vbox_V4_2.c | 1 + src/vbox/vbox_V4_2_20.c | 14 + src/vbox/vbox_V4_3.c | 1 + src/vbox/vbox_V4_3_4.c | 14 + src/vbox/vbox_driver.c | 20 +- src/vbox/vbox_tmpl.c | 12 +- 15 files changed, 19389 insertions(+), 5 deletions(-) create mode 100644 src/vbox/vbox_CAPI_v4_2_20.h create mode 100644 src/vbox/vbox_CAPI_v4_3_4.h create mode 100644 src/vbox/vbox_V4_2_20.c create mode 100644 src/vbox/vbox_V4_3_4.c -- 1.8.5.1
On 12/24/2013 08:47 AM, Jean-Baptiste Rouault 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 ?
Jean-Baptiste Rouault (1): vbox: add support for v4.2.20+ and v4.3.4+
src/Makefile.am | 4 +- src/vbox/vbox_CAPI_v4_2_20.h | 9001 +++++++++++++++++++++++++++++++++++ src/vbox/vbox_CAPI_v4_3_4.h | 10321 +++++++++++++++++++++++++++++++++++++++++
This patch is HUGE (620k, so the moderation queue is currently holding it as oversized, compared to the normal 150k limit). Is there any way to break it into smaller pieces, or compress it before sending to the list, or merely point to an external repo containing the patch, so that we aren't chewing up lots of bandwidth on mostly mechanical code? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tuesday 24 December 2013 17:00:19 Eric Blake wrote:
On 12/24/2013 08:47 AM, Jean-Baptiste Rouault 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 ?
Jean-Baptiste Rouault (1): vbox: add support for v4.2.20+ and v4.3.4+
src/Makefile.am | 4 +- src/vbox/vbox_CAPI_v4_2_20.h | 9001 +++++++++++++++++++++++++++++++++++ src/vbox/vbox_CAPI_v4_3_4.h | 10321 +++++++++++++++++++++++++++++++++++++++++
This patch is HUGE (620k, so the moderation queue is currently holding it as oversized, compared to the normal 150k limit). Is there any way to break it into smaller pieces, or compress it before sending to the list, or merely point to an external repo containing the patch, so that we aren't chewing up lots of bandwidth on mostly mechanical code?
The patch is available at http://git-lab.diateam.net/cots/libvirt.git/ The branch is named "vbox-4.2.20-4.3.4-support" Regards, -- Jean-Baptiste ROUAULT R&D Engineer - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051
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
Jean-Baptiste Rouault (1): vbox: add support for v4.2.20+ and v4.3.4+
src/Makefile.am | 4 +- src/vbox/vbox_CAPI_v4_2_20.h | 9001 +++++++++++++++++++++++++++++++++++ src/vbox/vbox_CAPI_v4_3_4.h | 10321 +++++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_V2_2.c | 1 + src/vbox/vbox_V3_0.c | 1 + src/vbox/vbox_V3_1.c | 1 + src/vbox/vbox_V3_2.c | 1 + src/vbox/vbox_V4_0.c | 1 + src/vbox/vbox_V4_1.c | 1 + src/vbox/vbox_V4_2.c | 1 + src/vbox/vbox_V4_2_20.c | 14 + src/vbox/vbox_V4_3.c | 1 + src/vbox/vbox_V4_3_4.c | 14 + src/vbox/vbox_driver.c | 20 +- src/vbox/vbox_tmpl.c | 12 +- 15 files changed, 19389 insertions(+), 5 deletions(-) create mode 100644 src/vbox/vbox_CAPI_v4_2_20.h create mode 100644 src/vbox/vbox_CAPI_v4_3_4.h create mode 100644 src/vbox/vbox_V4_2_20.c create mode 100644 src/vbox/vbox_V4_3_4.c
-- 1.8.5.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. -- Jean-Baptiste ROUAULT R&D Engineer - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051
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
-- Jean-Baptiste ROUAULT R&D Engineer - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051
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 -- Jean-Baptiste ROUAULT R&D Engineer - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051
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
participants (4)
-
Eric Blake -
Jean-Baptiste Rouault -
Matthias Bolte -
Ryota Ozaki