Re: [ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers
On Fri, Oct 12, 2018 at 06:02:27AM +0530, Numan Siddique wrote: > On Fri, Oct 12, 2018, 2:46 AM Ben Pfaff wrote: > > > On Tue, Oct 09, 2018 at 12:47:11PM +0530, nusid...@redhat.com wrote: > > > From: Numan Siddique > > > > > > When OVN db servers are started usinb ovn-ctl, if the pid files > > > (/var/run/openvswitch/ovnnb_db.pid for example) are already > > > present, then ovn-ctl passes "--pidfile=123" if the pid file has > > > '123' stored in it. Later on when OVN pacemaker RA script calls > > > status_ovnnb/status_ovnsb() functions, these returns "not running". > > > > > > The shell function 'pidfile_is_running()' stores the contents of > > > the pid file as "pid=`cat "$pidfile"`". If the caller also > > > uses the same variable "pid" to store the file name, it gets > > > overriden. > > > > > > This patch fixes this issue by renaming the local variable "pid" > > > in the "start_ovsdb__()" shell function to "db_file_name". > > > > > > Signed-off-by: Numan Siddique > > > > Thanks, applied to master. > > > > It would probably be a good idea to more consistently use "local". > > Using it for $pidfile and $pid in pidfile_is_running would have avoided > > this problem > > > Thanks for the review. Agree. > Is it possible to backport to 2.10 and 2.9 ? The issue is seen both the > branches. Sure. Done. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers
On Fri, Oct 12, 2018, 2:46 AM Ben Pfaff wrote: > On Tue, Oct 09, 2018 at 12:47:11PM +0530, nusid...@redhat.com wrote: > > From: Numan Siddique > > > > When OVN db servers are started usinb ovn-ctl, if the pid files > > (/var/run/openvswitch/ovnnb_db.pid for example) are already > > present, then ovn-ctl passes "--pidfile=123" if the pid file has > > '123' stored in it. Later on when OVN pacemaker RA script calls > > status_ovnnb/status_ovnsb() functions, these returns "not running". > > > > The shell function 'pidfile_is_running()' stores the contents of > > the pid file as "pid=`cat "$pidfile"`". If the caller also > > uses the same variable "pid" to store the file name, it gets > > overriden. > > > > This patch fixes this issue by renaming the local variable "pid" > > in the "start_ovsdb__()" shell function to "db_file_name". > > > > Signed-off-by: Numan Siddique > > Thanks, applied to master. > > It would probably be a good idea to more consistently use "local". > Using it for $pidfile and $pid in pidfile_is_running would have avoided > this problem Thanks for the review. Agree. Is it possible to backport to 2.10 and 2.9 ? The issue is seen both the branches. Thanks Numan . > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers
On Tue, Oct 09, 2018 at 12:47:11PM +0530, nusid...@redhat.com wrote: > From: Numan Siddique > > When OVN db servers are started usinb ovn-ctl, if the pid files > (/var/run/openvswitch/ovnnb_db.pid for example) are already > present, then ovn-ctl passes "--pidfile=123" if the pid file has > '123' stored in it. Later on when OVN pacemaker RA script calls > status_ovnnb/status_ovnsb() functions, these returns "not running". > > The shell function 'pidfile_is_running()' stores the contents of > the pid file as "pid=`cat "$pidfile"`". If the caller also > uses the same variable "pid" to store the file name, it gets > overriden. > > This patch fixes this issue by renaming the local variable "pid" > in the "start_ovsdb__()" shell function to "db_file_name". > > Signed-off-by: Numan Siddique Thanks, applied to master. It would probably be a good idea to more consistently use "local". Using it for $pidfile and $pid in pidfile_is_running would have avoided this problem. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers
Thanks Numan!
On Tue, Oct 9, 2018 at 9:17 AM wrote:
> From: Numan Siddique
>
> When OVN db servers are started usinb ovn-ctl, if the pid files
> (/var/run/openvswitch/ovnnb_db.pid for example) are already
> present, then ovn-ctl passes "--pidfile=123" if the pid file has
> '123' stored in it. Later on when OVN pacemaker RA script calls
> status_ovnnb/status_ovnsb() functions, these returns "not running".
>
> The shell function 'pidfile_is_running()' stores the contents of
> the pid file as "pid=`cat "$pidfile"`". If the caller also
> uses the same variable "pid" to store the file name, it gets
> overriden.
>
> This patch fixes this issue by renaming the local variable "pid"
> in the "start_ovsdb__()" shell function to "db_file_name".
>
> Signed-off-by: Numan Siddique
> ---
> ovn/utilities/ovn-ctl | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> index 3ff0df68e..950467c4e 100755
> --- a/ovn/utilities/ovn-ctl
> +++ b/ovn/utilities/ovn-ctl
> @@ -95,7 +95,7 @@ promote_ovnsb() {
>
> start_ovsdb__() {
> local DB=$1 db=$2 schema_name=$3 table_name=$4
> -local pid
> +local db_pid_file
> local cluster_local_addr
> local cluster_local_port
> local cluster_local_proto
> @@ -116,7 +116,7 @@ start_ovsdb__() {
> local addr
> local active_conf_file
> local use_remote_in_db
> -eval pid=\$DB_${DB}_PID
> +eval db_pid_file=\$DB_${DB}_PID
> eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
> eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
> eval cluster_local_proto=\$DB_${DB}_CLUSTER_LOCAL_PROTO
> @@ -139,7 +139,7 @@ start_ovsdb__() {
> eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
>
> # Check and eventually start ovsdb-server for DB
> -if pidfile_is_running $pid; then
> +if pidfile_is_running $db_pid_file; then
> return
> fi
>
> @@ -169,7 +169,7 @@ $cluster_remote_port
>
> set ovsdb-server
> set "$@" $log --log-file=$logfile
> -set "$@" --remote=punix:$sock --pidfile=$pid
> +set "$@" --remote=punix:$sock --pidfile=$db_pid_file
> set "$@" --unixctl=ovn${db}_db.ctl
>
> [ "$OVS_USER" != "" ] && set "$@" --user "$OVS_USER"
> --
> 2.17.1
>
> Acked-by: Daniel Alvarez
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers
From: Numan Siddique
When OVN db servers are started usinb ovn-ctl, if the pid files
(/var/run/openvswitch/ovnnb_db.pid for example) are already
present, then ovn-ctl passes "--pidfile=123" if the pid file has
'123' stored in it. Later on when OVN pacemaker RA script calls
status_ovnnb/status_ovnsb() functions, these returns "not running".
The shell function 'pidfile_is_running()' stores the contents of
the pid file as "pid=`cat "$pidfile"`". If the caller also
uses the same variable "pid" to store the file name, it gets
overriden.
This patch fixes this issue by renaming the local variable "pid"
in the "start_ovsdb__()" shell function to "db_file_name".
Signed-off-by: Numan Siddique
---
ovn/utilities/ovn-ctl | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 3ff0df68e..950467c4e 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -95,7 +95,7 @@ promote_ovnsb() {
start_ovsdb__() {
local DB=$1 db=$2 schema_name=$3 table_name=$4
-local pid
+local db_pid_file
local cluster_local_addr
local cluster_local_port
local cluster_local_proto
@@ -116,7 +116,7 @@ start_ovsdb__() {
local addr
local active_conf_file
local use_remote_in_db
-eval pid=\$DB_${DB}_PID
+eval db_pid_file=\$DB_${DB}_PID
eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
eval cluster_local_proto=\$DB_${DB}_CLUSTER_LOCAL_PROTO
@@ -139,7 +139,7 @@ start_ovsdb__() {
eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
# Check and eventually start ovsdb-server for DB
-if pidfile_is_running $pid; then
+if pidfile_is_running $db_pid_file; then
return
fi
@@ -169,7 +169,7 @@ $cluster_remote_port
set ovsdb-server
set "$@" $log --log-file=$logfile
-set "$@" --remote=punix:$sock --pidfile=$pid
+set "$@" --remote=punix:$sock --pidfile=$db_pid_file
set "$@" --unixctl=ovn${db}_db.ctl
[ "$OVS_USER" != "" ] && set "$@" --user "$OVS_USER"
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
