lib/tst_rmdir.c: close TESTDIR files before rmobj()
PATCH 1/1 lib/tst_rmdir.c: close TESTDIR files before rmobj()
NFS creates special .nfs* files when test file was removed and left open. In that case, tst_rmdir() fails to remove test dirs:
fstatat01 TWARN : tst_tmpdir.c:260: tst_rmdir:
rmobj(/tmp/netpan-10380/mnt/fst9dM6nA) failed:
remove(/tmp/netpan-10380/mnt/fst9dM6nA/fstatattestdir) failed; errno=39: Directory not empty
--- lib/tst_tmpdir.c | 37 +++++++++++++++++++++++++++++++++++++ 1 files changed, 37 insertions(+), 0 deletions(-)
lib/tst_tmpdir.c +37 -0
@@ -67,6 +67,7 @@#include <stdlib.h> #include <string.h> #include <unistd.h> +#include <dirent.h> #include "test.h" #include "rmobj.h"
@@ -188,6 +189,35 @@ void tst_tmpdir(void) } } +static void tmpdir_close_fds (void ) +{ +int fd; +char buf[PATH_MAX ], fd_path[64 ]; +struct dirent *entry; + +DIR *dir =opendir ("/proc/self/fd/" ); + +while ((entry =readdir (dir)) !=NULL ) { +const char *file = entry->d_name; + +if (!strcmp (file,".." ) || !strcmp (file,"." )) +continue ; + + fd =atoi (file); + +if (sprintf (fd_path,"/proc/self/fd/%d" , fd) <=0 ) +continue ; + +if (readlink (fd_path, buf,PATH_MAX ) <0 ) +continue ; + +if (!strncmp (buf,TESTDIR ,strlen (TESTDIR ))) +close (fd); + } + +closedir (dir); +} +void tst_rmdir (void ) {char *errmsg;
@@ -212,6 +242,13 @@ void tst_rmdir(void) }/* +* Close open files in TESTDIR before rmobj(). +* When running on NFS, directories might have .nfs* files, which appear +* if a file was removed but left open. +*/ +tmpdir_close_fds (); + +/* * Attempt to remove the "TESTDIR" directory, using rmobj(). */ if (rmobj (TESTDIR , &errmsg) ==-1 ) {
Signature · 1 line
1.7.1
PATCH 1/1 lib/tst_rmdir.c: close TESTDIR files before rmobj()
Hi!
Quoted 2 lines
NFS creates special .nfs* files when test file was removed and left open. In that case,
tst_rmdir()fails to remove test dirs:
This is the reason why we state in test writing guidelines that all open file descriptors should be closed in the cleanup() function.
I'm not sure that it's a good idea to close them in the library this way.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 lib/tst_rmdir.c: close TESTDIR files before rmobj()
Hi,
Quoted 5 lines · Cyril Hrubis
Hi!
Quoted 2 lines
NFS creates special .nfs* files when test file was removed and left open. In that case,
tst_rmdir()fails to remove test dirs:This is the reason why we state in test writing guidelines that all open file descriptors should be closed in the
cleanup()function.
Most tests are not doing it and just call tst_rmdir() in cleanup. When a testfails in the middle, e.g. with tst_brkm(TFAIL,...), it will getworthlessTWARN along with TFAIL.
Some tests closes fds in cleanup, and it means that they set global vars to keep track of open files. I think, itcomplicates a little bit writing and reading such tests.So that is why I think it's better to have such cleanup operations insidethe library.
Thanks,
Alexey
Quoted 2 lines
I'm not sure that it's a good idea to close them in the library this way.
PATCH 1/1 lib/tst_rmdir.c: close TESTDIR files before rmobj()
Hi!
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
NFS creates special .nfs* files when test file was removed and left open. In that case,
tst_rmdir()fails to remove test dirs:This is the reason why we state in test writing guidelines that all open file descriptors should be closed in the
cleanup()function.
Quoted 8 lines
Most tests are not doing it and just call
tst_rmdir()in cleanup. When a testfails in the middle, e.g. withtst_brkm(TFAIL,...), it will getworthlessTWARN along with TFAIL.Some tests closes fds in cleanup, and it means that they set global vars to keep track of open files. I think, itcomplicates a little bit writing and reading such tests.So that is why I think it's better to have such cleanup operations insidethe library.
I'm not saying this is a bad idea. I'm not 100% sure if there are any side effects. Maybe we can add a close_fds flag to the test structure in the new test library and enable it only when it's needed.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 lib/tst_rmdir.c: close TESTDIR files before rmobj()
Quoted 9 lines · Cyril Hrubis
Hi!
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
NFS creates special .nfs* files when test file was removed and left open. In that case,
tst_rmdir()fails to remove test dirs:This is the reason why we state in test writing guidelines that all open file descriptors should be closed in the
cleanup()function.Most tests are not doing it and just call
tst_rmdir()in cleanup. When a testfails in the middle, e.g. withtst_brkm(TFAIL,...), it will getworthlessTWARN along with TFAIL.
Quoted 7 lines
Quoted 4 lines
Some tests closes fds in cleanup, and it means that they set global vars to keep track of open files. I think, itcomplicates a little bit writing and reading such tests.So that is why I think it's better to have such cleanup operations insidethe library.
I'm not saying this is a bad idea. I'm not 100% sure if there are any side effects. Maybe we can add a
close_fdsflag to the test structure in the new test library and enable it only when it's needed.
OK, may be it's a good idea. So, should I wait till the merge or send youa new patch for your repo first?
Thanks,
Alexey
PATCH 1/1 lib/tst_rmdir.c: close TESTDIR files before rmobj()
Hi!
Quoted 2 lines
OK, may be it's a good idea. So, should I wait till the merge or send youa new patch for your repo first?
Given that the new library patch may still change let's wait till it's mergerd before we start adding more functionality to the new test library.
Signature · 2 lines
Cyril Hrubis [email protected]