[CCing Alin for his opinion on Windows issue] On Mon, Nov 05, 2018 at 04:20:18PM -0800, Yifeng Sun wrote: > DNS resolution should fail if no DNS servers are available. This > patch fixes it and also enables users to use environment variable > OVS_RESOLV_CONF to specify the path for DNS server configuration > file. > > Suggested-by: Ben Pfaff <b...@ovn.org> > Suggested-by: Mark Michelson <mmich...@redhat.com> > Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> > --- > lib/dns-resolve.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/lib/dns-resolve.c b/lib/dns-resolve.c > index 3c6d70e8fbba..60757cb1eb8a 100644 > --- a/lib/dns-resolve.c > +++ b/lib/dns-resolve.c > @@ -83,14 +83,26 @@ dns_resolve_init(bool is_daemon) > } > > #ifdef __linux__ > - const char *filename = "/etc/resolv.conf"; > + const char *filename = getenv("OVS_RESOLV_CONF"); > + if (filename == NULL) { > + filename = "/etc/resolv.conf"; > + } > struct stat s; > if (!stat(filename, &s) || errno != ENOENT) { > int retval = ub_ctx_resolvconf(ub_ctx__, filename); > if (retval != 0) { > VLOG_WARN_RL(&rl, "Failed to read %s: %s", > filename, ub_strerror(retval)); > + ub_ctx_delete(ub_ctx__); > + ub_ctx__ = NULL; > + return; > } > + } else { > + VLOG_WARN_RL(&rl, "Failed to read %s: %s", > + filename, ovs_strerror(errno)); > + ub_ctx_delete(ub_ctx__); > + ub_ctx__ = NULL; > + return; > } > #endif
Thanks for the improvement. It spurs a few thoughts. It looks like part of this may be a somewhat independent bug fix because, previously, ub_ctx__ was not deleted or set to NULL on error. Is that part of this patch also a bug fix? This looks a little unfair to Windows. I think that ub_ctx_resolvconf() works on Windows if you supply a file name, or even if you pass NULL to use the default Windows resolver parameters. I think that the overall logic, then, could more fairly to Windows be something like this: const char *filename = getenv("OVS_RESOLV_CONF"); if (!filename) { #ifdef _WIN32 /* On Windows, NULL means to use the system default nameserver. */ #else filename = "/etc/resolv.conf"; #endif } The documentation for ub_ctx_resolvconf() is here: https://linux.die.net/man/3/ub_ctx_resolvconf (If we use this implementation, we need to be careful about error messages, because NULL shouldn't be used for %s.) The new environment variable should be documented in some appropriate place. Thanks a lot! _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev