On 12/8/22 09:18, Frode Nordahl wrote: > Commit 08e9e5337383 fixed proper initialization of the dns-resolve > module, and made DNS resolution asynchronous. > > A side effect of that change revealed a long standing logic bug > which broke ovsdb-server listener configuration using DNS names.
It's kind of strange to listen on a DNS name. But, OK, if it worked, then we should fix it. General comment: Might be better to squash this patch set into a single patch. It would be easier to review. And all the patches are tightly coupled anyway. See some comments inline. Best regards, Ilya Maximets. > > Reported-at: https://bugs.launchpad.net/bugs/1998781 > Fixes: 08e9e5337383 ("ovsdb: raft: Fix inability to read the database with > DNS host names.") > Fixes: 771680d96fb6 ("DNS: Add basic support for asynchronous DNS resolving") > Signed-off-by: Frode Nordahl <[email protected]> > --- > ovsdb/jsonrpc-server.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c > index 916a1f414..1e92785a6 100644 > --- a/ovsdb/jsonrpc-server.c > +++ b/ovsdb/jsonrpc-server.c > @@ -267,9 +267,25 @@ ovsdb_jsonrpc_server_add_remote(struct > ovsdb_jsonrpc_server *svr, > int error; > > error = jsonrpc_pstream_open(name, &listener, options->dscp); > - if (error && error != EAFNOSUPPORT) { > - VLOG_ERR_RL(&rl, "%s: listen failed: %s", name, ovs_strerror(error)); > - return NULL; > + if (error) { > + switch (error) { > + case EAFNOSUPPORT: > + /* Not a listener, attempt creation of active jsonrpc sesssion */ Period at the end of a sentence. > + break; Cases should be separated by an empty line. > + case ENODATA: > + /* DNS resolution in progress, or the name does currently not > + * resolve, need to try again later */ Not sure if we need to repeat the same thing that we're printing in the log. The non-duplicated information here is only that we're going to try again. And a period at the end of a sentence. > + VLOG_DBG_RL(&rl, > + "%s: listen failed: DNS resolution in progress" > + "or host not found", There is a missing space between the 'progress' and 'or'. It might be better to split this way: VLOG_DBG_RL(&rl, "%s: listen failed: " "DNS resolution in progress or host not found", name); Or even: VLOG_DBG_RL(&rl, "%s: listen failed: " "DNS resolution in progress or host not found", name); if we remove the 'if' and save an indentation level. See below. > + name);> + return NULL; > + default: > + VLOG_ERR_RL(&rl, "%s: listen failed: %s", name, > + ovs_strerror(error)); > + return NULL; > + } > + /* FALL THROUGH */ Might be better to remove the 'if', handle the 'case 0' explicitly and not have this fall-through comment, as it is a bit cryptic. > } > > remote = xmalloc(sizeof *remote); _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
