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

Re: [libvirt] [PATCHv3 1/2] build: define WITH_INTERFACE for the driver



On 09/17/2012 07:27 PM, Doug Goldstein wrote:
> Based exclusively on work by Eric Blake in a patch posted with the same
> subject. However some modifications related to comments and my plans to
> add another backend.
> 
> Added WITH_INTERFACE as the only automake variable deciding whether to
> build the driver and using WITH_NETCF to identify that we're wanting to
> use the netcf library as the backend.
> 
> * configure.ac: Added with_interface
> * src/interface/netcf_driver.c: Renamed..
> * src/interface/interface_backend_netcf.c: ..to this to match storage.
> * src/interface/netcf_driver.h: Renamed..
> * src/interface/interface_driver.h: ..to this.
> * daemon/Makefile.am: Respect WITH_INTERFACE and WITH_NETCF.
> * libvirt.spec.in: Add RPM support for --with-interface

'make syntax-check' says you missed a change to po/POTFILES.in.

> ---
> 
> Change from v2:
> * rebase against master
> * v2 conditionally ACK'd by Laine if Eric ACK'd it

Apologies for not having enough time to review this before rc1, but
let's get it in for rc2.

> +++ b/configure.ac
> @@ -1948,7 +1948,6 @@ AM_CONDITIONAL([WITH_NETCF], [test "$with_netcf" = "yes"])
>  AC_SUBST([NETCF_CFLAGS])
>  AC_SUBST([NETCF_LIBS])
>  
> -
>  AC_ARG_WITH([secrets],

Spurious whitespace change.

> +++ b/libvirt.spec.in
> @@ -92,6 +92,7 @@
>  # A few optional bits off by default, we enable later
>  %define with_polkit        0%{!?_without_polkit:0}
>  %define with_capng         0%{!?_without_capng:0}
> +%define with_interface     0%{!?_without_interface:0}
>  %define with_netcf         0%{!?_without_netcf:0}
>  %define with_udev          0%{!?_without_udev:0}
>  %define with_hal           0%{!?_without_hal:0}
> @@ -200,6 +201,12 @@
>  %define with_netcf     0%{!?_without_netcf:%{server_drivers}}
>  %endif
>  
> +# interface is the driver that wraps netcf or udev interface management
> +# backends in Fedora 18 / RHEL-7 or newer

Not quite right.  It wraps the netcf interface management in all
versions of Fedora.

> +%if 0%{?fedora} >= 18 || 0%{?rhel} >= 7
> +%define with_interface     0%{!?_without_interface:%{server_drivers}}
> +%endif

So I think a better condition is to define with_interface to 1 if
with_netcf is defined; then when you add a udev backend, define
with_interface to 1 if either with_netcf or with_udev is defined.

>  
> -%if %{with_netcf}
> -%define with_interface 1
> -%else
> -%define with_interface 0
> -%endif

That is, this code already looked like it was doing the right thing.

I think I can fix these things and push the amended version soon, but I
ran out of time today.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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