On 7/15/21 7:37 PM, Han Zhou wrote: > On Thu, Jul 15, 2021 at 9:17 AM Ilya Maximets <i.maxim...@ovn.org> wrote: >> >> On 6/29/21 9:57 PM, Ilya Maximets wrote: >>>>> Regarding the current patch, I think it's better to add a test case to >>>>> cover the scenario and confirm that existing connections didn't > reset. With >>>>> that: >>>>> Acked-by: Han Zhou <hz...@ovn.org> >>> >>> I'll work on a unit test for this. >> >> Hi. Here is a unit test that I came up with: >> >> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at >> index 62181dd4d..e32f9ec89 100644 >> --- a/tests/ovsdb-idl.at >> +++ b/tests/ovsdb-idl.at >> @@ -2282,3 +2282,27 @@ OVSDB_CHECK_CLUSTER_IDL_C([simple idl, > monitor_cond_since, cluster disconnect], >> 008: table simple: i=1 r=2 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=<2> >> 009: done >> ]]) >> + >> +dnl This test checks that IDL keeps the existing connection to the > server if >> +dnl it's still on a list of remotes after update. >> +OVSDB_CHECK_IDL_C([simple idl, initially empty, set remotes], >> + [], >> + [['set-remote unix:socket' \ >> + '+set-remote unix:bad_socket,unix:socket' \ >> + '+set-remote unix:bad_socket' \ >> + '+set-remote unix:socket' \ >> + 'set-remote unix:bad_socket,unix:socket' \ >> + '+set-remote unix:socket' \ >> + '+reconnect']], >> + [[000: empty >> +001: new remotes: unix:socket, is connected: true >> +002: new remotes: unix:bad_socket,unix:socket, is connected: true >> +003: new remotes: unix:bad_socket, is connected: false >> +004: new remotes: unix:socket, is connected: false >> +005: empty >> +006: new remotes: unix:bad_socket,unix:socket, is connected: true >> +007: new remotes: unix:socket, is connected: true >> +008: reconnect >> +009: empty >> +010: done >> +]]) >> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c >> index a886f971e..93329cd4c 100644 >> --- a/tests/test-ovsdb.c >> +++ b/tests/test-ovsdb.c >> @@ -2621,6 +2621,7 @@ do_idl(struct ovs_cmdl_context *ctx) >> setvbuf(stdout, NULL, _IONBF, 0); >> >> symtab = ovsdb_symbol_table_create(); >> + const char remote_s[] = "set-remote "; >> const char cond_s[] = "condition "; >> if (ctx->argc > 2 && strstr(ctx->argv[2], cond_s)) { >> update_conditions(idl, ctx->argv[2] + strlen(cond_s)); >> @@ -2664,6 +2665,11 @@ do_idl(struct ovs_cmdl_context *ctx) >> if (!strcmp(arg, "reconnect")) { >> print_and_log("%03d: reconnect", step++); >> ovsdb_idl_force_reconnect(idl); >> + } else if (!strncmp(arg, remote_s, strlen(remote_s))) { >> + ovsdb_idl_set_remote(idl, arg + strlen(remote_s), true); >> + print_and_log("%03d: new remotes: %s, is connected: %s", > step++, >> + arg + strlen(remote_s), >> + ovsdb_idl_is_connected(idl) ? "true" : > "false"); >> } else if (!strncmp(arg, cond_s, strlen(cond_s))) { >> update_conditions(idl, arg + strlen(cond_s)); >> print_and_log("%03d: change conditions", step++); >> --- >> >> Dumitru, Han, if it looks good to you, I can squash it in before >> applying the patch. What do you think? > > Thanks Ilya. LGTM.
Looks good to me too, thanks! > >> >> Best regards, Ilya Maximets. > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev