[5/5] test: Add unlink_dir test

Submitted by Cyrill Gorcunov on Feb. 26, 2019, 9:13 a.m.

Details

Message ID 20190226091335.21922-6-gorcunov@gmail.com
State New
Series "Fix order problem in ghost files cleanup"
Headers show

Commit Message

Cyrill Gorcunov Feb. 26, 2019, 9:13 a.m.
To check ghost directories cleanup.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 test/zdtm/static/Makefile     |   1 +
 test/zdtm/static/unlink_dir.c | 143 ++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+)
 create mode 100644 test/zdtm/static/unlink_dir.c

Patch hide | download patch | download mbox

diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
index a31c202d5409..303ed7d40ffe 100644
--- a/test/zdtm/static/Makefile
+++ b/test/zdtm/static/Makefile
@@ -345,6 +345,7 @@  TST_DIR		=				\
 		private_bind_propagation	\
 		ghost_on_rofs			\
 		overmounted_file		\
+		unlink_dir			\
 
 TST_DIR_FILE	=				\
 		chroot				\
diff --git a/test/zdtm/static/unlink_dir.c b/test/zdtm/static/unlink_dir.c
new file mode 100644
index 000000000000..47a5673b8b5f
--- /dev/null
+++ b/test/zdtm/static/unlink_dir.c
@@ -0,0 +1,143 @@ 
+#include <stdlib.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <linux/limits.h>
+
+#include "zdtmtst.h"
+
+const char *test_doc	= "Check cleanup order of ghost directory and files inside";
+const char *test_author	= "Cyrill Gorcunov <gorcunov@gmail.com>";
+
+char *dirname;
+TEST_OPTION(dirname, string, "directory name", 1);
+
+int main(int argc, char ** argv)
+{
+	int dirfd1, dirfd2, dirfd2_dup, dirfd4;
+	char path_dir1[PATH_MAX];
+	char path_dir2[PATH_MAX];
+	char path_dir3[PATH_MAX];
+	char path_dir4[PATH_MAX];
+
+	/* Order does matter */
+	char *path_dirs[] = {
+		path_dir4,
+		path_dir3,
+		path_dir2,
+		path_dir1,
+	};
+
+	char path[PATH_MAX];
+	int fds[4], i;
+
+	int lo = ARRAY_SIZE(fds) / 2;
+	int hi = ARRAY_SIZE(fds);
+
+	test_init(argc, argv);
+
+	if (mkdir(dirname, 0700) < 0) {
+		pr_perror("Can't create directory %s", dirname);
+		return 1;
+	}
+
+	ssprintf(path_dir1, "%s/%s", dirname, "gd1");
+	if (mkdir(path_dir1, 0700) < 0) {
+		pr_perror("Can't create directory %s", path_dir1);
+		return 1;
+	}
+
+	ssprintf(path_dir2, "%s/%s/%s", dirname, "gd1", "gd2");
+	if (mkdir(path_dir2, 0700) < 0) {
+		pr_perror("Can't create directory %s", path_dir2);
+		return 1;
+	}
+
+	ssprintf(path_dir3, "%s/%s/%s/%s", dirname, "gd1", "gd2", "gd3");
+	if (mkdir(path_dir3, 0700) < 0) {
+		pr_perror("Can't create directory %s", path_dir3);
+		return 1;
+	}
+
+	ssprintf(path_dir4, "%s/%s/%s/%s/%s", dirname, "gd1", "gd2", "gd3", "gd4");
+	if (mkdir(path_dir4, 0700) < 0) {
+		pr_perror("Can't create directory %s", path_dir4);
+		return 1;
+	}
+
+	for (i = 0; i < lo; i++) {
+		ssprintf(path, "%s/%d", path_dir1, i);
+		fds[i] = open(path, O_RDONLY | O_CREAT | O_TRUNC);
+		if (fds[i] < 0) {
+			pr_perror("Can't open %s", path);
+			return 1;
+		}
+		if (unlink(path)) {
+			pr_perror("Can't unlink %s", path);
+			return 1;
+		}
+	}
+
+	dirfd2 = open(path_dir2, O_RDONLY | O_DIRECTORY);
+	if (dirfd2 < 0) {
+		pr_perror("Can't open %s", path_dir2);
+		return 1;
+	}
+
+	dirfd2_dup = dup(dirfd2);
+	if (dirfd2_dup < 0) {
+		pr_perror("Can't dup on %s\n", path_dir2);
+		return 1;
+	}
+
+	dirfd4 = open(path_dir4, O_RDONLY | O_DIRECTORY);
+	if (dirfd4 < 0) {
+		pr_perror("Can't open %s", path_dir4);
+		return 1;
+	}
+
+	for (i = lo; i < hi; i++) {
+		ssprintf(path, "%s/%d", path_dir2, i);
+		fds[i] = open(path, O_RDONLY | O_CREAT | O_TRUNC);
+		if (fds[i] < 0) {
+			pr_perror("Can't open %s", path);
+			return 1;
+		}
+		if (unlink(path)) {
+			pr_perror("Can't unlink %s", path);
+			return 1;
+		}
+	}
+
+	dirfd1 = open(path_dir1, O_RDONLY | O_DIRECTORY);
+	if (dirfd1 < 0) {
+		pr_perror("Can't open %s", path_dir1);
+		return 1;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(path_dirs); i++) {
+		if (rmdir(path_dirs[i])) {
+			pr_perror("Can't rmdir %s", path_dirs[i]);
+			return 1;
+		}
+	}
+
+	test_daemon();
+	test_waitsig();
+
+	for (i = 0; i < ARRAY_SIZE(path_dirs); i++) {
+		if (access(path_dirs[i], F_OK)) {
+			if (errno == ENOENT)
+				continue;
+			fail("Unexpected error on %s", path_dirs[i]);
+			exit(1);
+		}
+	}
+	pass();
+	return 0;
+}

