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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

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?

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]