[libvirt] [PATCH] build: define WITH_INTERFACE for the driver

[Osier Yang]

On 2012年06月30日 00:32, Daniel P. Berrange wrote:
> On Fri, Jun 29, 2012 at 10:18:47AM -0600, Eric Blake wrote:
>> On 06/29/2012 01:34 AM, Osier Yang wrote:
>>> On 2012年06月29日 07:57, Eric Blake wrote:
>>>> Our code was mistakenly relying on an undefined macro, WITH_INTERFACE,
>>>> for determining whether to load the interface driver which wraps the
>>>> netcf library.  Clean this situation up by having only one automake
>>>> conditional for the driver, and having both WITH_NETCF (library
>>>> detected) and WITH_INTERFACE (driver enabled) in C code, in case a
>>>> future patch ever adds a network management via means other than
>>
>> s/network/interface/
>>
>>>> the netcf library.
>>>
>>> Foresighted. :-)
>>
>> Trying to model after the storage driver, and how it has several backends.
>>
>>>>
>>>> -dnl netcf library
>>>> +dnl check if the interface driver should be compiled
>>>> +
>>>> +AC_ARG_WITH([interface],
>>>> +  AC_HELP_STRING([--with-interface],
>>>> +    [with host interface driver @<:@default=check@:>@]),[],
>>>> +    [with_interface=check])
>>>
>>> Do we have to expose "with-interface"? It will give the user
>>> a logic question, pick "with-interface", or 'with-netcf', or
>>> both, even more when we have other implementations of interface
>>> driver in future. however, the logic is simple, and we do it
>>> inside actually: as long as one implementation of the interface
>>> driver is picked to compile, we have the WITH_INTERFACE. so IMHO
>>> no need to give the user the simple logic question. :-)
>>
>> Good point.  Looking at how storage did it, we have:
>>
>> --with-storage-dir
>> --with-storage-fs
>> ...
>>
>> but no top-level --with-storage.  That is, you get WITH_STORAGE if any
>> of the --with-storage-backends ended up as yes.
>>
>> At first, I was worried about back-compat (old builds were used to
>> --with-netcf, and I didn't want to break that), but the more I think
>> about it, the more I think that it's okay to break naming conventions
>> for something that is easier to explain.
>>
>> I see two possible solutions, then:
>>
>> 1. Assume that like the storage driver, the interface driver will
>> eventually have multiple backends.  Then we would have:
>>
>> --with-interface-netcf
>>
>> as a way to select the netcf backend in the interface driver, and
>> WITH_INTERFACE would be automatic if at least one backend (in this case,
>> netcf being the only backend) is found.
>>
>> 2. Save the complexity of multiple backends for the day when we actually
>> have multiple backends, and for now just have a single configure option
>> --with-interface.
>>
>> Either way, I would completely ditch --with-netcf, and refactor the
>> logic to be:
>>
>> if test "$with_libvirtd" = no; then
>>      with_interface_netcf=no
>> fi
>> if test "$with_interface_netcf" = yes || \
>>      test "$with_interface_netcf" = check; then
>>      probe for netcf, fail if it was required
>> fi
>> if test "$with_interface_netcf" = yes; then
>>      set WITH_INTERFACE witness
>> fi
>>
>> I'll go ahead and respin this patch along those lines.
>
> I'm not a fan of this, because you are too tightly associating use
> of the netcf library, with use of the interface drivers, and also
> presuming a 1-1 relationship between a logical driver, and an external
> library. THis breaks down if a module like the inteface driver needs
> to check for multiple external libraries, and if the external libraries
> are used by multiple different areas of the libvirt code.

Good point!

>
> My view is that in the configure script, we have two types of checks
> and we must keep them strictly separated.
>
>    - External modules (netcf, lvm, other libraries)
>    - Logical modules  (storage driver, network driver, interface driver)
>
> We should first do checks for the external modules. These checks
> can be disabled/forced using --with-netcf/--without-netcf
>
> The checks for logical modules, should just look to see if
> their all of their prerequisites are present, but again allow
> you to turn off the module using  --with-interface/--without-interface
>
>
> My long term vision is that we one day refactor our enourmous
> configure script into a set of isolated modules.
>
> So, you'd be able to declare logical modules
>
>     AC_DEFUN(LIBVIRT_LIBRARY_NETCF, [
>       ...code to check for netcf and
>          CLI args to enable/disable
>     ])
>
>
>     AC_DEFUN(LIBVIRT_DRIVER_INTERFACE,
>     [
>        AC_REQUIRE([LIBVIRT_DEP_NETCF])
>
>        ...code to enable interface
>           driver if netcf was present
>     ])
>
>     AC_DEFUN(LIBVIRT_DRIVER_STORAGE,
>     [
>        AC_REQUIRE([LIBVIRT_DEP_LVM])
>        AC_REQUIRE([LIBVIRT_DEP_QEMU_IMG])
>        AC_REQUIRE([LIBVIRT_DEP_ISCSI])
>
>        ...code to enable storage
>           driver parts...
>     ])
>

This model much more flexible and modularized. It seperates the
definition of library and module. We will need this as current
configure script is a bit disorderly.

Regards,
Osier