Re: [PATCH v2 1/7] vbox: drop support for VirtualBox 4.x releases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux