On Wed, Nov 8, 2017 at 9:28 AM, Ben Pfaff <[email protected]> wrote: > On Wed, Nov 08, 2017 at 08:48:25AM +0530, Numan Siddique wrote: > > On Wed, Nov 8, 2017 at 8:39 AM, Ben Pfaff <[email protected]> wrote: > > > > > On Tue, Nov 07, 2017 at 09:01:27PM +0530, [email protected] wrote: > > > > From: Numan Siddique <[email protected]> > > > > > > > > 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 <[email protected]> > > > > --- > > > > 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); > > > > + } > > > > > > This is extraordinarily specific to a particular use case. It would be > > > better, I think, to have the ovs-appctl invocation pass the absolute > > > path to the target, in which case the filename will be used as-is, > > > without appending anything. > > > > > > > > > Thanks Ben. Does it makes sense to fix logrotate script here - > > https://github.com/openvswitch/ovs/blob/master/rhel/etc_logrotate.d_ > openvswitch > > (and debian version) > > to pass absolute path ? > > I think that we solve the problem by changing it to something like this: > > for ctl in /var/run/openvswitch/*.ctl; do > ovs-appctl -t "$ctl" vlog/reopen 2>/dev/null || : > done > > Are you able to test that change? If so, then it seems like a better > solution to me. >
Agree. This should work. I will test it out and respin v2. Thanks Numan _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
