[ovs-dev] [PATCH 2/2] ovs-appctl: Allow ovnnb_db and ovnsb_db as targets

Numan Siddique nusiddiq at redhat.com
Wed Nov 8 03:02:30 UTC 2017


Thanks Mark for the comments.


On Wed, Nov 8, 2017 at 2:30 AM, Mark Michelson <mmichels at redhat.com> wrote:

> I'm not sure what to think of this one. This fixes a problem when using
> ovn-ctl to start the databases. But I'm concerned about this change causing
> breakage for deployments that don't use ovn-ctl.
>

When ovn-ctl is used, ovnnb_db.pid and ovnsb_db.pid files are created in
/var/run/openvswitch. When the log rotate script [1] runs it would call
"ovs-appctl -t ovnnb_db .." and "ovs-appctl -t ovnsb_db .." and this fails.
I don't think it would break any thing for usecases which dont use ovn-ctl.
ovs-appctl supports providing the target as complete "socket path" which
can be used.

We could probably make this patch more generic so that when OVN db servers
are started, any value can be provided to "--unixctl". Right now it is
limited to just "ovnnb_db" and "ovnsb_db".
Eg. User can start ovsdb-server for NB DB as " ovsdb-server ...
--unixctl=nbdbctl"  and we can allow "ovs-appctl -t nbdbctl ...". Right now
it is limited to just "ovnnb_db" and "ovnsb_db".
But I am not sure if this is really required.

Thanks
Numan




[1] -
https://github.com/openvswitch/ovs/blob/master/rhel/etc_logrotate.d_openvswitch


So I guess the question is, does anyone ever actually start the OVN
> databases with something other than ovn-ctl? Am I concerned for a
> non-existent userbase :) ?
>
> "ovnsb_db" is misspelled twice in the commit message, but it's not worth
> posting another version of the patch to fix those. Whoever commits this
> change can edit those when merging.
>
> On Tue, Nov 7, 2017 at 9:32 AM <nusiddiq at redhat.com> wrote:
>
>> From: Numan Siddique <nusiddiq at redhat.com>
>>
>> ovn-ctl starts OVN DB servers with the options "--unixctl=ovnnb_db"
>> and "--unixctl=ovnsb_db". When "ovs-appctl -t ovnnb_db" is run the
>> below error is seen
>>
>> 2017-11-07T10:18:17Z|00001|unixctl|WARN|failed to connect to
>> /var/run/openvswitch/ovnnb_db.29536.ctl.
>>
>> This patch allows the targets "ovnnb_db" and ovndb_db" by not
>> appending the pid to the socket path.
>>
>> This issue has broken the openvswitch logrotate script because of
>> the invalid socket path and results in the OVN DB log files not
>> generated properly.
>>
>> Other approach to fix is to not pass "--unixctl" option, in which case
>> the ctl socket will be created as "ovsdb-server-<pid>.ctl". This would
>> break the functions 'stop_nb_ovsdb', 'stop_sb_ovsdb', 'demote_ovnnb'
>> and others. And we will not be able to use "ovnnb_db" and "ovnsd_db"
>> as target options to 'ovs-appctl'.
>>
>> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
>> ---
>>  utilities/ovs-appctl.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c
>> index 8f87cc4f6..b5a1a218b 100644
>> --- a/utilities/ovs-appctl.c
>> +++ b/utilities/ovs-appctl.c
>> @@ -210,8 +210,18 @@ connect_to_target(const char *target)
>>              ovs_fatal(-pid, "cannot read pidfile \"%s\"", pidfile_name);
>>          }
>>          free(pidfile_name);
>> -        socket_name = xasprintf("%s/%s.%ld.ctl",
>> -                                ovs_rundir(), target, (long int) pid);
>> +
>> +        /* If the target is ovnnb_db or ovnsb_db, then do not append the
>> pid
>> +         * to the socket_name. When OVN DB servers are started using
>> ovn-ctl
>> +         * they are started with the option
>> +         * "--unixctl=ovnnb_db.ctl/ovnsb_db.ctl". */
>> +        if (!strcmp(target, "ovnnb_db") || !strcmp(target, "ovnsb_db")) {
>> +            socket_name = xasprintf("%s/%s.ctl", ovs_rundir(), target);
>> +        } else {
>> +            socket_name = xasprintf("%s/%s.%ld.ctl",
>> +                                    ovs_rundir(), target, (long int)
>> pid);
>> +        }
>> +
>>  #else
>>      /* On windows, if the 'target' contains ':', we make an assumption
>> that
>>       * it is an absolute path. */
>> --
>> 2.13.5
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>


More information about the dev mailing list