On 7/15/21 8:31 PM, Dumitru Ceara wrote:
> 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!
Applied. Thanks!
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
