| Message ID | 20240322135015.14712-3-dwagner@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | refactoring and various cleanups/fixes | expand |
On Fri, Mar 22, 2024 at 02:49:59PM +0100, Daniel Wagner wrote: > When the ctl file is missing we are logging > > tests/nvme/rc: line 265: /sys/class/fcloop/ctl/del_target_port: No such file or directory > tests/nvme/rc: line 257: /sys/class/fcloop/ctl/del_local_port: No such file or directory > tests/nvme/rc: line 249: /sys/class/fcloop/ctl/del_remote_port: No such file or directory > > because the first redirect operator fails. Also it's not possible to > redirect the 'echo' error to /dev/null, because it's a builtin command > which escapes the stderr redirect operator (why?). > > Anyway, the simplest way to catch this error is to first check if the > control file exists before attempting to write to it. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > tests/nvme/rc | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/tests/nvme/rc b/tests/nvme/rc > index 78d84af72e73..865c8c351159 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -234,7 +234,10 @@ _nvme_fcloop_del_rport() { > local remote_wwpn="$4" > local loopctl=/sys/class/fcloop/ctl > > - echo "wwnn=${remote_wwnn},wwpn=${remote_wwpn}" > ${loopctl}/del_remote_port 2> /dev/null > + if [[ ! -f "${loopctl}/del_remote_port" ]]; then > + return > + fi > + echo "wwnn=${remote_wwnn},wwpn=${remote_wwpn}" > "${loopctl}/del_remote_port" BTW, I was told why the redirect doesn't work. The reason is that the 'No such file or directly' is issued by the shell and not 'echo' because 'echo' is a builtin command. Thus the right way to catch is to do (echo "foo" > file) 2>/dev/null {echo "foo" > file} 2>/dev/null I suppose we want to keep it simple and just add the brackets around the echos.
On Mar 25, 2024 / 19:27, Daniel Wagner wrote: > On Fri, Mar 22, 2024 at 02:49:59PM +0100, Daniel Wagner wrote: > > When the ctl file is missing we are logging > > > > tests/nvme/rc: line 265: /sys/class/fcloop/ctl/del_target_port: No such file or directory > > tests/nvme/rc: line 257: /sys/class/fcloop/ctl/del_local_port: No such file or directory > > tests/nvme/rc: line 249: /sys/class/fcloop/ctl/del_remote_port: No such file or directory > > > > because the first redirect operator fails. Also it's not possible to > > redirect the 'echo' error to /dev/null, because it's a builtin command > > which escapes the stderr redirect operator (why?). > > > > Anyway, the simplest way to catch this error is to first check if the > > control file exists before attempting to write to it. > > > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > > --- > > tests/nvme/rc | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/tests/nvme/rc b/tests/nvme/rc > > index 78d84af72e73..865c8c351159 100644 > > --- a/tests/nvme/rc > > +++ b/tests/nvme/rc > > @@ -234,7 +234,10 @@ _nvme_fcloop_del_rport() { > > local remote_wwpn="$4" > > local loopctl=/sys/class/fcloop/ctl > > > > - echo "wwnn=${remote_wwnn},wwpn=${remote_wwpn}" > ${loopctl}/del_remote_port 2> /dev/null > > + if [[ ! -f "${loopctl}/del_remote_port" ]]; then > > + return > > + fi > > + echo "wwnn=${remote_wwnn},wwpn=${remote_wwpn}" > "${loopctl}/del_remote_port" > > BTW, I was told why the redirect doesn't work. The reason is that the > 'No such file or directly' is issued by the shell and not 'echo' because > 'echo' is a builtin command. Thus the right way to catch is to do > > (echo "foo" > file) 2>/dev/null > {echo "foo" > file} 2>/dev/null > > I suppose we want to keep it simple and just add the brackets around the echos. Both has pros and cons: added brackets are simpler, while file existence check is more readable, IMO. I think either way is fine.
On Tue, Mar 26, 2024 at 08:39:18AM +0000, Shinichiro Kawasaki wrote: > On Mar 25, 2024 / 19:27, Daniel Wagner wrote: > (echo "foo" > file) 2>/dev/null > > {echo "foo" > file} 2>/dev/null > > > > I suppose we want to keep it simple and just add the brackets around the echos. > > Both has pros and cons: added brackets are simpler, while file existence check > is more readable, IMO. I think either way is fine. I see, so let's go with the existence check, I also find it simpler to read (understand).
diff --git a/tests/nvme/rc b/tests/nvme/rc index 78d84af72e73..865c8c351159 100644 --- a/tests/nvme/rc +++ b/tests/nvme/rc @@ -234,7 +234,10 @@ _nvme_fcloop_del_rport() { local remote_wwpn="$4" local loopctl=/sys/class/fcloop/ctl - echo "wwnn=${remote_wwnn},wwpn=${remote_wwpn}" > ${loopctl}/del_remote_port 2> /dev/null + if [[ ! -f "${loopctl}/del_remote_port" ]]; then + return + fi + echo "wwnn=${remote_wwnn},wwpn=${remote_wwpn}" > "${loopctl}/del_remote_port" } _nvme_fcloop_del_lport() { @@ -242,7 +245,10 @@ _nvme_fcloop_del_lport() { local wwpn="$2" local loopctl=/sys/class/fcloop/ctl - echo "wwnn=${wwnn},wwpn=${wwpn}" > ${loopctl}/del_local_port 2> /dev/null + if [[ ! -f "${loopctl}/del_local_port" ]]; then + return + fi + echo "wwnn=${wwnn},wwpn=${wwpn}" > "${loopctl}/del_local_port" } _nvme_fcloop_del_tport() { @@ -250,7 +256,10 @@ _nvme_fcloop_del_tport() { local wwpn="$2" local loopctl=/sys/class/fcloop/ctl - echo "wwnn=${wwnn},wwpn=${wwpn}" > ${loopctl}/del_target_port 2> /dev/null + if [[ ! -f "${loopctl}/del_target_port" ]]; then + return + fi + echo "wwnn=${wwnn},wwpn=${wwpn}" > "${loopctl}/del_target_port" } _cleanup_fcloop() {
When the ctl file is missing we are logging tests/nvme/rc: line 265: /sys/class/fcloop/ctl/del_target_port: No such file or directory tests/nvme/rc: line 257: /sys/class/fcloop/ctl/del_local_port: No such file or directory tests/nvme/rc: line 249: /sys/class/fcloop/ctl/del_remote_port: No such file or directory because the first redirect operator fails. Also it's not possible to redirect the 'echo' error to /dev/null, because it's a builtin command which escapes the stderr redirect operator (why?). Anyway, the simplest way to catch this error is to first check if the control file exists before attempting to write to it. Signed-off-by: Daniel Wagner <dwagner@suse.de> --- tests/nvme/rc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)