Sure, will do. Thanks for the comments. Best, Yifeng
On Tue, Nov 6, 2018 at 10:04 AM Ben Pfaff <[email protected]> wrote: > On Tue, Nov 06, 2018 at 09:48:36AM -0800, Yifeng Sun wrote: > > On Mon, Nov 5, 2018 at 7:58 PM Ben Pfaff <[email protected]> wrote: > > > > > [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 <[email protected]> > > > > Suggested-by: Mark Michelson <[email protected]> > > > > Signed-off-by: Yifeng Sun <[email protected]> > > > > --- > > > > 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? > > > > > > > It is a fix based on your previous suggestion: DNS resolving should stop > > if there is no DNS server configured. Do you think we should create > another > > patch only for this fix? > > Oh, I did not realize that this was the change that implemented that. > > If it is not troublesome, then I would divide this into two patches, > because that would make it clear which part of the patch implements each > change. > > Thanks for the other responses too. > > Thanks, > > Ben. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
