| Message ID | 20220506101039.3657137-1-amorenoz@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] ovs-save: add bindir to PATH | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Hello Adrian, On Fri, May 6, 2022 at 12:11 PM Adrian Moreno <amorenoz@redhat.com> wrote: > > If openvswitch is not installed in the default system's path ovs-save > script will fail to find the tools it requires. > > Fix this by adding $bindir to the PATH. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > utilities/ovs-save | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/utilities/ovs-save b/utilities/ovs-save > index fb2025b76..b3529ed78 100755 > --- a/utilities/ovs-save > +++ b/utilities/ovs-save > @@ -20,6 +20,17 @@ case $0 in > esac > . "$dir0/ovs-lib" || exit 1 > > +for dir in "$bindir" /sbin /bin /usr/sbin /usr/bin; do > + case :$PATH: in > + *:$dir:*) ;; > + *) > + case $dir in > + $bindir) PATH=$dir:$PATH ;; > + *) PATH=$PATH:$dir ;; > + esac > + esac > +done > + Seeing how we have similar code in other scripts (ovs-ctl, ovs-kmod-ctl), would it make sense to move this to ovs-lib?
On 5/6/22 13:45, David Marchand wrote: > Hello Adrian, > > On Fri, May 6, 2022 at 12:11 PM Adrian Moreno <amorenoz@redhat.com> wrote: >> >> If openvswitch is not installed in the default system's path ovs-save >> script will fail to find the tools it requires. >> >> Fix this by adding $bindir to the PATH. >> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- >> utilities/ovs-save | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/utilities/ovs-save b/utilities/ovs-save >> index fb2025b76..b3529ed78 100755 >> --- a/utilities/ovs-save >> +++ b/utilities/ovs-save >> @@ -20,6 +20,17 @@ case $0 in >> esac >> . "$dir0/ovs-lib" || exit 1 >> >> +for dir in "$bindir" /sbin /bin /usr/sbin /usr/bin; do >> + case :$PATH: in >> + *:$dir:*) ;; >> + *) >> + case $dir in >> + $bindir) PATH=$dir:$PATH ;; >> + *) PATH=$PATH:$dir ;; >> + esac >> + esac >> +done >> + > > Seeing how we have similar code in other scripts (ovs-ctl, > ovs-kmod-ctl), would it make sense to move this to ovs-lib? > Hi David, You're right, it does make sense. I initially thought of that but I ended up discarding the idea because ovs-lib is also used in other scripts where modifying the PATH is not needed (like xenserver/etc_init.d_openvswitch) and because in this case I intentionally don't add $sbindir that is included in other scripts like ovs-ctl. But I don't have a strong opinion, and it is cleaner to move it to ovs-lib. Maybe we can create a couple of variables (e.g: OVS_BIN_PATH and OVS_SBIN_PATH) in ovs-lib and let each script use the one it needs? Thanks.
Hello Adrian, On Fri, May 6, 2022 at 3:52 PM Adrian Moreno <amorenoz@redhat.com> wrote: > >> If openvswitch is not installed in the default system's path ovs-save > >> script will fail to find the tools it requires. > >> > >> Fix this by adding $bindir to the PATH. > > > > Seeing how we have similar code in other scripts (ovs-ctl, > > ovs-kmod-ctl), would it make sense to move this to ovs-lib? > > > > You're right, it does make sense. I initially thought of that but I ended up > discarding the idea because ovs-lib is also used in other scripts where > modifying the PATH is not needed (like xenserver/etc_init.d_openvswitch) and > because in this case I intentionally don't add $sbindir that is included in > other scripts like ovs-ctl. > > But I don't have a strong opinion, and it is cleaner to move it to ovs-lib. > Maybe we can create a couple of variables (e.g: OVS_BIN_PATH and OVS_SBIN_PATH) > in ovs-lib and let each script use the one it needs? Sorry, it fell through the cracks.. Your proposal lgtm.
diff --git a/utilities/ovs-save b/utilities/ovs-save index fb2025b76..b3529ed78 100755 --- a/utilities/ovs-save +++ b/utilities/ovs-save @@ -20,6 +20,17 @@ case $0 in esac . "$dir0/ovs-lib" || exit 1 +for dir in "$bindir" /sbin /bin /usr/sbin /usr/bin; do + case :$PATH: in + *:$dir:*) ;; + *) + case $dir in + $bindir) PATH=$dir:$PATH ;; + *) PATH=$PATH:$dir ;; + esac + esac +done + usage() { UTIL=$(basename $0) cat <<EOF
If openvswitch is not installed in the default system's path ovs-save script will fail to find the tools it requires. Fix this by adding $bindir to the PATH. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- utilities/ovs-save | 11 +++++++++++ 1 file changed, 11 insertions(+)