Comments

Andrei Vagin Feb. 27, 2019, 6:51 a.m.
Hi Cyrill,

Looks at this patch. The test passes without C/R and fails with C/R.

@@ -3,6 +3,7 @@
 #include <errno.h>
 #include <unistd.h>
 #include <string.h>
+#include <sched.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -126,9 +127,22 @@ int main(int argc, char ** argv)
                        return 1;
                }
        }
-
+       if (mkdir(path_dir1, 0700) < 0) {
+               pr_perror("Can't create directory %s", path_dir1);
+               return 1;
+       }
+       dirfd1 = open(path_dir1, O_RDONLY | O_DIRECTORY);
+       if (dirfd1 < 0) {
+               pr_perror("Can't open %s", path_dir1);
+               return 1;
+       }
        test_daemon();
        test_waitsig();
+               if (rmdir(path_dir1)) {
+                       pr_perror("Can't rmdir %s", path_dir1);
+                       return 1;
+               }
+
 
        for (i = 0; i < ARRAY_SIZE(path_dirs); i++) {
                if (access(path_dirs[i], F_OK)) {

[root@fc24 criu]# python test/zdtm.py run -t zdtm/static/unlink_dir 
=== Run 1/1 ================ zdtm/static/unlink_dir
======================= Run zdtm/static/unlink_dir in h ========================
Start test
./unlink_dir --pidfile=unlink_dir.pid --outfile=unlink_dir.out --dirname=unlink_dir.test
Run criu dump
Run criu restore
Send the 15 signal to  36
Wait for zdtm/static/unlink_dir(36) to die for 0.100000
############### Test zdtm/static/unlink_dir FAIL at result check ###############
Test output: ================================
09:49:53.510:    36: ERR: unlink_dir.c:142: Can't rmdir unlink_dir.test/gd1 (errno = 2 (No such file or directory))

 <<< ================================
##################################### FAIL #####################################


[root@fc24 criu]# python test/zdtm.py run -t zdtm/static/unlink_dir  --iter 0
=== Run 1/1 ================ zdtm/static/unlink_dir
======================= Run zdtm/static/unlink_dir in h ========================
Start test
./unlink_dir --pidfile=unlink_dir.pid --outfile=unlink_dir.out --dirname=unlink_dir.test
Send the 15 signal to  36
Wait for zdtm/static/unlink_dir(36) to die for 0.100000
Removing dump/zdtm/static/unlink_dir/36
======================= Test zdtm/static/unlink_dir PASS =======================


On Tue, Feb 26, 2019 at 12:13:35PM +0300, Cyrill Gorcunov wrote:
> To check ghost directories cleanup.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  test/zdtm/static/Makefile     |   1 +
>  test/zdtm/static/unlink_dir.c | 143 ++++++++++++++++++++++++++++++++++
>  2 files changed, 144 insertions(+)
>  create mode 100644 test/zdtm/static/unlink_dir.c
> 
> diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
> index a31c202d5409..303ed7d40ffe 100644
> --- a/test/zdtm/static/Makefile
> +++ b/test/zdtm/static/Makefile
> @@ -345,6 +345,7 @@ TST_DIR		=				\
>  		private_bind_propagation	\
>  		ghost_on_rofs			\
>  		overmounted_file		\
> +		unlink_dir			\
>  
>  TST_DIR_FILE	=				\
>  		chroot				\
> diff --git a/test/zdtm/static/unlink_dir.c b/test/zdtm/static/unlink_dir.c
> new file mode 100644
> index 000000000000..47a5673b8b5f
> --- /dev/null
> +++ b/test/zdtm/static/unlink_dir.c
> @@ -0,0 +1,143 @@
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +#include <linux/limits.h>
> +
> +#include "zdtmtst.h"
> +
> +const char *test_doc	= "Check cleanup order of ghost directory and files inside";
> +const char *test_author	= "Cyrill Gorcunov <gorcunov@gmail.com>";
> +
> +char *dirname;
> +TEST_OPTION(dirname, string, "directory name", 1);
> +
> +int main(int argc, char ** argv)
> +{
> +	int dirfd1, dirfd2, dirfd2_dup, dirfd4;
> +	char path_dir1[PATH_MAX];
> +	char path_dir2[PATH_MAX];
> +	char path_dir3[PATH_MAX];
> +	char path_dir4[PATH_MAX];
> +
> +	/* Order does matter */
> +	char *path_dirs[] = {
> +		path_dir4,
> +		path_dir3,
> +		path_dir2,
> +		path_dir1,
> +	};
> +
> +	char path[PATH_MAX];
> +	int fds[4], i;
> +
> +	int lo = ARRAY_SIZE(fds) / 2;
> +	int hi = ARRAY_SIZE(fds);
> +
> +	test_init(argc, argv);
> +
> +	if (mkdir(dirname, 0700) < 0) {
> +		pr_perror("Can't create directory %s", dirname);
> +		return 1;
> +	}
> +
> +	ssprintf(path_dir1, "%s/%s", dirname, "gd1");
> +	if (mkdir(path_dir1, 0700) < 0) {
> +		pr_perror("Can't create directory %s", path_dir1);
> +		return 1;
> +	}
> +
> +	ssprintf(path_dir2, "%s/%s/%s", dirname, "gd1", "gd2");
> +	if (mkdir(path_dir2, 0700) < 0) {
> +		pr_perror("Can't create directory %s", path_dir2);
> +		return 1;
> +	}
> +
> +	ssprintf(path_dir3, "%s/%s/%s/%s", dirname, "gd1", "gd2", "gd3");
> +	if (mkdir(path_dir3, 0700) < 0) {
> +		pr_perror("Can't create directory %s", path_dir3);
> +		return 1;
> +	}
> +
> +	ssprintf(path_dir4, "%s/%s/%s/%s/%s", dirname, "gd1", "gd2", "gd3", "gd4");
> +	if (mkdir(path_dir4, 0700) < 0) {
> +		pr_perror("Can't create directory %s", path_dir4);
> +		return 1;
> +	}
> +
> +	for (i = 0; i < lo; i++) {
> +		ssprintf(path, "%s/%d", path_dir1, i);
> +		fds[i] = open(path, O_RDONLY | O_CREAT | O_TRUNC);
> +		if (fds[i] < 0) {
> +			pr_perror("Can't open %s", path);
> +			return 1;
> +		}
> +		if (unlink(path)) {
> +			pr_perror("Can't unlink %s", path);
> +			return 1;
> +		}
> +	}
> +
> +	dirfd2 = open(path_dir2, O_RDONLY | O_DIRECTORY);
> +	if (dirfd2 < 0) {
> +		pr_perror("Can't open %s", path_dir2);
> +		return 1;
> +	}
> +
> +	dirfd2_dup = dup(dirfd2);
> +	if (dirfd2_dup < 0) {
> +		pr_perror("Can't dup on %s\n", path_dir2);
> +		return 1;
> +	}
> +
> +	dirfd4 = open(path_dir4, O_RDONLY | O_DIRECTORY);
> +	if (dirfd4 < 0) {
> +		pr_perror("Can't open %s", path_dir4);
> +		return 1;
> +	}
> +
> +	for (i = lo; i < hi; i++) {
> +		ssprintf(path, "%s/%d", path_dir2, i);
> +		fds[i] = open(path, O_RDONLY | O_CREAT | O_TRUNC);
> +		if (fds[i] < 0) {
> +			pr_perror("Can't open %s", path);
> +			return 1;
> +		}
> +		if (unlink(path)) {
> +			pr_perror("Can't unlink %s", path);
> +			return 1;
> +		}
> +	}
> +
> +	dirfd1 = open(path_dir1, O_RDONLY | O_DIRECTORY);
> +	if (dirfd1 < 0) {
> +		pr_perror("Can't open %s", path_dir1);
> +		return 1;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(path_dirs); i++) {
> +		if (rmdir(path_dirs[i])) {
> +			pr_perror("Can't rmdir %s", path_dirs[i]);
> +			return 1;
> +		}
> +	}
> +
> +	test_daemon();
> +	test_waitsig();
> +
> +	for (i = 0; i < ARRAY_SIZE(path_dirs); i++) {
> +		if (access(path_dirs[i], F_OK)) {
> +			if (errno == ENOENT)
> +				continue;
> +			fail("Unexpected error on %s", path_dirs[i]);
> +			exit(1);
> +		}
> +	}
> +	pass();
> +	return 0;
> +}
> -- 
> 2.20.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Cyrill Gorcunov Feb. 27, 2019, 6:59 a.m.
On Tue, Feb 26, 2019 at 10:51:32PM -0800, Andrei Vagin wrote:
> Hi Cyrill,
> 
> Looks at this patch. The test passes without C/R and fails with C/R.

Thank you! Will do
Cyrill Gorcunov Feb. 27, 2019, 8:58 a.m.
On Tue, Feb 26, 2019 at 10:51:32PM -0800, Andrei Vagin wrote:
> Hi Cyrill,
> 
> Looks at this patch. The test passes without C/R and fails with C/R.

You know, it is a different issue -- the ghost directory intersects
with alive one. We never supported such things. But think we should
add a support.
Andrei Vagin Feb. 28, 2019, 6:48 a.m.
On Wed, Feb 27, 2019 at 12:58 AM Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>
> On Tue, Feb 26, 2019 at 10:51:32PM -0800, Andrei Vagin wrote:
> > Hi Cyrill,
> >
> > Looks at this patch. The test passes without C/R and fails with C/R.
>
> You know, it is a different issue -- the ghost directory intersects
> with alive one. We never supported such things. But think we should
> add a support.

If we don't support such things, criu dump has to fail. Now we corrupt
a user file system in this case, what is unacceptable...
Cyrill Gorcunov Feb. 28, 2019, 7:17 a.m.
On Wed, Feb 27, 2019 at 10:48:29PM -0800, Andrei Vagin wrote:
> On Wed, Feb 27, 2019 at 12:58 AM Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> >
> > On Tue, Feb 26, 2019 at 10:51:32PM -0800, Andrei Vagin wrote:
> > > Hi Cyrill,
> > >
> > > Looks at this patch. The test passes without C/R and fails with C/R.
> >
> > You know, it is a different issue -- the ghost directory intersects
> > with alive one. We never supported such things. But think we should
> > add a support.
> 
> If we don't support such things, criu dump has to fail. Now we corrupt
> a user file system in this case, what is unacceptable...

We *already* do, from the beginning of ghost directories support,
and to address this problem we need a significant rework of file
engine (in particular we need to remember paths collected and
find if some of ghost entries are intersected with alive ones).
I'm planning to address it once time permit. Meanwhile I sent
another light series where we only order ghost entries which
doesn't change anything but simply fix easy case.
Pavel Tikhomirov Feb. 28, 2019, 7:34 a.m.
On 2/28/19 10:17 AM, Cyrill Gorcunov wrote:
> On Wed, Feb 27, 2019 at 10:48:29PM -0800, Andrei Vagin wrote:
>> On Wed, Feb 27, 2019 at 12:58 AM Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>>>
>>> On Tue, Feb 26, 2019 at 10:51:32PM -0800, Andrei Vagin wrote:
>>>> Hi Cyrill,
>>>>
>>>> Looks at this patch. The test passes without C/R and fails with C/R.
>>>
>>> You know, it is a different issue -- the ghost directory intersects
>>> with alive one. We never supported such things. But think we should
>>> add a support.
>>
>> If we don't support such things, criu dump has to fail. Now we corrupt
>> a user file system in this case, what is unacceptable...
> 
> We *already* do, from the beginning of ghost directories support,
> and to address this problem we need a significant rework of file
> engine (in particular we need to remember paths collected and
> find if some of ghost entries are intersected with alive ones).
> I'm planning to address it once time permit. Meanwhile I sent
> another light series where we only order ghost entries which
> doesn't change anything but simply fix easy case.
> 

AFAICS We just need to fix mkdirpat_precise in v6. Just make it fail in 
errno == EEXIST branch if pos == NULL. These way mkdirpat will fail if 
alive directory exists in place of on our deleted directory at leas on 
restore.
Cyrill Gorcunov Feb. 28, 2019, 7:50 a.m.
On Thu, Feb 28, 2019 at 07:34:58AM +0000, Pavel Tikhomirov wrote:
> > 
> > We *already* do, from the beginning of ghost directories support,
> > and to address this problem we need a significant rework of file
> > engine (in particular we need to remember paths collected and
> > find if some of ghost entries are intersected with alive ones).
> > I'm planning to address it once time permit. Meanwhile I sent
> > another light series where we only order ghost entries which
> > doesn't change anything but simply fix easy case.
> > 
> 
> AFAICS We just need to fix mkdirpat_precise in v6. Just make it fail in 
> errno == EEXIST branch if pos == NULL. These way mkdirpat will fail if 
> alive directory exists in place of on our deleted directory at leas on 
> restore.

What we need is to detect such collision on _dump_ not on restore :(