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

On 06/29/2012 01:34 AM, Osier Yang wrote: s/network/interface/ Trying to model after the storage driver, and how it has several backends. 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. Yep, we're thinking on the same lines - probe for each backend, then make the driver decision based on the result of the backend probes, but only expose the backends as the configure options. Actually, no with_netcf here (this is the driver section, but with_netcf is already present in the library section), so we DO want a listing here of whether the with_interface driver was selected (whether by with_netcf or by some other backend). The logic here would be a bit different; just as the spec file has to know when to package the storage driver (if any of the storage backends were selected), we still have to package the interface driver, so this variable is still useful. Agreed - the netcf backend check should be sufficient here. The driver frontend should still be interface_driver.c. Rather, if we create a new backend, we would end up moving half the code into a new file interface_backend_netcf.c, as well as adding a new interface_backend_foo.c, where both backends are driven by interface_driver.c. But I agree that we should save that for a future day if we ever add another backend. Actually, this is useful - we want to list all the drivers, as well as all the backends for each driver. 'Network', 'Interface', and 'Nwfilter' are drivers, while... 'Bridging', 'Netcf', and 'VirtualPort' are backends (not all for the same driver, but hopefully this is helping make the point). -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org