macsec: fix double delete the tmp file

Message ID 20180609104138.8375-1-sunlw.fnst@cn.fujitsu.com
State Rejected
Delegated to: Petr Vorel
Headers show
Series
  • macsec: fix double delete the tmp file
Related show

Commit Message

Sun Lianwen June 9, 2018, 10:41 a.m.
The virt_cleanup and tst_ipsec_cleanup in both in the cleanup() which will 
double delete the tmp file. and result to report below info:
"rm: cannot remove ‘/tmp/ltp-ddLUcf8ACl/macsec01.6YQ9qGRNuC’: No such file 
or directory"

Signed-off-by: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
---
 testcases/network/virt/macsec01.sh | 10 +++++++++-
 testcases/network/virt/macsec02.sh | 12 ++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

Comments

Petr Vorel June 9, 2018, 3:06 p.m. | #1
Hi Sun,

> The virt_cleanup and tst_ipsec_cleanup in both in the cleanup() which will 
> double delete the tmp file. and result to report below info:
> "rm: cannot remove ‘/tmp/ltp-ddLUcf8ACl/macsec01.6YQ9qGRNuC’: No such file 
> or directory"
Thanks for your patch. I've noticed that, but didn't try to fix that as macsec
tests run under new API doesn't complain, because tst_test.sh (unlike test.sh)
actually check whether directory exist. (Next week hopefully I'm going to send
patch which rewrites tests which uses ipsec_lib.sh and virt_lib.sh).

Your test introduces duplicity with tst_ipsec_cleanup, it's here 3 times
+ code in macsec02.sh uses spaces instead of tabs.

As the issue isn't specific and it's going to be fixed just by migrating to new
API, I'd propose not to accept it. Alexey, what do you think?


Kind regards,
Petr
Sun Lianwen June 10, 2018, 11:51 p.m. | #2
Hi Petr

On 06/09/2018 11:06 PM, Petr Vorel wrote:
> Hi Sun,
> 
>> The virt_cleanup and tst_ipsec_cleanup in both in the cleanup() which will 
>> double delete the tmp file. and result to report below info:
>> "rm: cannot remove ‘/tmp/ltp-ddLUcf8ACl/macsec01.6YQ9qGRNuC’: No such file 
>> or directory"
> Thanks for your patch. I've noticed that, but didn't try to fix that as macsec
> tests run under new API doesn't complain, because tst_test.sh (unlike test.sh)
> actually check whether directory exist. (Next week hopefully I'm going to send
> patch which rewrites tests which uses ipsec_lib.sh and virt_lib.sh).
> 
> Your test introduces duplicity with tst_ipsec_cleanup, it's here 3 times
> + code in macsec02.sh uses spaces instead of tabs.
> 
> As the issue isn't specific and it's going to be fixed just by migrating to new
> API, I'd propose not to accept it. Alexey, what do you think?
> 
Thanks your answer, I think you are right. It is nice if the new API can fix this.
> 
> Kind regards,
> Petr
> 
> 
> .
>

Patch

diff --git a/testcases/network/virt/macsec01.sh b/testcases/network/virt/macsec01.sh
index 52d6f071a..1a3499648 100755
--- a/testcases/network/virt/macsec01.sh
+++ b/testcases/network/virt/macsec01.sh
@@ -31,7 +31,15 @@  VIRT_PERF_THRESHOLD=${VIRT_PERF_THRESHOLD:-100}
 cleanup()
 {
 	virt_cleanup
-	tst_ipsec_cleanup
+
+	ip xfrm state flush
+	ip xfrm policy flush
+	tst_rhost_run -c "ip xfrm state flush && ip xfrm policy flush"
+
+	if [ -n "$cleanup_vti" ]; then
+		ip li del $cleanup_vti 2>/dev/null
+		tst_rhost_run -c "ip li del $cleanup_vti 2>/dev/null"
+	fi
 }
 TST_CLEANUP="cleanup"
 
diff --git a/testcases/network/virt/macsec02.sh b/testcases/network/virt/macsec02.sh
index 617860d41..6276a6b09 100755
--- a/testcases/network/virt/macsec02.sh
+++ b/testcases/network/virt/macsec02.sh
@@ -30,8 +30,16 @@  VIRT_PERF_THRESHOLD=${VIRT_PERF_THRESHOLD:-100}
 
 cleanup()
 {
-	virt_cleanup
-	tst_ipsec_cleanup
+    virt_cleanup
+
+    ip xfrm state flush
+    ip xfrm policy flush
+    tst_rhost_run -c "ip xfrm state flush && ip xfrm policy flush"
+
+    if [ -n "$cleanup_vti" ]; then
+        ip li del $cleanup_vti 2>/dev/null
+        tst_rhost_run -c "ip li del $cleanup_vti 2>/dev/null"
+    fi
 }
 TST_CLEANUP="cleanup"