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

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.