On Fri, 2019-04-12 at 14:00 +0100, Daniel P. Berrangé wrote:
> Support for all the 4.x releases was ended by VirtualBox maintainers in
> Dec 2015. Even the "newest" 4.3.40 of those is only supported on old
> versions of Linux (Ubuntu <= 13.03, RHEL <= 6, SLES <= 11), which are all
> discontinued hosts from Libvirt's POV.
s/Libvirt/libvirt/
> We can thus reasonably drop all 4.x support from the libvirt VirtualBox
> driver.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> src/vbox/Makefile.inc.am | 6 -
> src/vbox/vbox_V4_0.c | 37 ------
> src/vbox/vbox_V4_1.c | 37 ------
> src/vbox/vbox_V4_2.c | 13 --
> src/vbox/vbox_V4_2_20.c | 13 --
> src/vbox/vbox_V4_3.c | 13 --
> src/vbox/vbox_V4_3_4.c | 13 --
> src/vbox/vbox_XPCOMCGlue.h | 2 +-
> src/vbox/vbox_common.h | 14 +--
> src/vbox/vbox_storage.c | 14 +--
> src/vbox/vbox_tmpl.c | 218 +---------------------------------
> src/vbox/vbox_uniformed_api.h | 10 +-
> 12 files changed, 8 insertions(+), 382 deletions(-)
> delete mode 100644 src/vbox/vbox_V4_0.c
> delete mode 100644 src/vbox/vbox_V4_1.c
> delete mode 100644 src/vbox/vbox_V4_2.c
> delete mode 100644 src/vbox/vbox_V4_2_20.c
> delete mode 100644 src/vbox/vbox_V4_3.c
> delete mode 100644 src/vbox/vbox_V4_3_4.c
I'm not 100% positive I understand how the vbox driver magic works,
so even if fully ACKed you might want to leave the series on the
list for a few days before pushing.
[...]
> @@ -472,10 +434,9 @@ _deleteConfig(IMachine *machine)
>
> static int _pfnInitialize(vboxDriverPtr driver)
> {
> + nsresult rc;
Maybe leave an empty line here.
[...]
> @@ -1764,14 +1611,6 @@ static nsresult
> _usbCommonEnable(IUSBCommon *USBCommon ATTRIBUTE_UNUSED)
> {
> nsresult rc = 0;
> -#if VBOX_API_VERSION < 4003000
> - USBCommon->vtbl->SetEnabled(USBCommon, 1);
> -# if VBOX_API_VERSION < 4002000
> - rc = USBCommon->vtbl->SetEnabledEhci(USBCommon, 1);
> -# else /* VBOX_API_VERSION >= 4002000 */
> - rc = USBCommon->vtbl->SetEnabledEHCI(USBCommon, 1);
> -# endif /* VBOX_API_VERSION >= 4002000 */
> -#endif /* VBOX_API_VERSION >= 4003000 */
> /* We don't need to set usb enabled for vbox 4.3 and later */
> return rc;
You can ditch 'nsresult rc' entirely and just 'return 0' here.
[...]
> @@ -85,9 +85,9 @@ struct _vboxDriver {
> PCVBOXXPCOM pFuncs;
> IVirtualBox *vboxObj;
> ISession *vboxSession;
> -# if VBOX_API_VERSION == 4002020 || VBOX_API_VERSION >= 4003004
> +#ifdef VBOX_API_VERSION
> IVirtualBoxClient *vboxClient;
> -# endif
> +#endif
You still need a space after '#'s, or syntax-check will
complain.
Everything else looks good.
Personally I would have split this differently: first drop all
of the .c file, as you've done here, then drop all the .h files,
as you do in the subsequent commits, and *only then* drop all
preprocessor-based version checks.
But either way, the result doesn't change so, with at least the
last comment above addressed,
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
