[libvirt] [PATCH] configure: Remove build time checks for (ip|ip6|eb)tables

[Cole Robinson]

On 04/22/2016 09:18 AM, Andrea Bolognani wrote:
> On Thu, 2016-04-21 at 17:06 -0400, Cole Robinson wrote:
>> And the 'ip' tool. There isn't much benefit to checking this at
>> configure time when we have infrastructure nowadays for looking up
>> binaries in the PATH
>>  
>> https://bugzilla.redhat.com/show_bug.cgi?id=661262
>> ---
>>   configure.ac            | 12 ------
>>   src/util/virfirewall.c  | 18 +++++----
>>   src/util/virnetdev.c    |  6 +--
>>   tests/virfirewalltest.c | 98 ++++++++++++++++++++++++-------------------------
>>   4 files changed, 62 insertions(+), 72 deletions(-)
> 
> I haven't tried running this so I'm probably missing
> something, but...
> 
>> @@ -182,17 +182,19 @@ virFirewallValidateBackend(virFirewallBackend backend)
>>   
>>       if (backend == VIR_FIREWALL_BACKEND_DIRECT) {
>>           const char *commands[] = {
>> -            IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH
>> +            "iptables", "ip6tables", "ebtables"
>>           };
>>           size_t i;
>>   
>>           for (i = 0; i < ARRAY_CARDINALITY(commands); i++) {
>> -            if (!virFileIsExecutable(commands[i])) {
>> +            char *path = virFindFileInPath(commands[i]);
>> +            if (!path) {
>>                   virReportSystemError(errno,
>>                                        _("direct firewall backend requested, but %s is not available"),
>>                                        commands[i]);
>>                   return -1;
>>               }
>> +            VIR_FREE(path);
>>           }
>>           VIR_DEBUG("found iptables/ip6tables/ebtables, using direct backend");
>>       }
> 
> ... how is this fixing the issue reported above?
> 

Oh, hmm, maybe it doesn't, sorry. I was misreading; I thought the report was
'build libvirtd without iptables, install it later, libvirt won't work'.

> AFAICT you just changed it to perform a filesystem lookup instead
> of relying on the information obtained at configure time. And you
> removed the check on the file being executable, which is probably
> not a good idea?

Judging from the error message it seems like virFileIsExecutable was just a
surrogate for access(path, F_OK), but I can re add it. As long as someone at
least thinks this is a worthwhile patch otherwise

- Cole