On 04/22/2016 10:14 AM, Laine Stump wrote: > On 04/22/2016 09:42 AM, Cole Robinson wrote: >> 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 > > I think it's worthwhile; even though the number of self-builders is fairly > low, they do take time sorting out on IRC. I agree with Andrea that the > virFileIsExecutable call should remain in, since you can never count on one of > these self-built systems to have *anything* setup sanely :-P > > Once this change is made, I think we can remove all the "BuildRequires: > (ebtables/iptables/iptables-ipv6)" from the specfile (as long as there is no > other odd usage of them in configure.ac) > Okay I'll look into it Thanks, Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list