* [LTP] [PATCH] lib/tst_rmdir.c: close TESTDIR files before rmobj()
@ 2016-03-17 15:35 Alexey Kodanev
2016-03-17 16:15 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kodanev @ 2016-03-17 15:35 UTC (permalink / raw)
To: ltp
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
Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
lib/tst_tmpdir.c | 37 +++++++++++++++++++++++++++++++++++++
1 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index 3ea1a8b..02584da 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -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) {
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [LTP] [PATCH] lib/tst_rmdir.c: close TESTDIR files before rmobj()
2016-03-17 15:35 [LTP] [PATCH] lib/tst_rmdir.c: close TESTDIR files before rmobj() Alexey Kodanev
@ 2016-03-17 16:15 ` Cyril Hrubis
2016-03-18 12:02 ` Alexey Kodanev
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2016-03-17 16:15 UTC (permalink / raw)
To: ltp
Hi!
> 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.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] lib/tst_rmdir.c: close TESTDIR files before rmobj()
2016-03-17 16:15 ` Cyril Hrubis
@ 2016-03-18 12:02 ` Alexey Kodanev
2016-03-21 12:27 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kodanev @ 2016-03-18 12:02 UTC (permalink / raw)
To: ltp
Hi,
On 03/17/2016 07:15 PM, Cyril Hrubis wrote:
> Hi!
>> 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
> I'm not sure that it's a good idea to close them in the library this
> way.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] lib/tst_rmdir.c: close TESTDIR files before rmobj()
2016-03-18 12:02 ` Alexey Kodanev
@ 2016-03-21 12:27 ` Cyril Hrubis
2016-03-22 9:40 ` Alexey Kodanev
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2016-03-21 12:27 UTC (permalink / raw)
To: ltp
Hi!
> >> 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.
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.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] lib/tst_rmdir.c: close TESTDIR files before rmobj()
2016-03-21 12:27 ` Cyril Hrubis
@ 2016-03-22 9:40 ` Alexey Kodanev
2016-03-31 10:19 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kodanev @ 2016-03-22 9:40 UTC (permalink / raw)
To: ltp
On 03/21/2016 03:27 PM, Cyril Hrubis wrote:
> Hi!
>>>> 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.
> 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-31 10:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-17 15:35 [LTP] [PATCH] lib/tst_rmdir.c: close TESTDIR files before rmobj() Alexey Kodanev
2016-03-17 16:15 ` Cyril Hrubis
2016-03-18 12:02 ` Alexey Kodanev
2016-03-21 12:27 ` Cyril Hrubis
2016-03-22 9:40 ` Alexey Kodanev
2016-03-31 10:19 ` Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox