[1/2] test: Add unlink_dir test

Submitted by Cyrill Gorcunov on Feb. 7, 2019, 12:10 p.m.

Details

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

Commit Message

Cyrill Gorcunov Feb. 7, 2019, 12:10 p.m.
To make sure we reorder ghost directories properly.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 test/zdtm/static/Makefile     |   1 +
 test/zdtm/static/unlink_dir.c | 106 ++++++++++++++++++++++++++++++++++
 2 files changed, 107 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 ddbc0fdd67c8..4700ac783baa 100644
--- a/test/zdtm/static/Makefile
+++ b/test/zdtm/static/Makefile
@@ -343,6 +343,7 @@  TST_DIR		=				\
 		shared_slave_mount_children	\
 		non_uniform_share_propagation	\
 		private_bind_propagation	\
+		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..1d9a19bc481c
--- /dev/null
+++ b/test/zdtm/static/unlink_dir.c
@@ -0,0 +1,106 @@ 
+#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@openvz.org>";
+
+char *dirname;
+TEST_OPTION(dirname, string, "directory name", 1);
+
+int main(int argc, char ** argv)
+{
+	int fds[4], dirfd1, dirfd2, i, len;
+	char path_dir1[PATH_MAX];
+	char path_dir2[PATH_MAX];
+	char path[PATH_MAX];
+
+	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;
+	}
+
+	len = snprintf(path_dir1, sizeof(path_dir1), "%s/%s", dirname, "gd1");
+	if (len == sizeof(path_dir1)) path_dir1[len-1] = '\0';
+	if (mkdir(path_dir1, 0700) < 0) {
+		pr_perror("Can't create directory %s", path_dir1);
+		return 1;
+	}
+
+	len = snprintf(path_dir2, sizeof(path_dir2), "%s/%s/%s", dirname, "gd1", "gd2");
+	if (len == sizeof(path_dir2)) path_dir2[len-1] = '\0';
+	if (mkdir(path_dir2, 0700) < 0) {
+		pr_perror("Can't create directory %s", path_dir2);
+		return 1;
+	}
+
+	for (i = 0; i < lo; i++) {
+		len = snprintf(path, sizeof(path), "%s/%d", path_dir1, i);
+		if (len == sizeof(path)) path[len-1] = '\0';
+		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;
+	}
+
+	for (i = lo; i < hi; i++) {
+		len = snprintf(path, sizeof(path), "%s/%d", path_dir2, i);
+		if (len == sizeof(path)) path[len-1] = '\0';
+		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;
+	}
+
+	if (rmdir(path_dir2)) {
+		pr_perror("Can't rmdir %s", path_dir2);
+		return 1;
+	}
+
+	if (rmdir(path_dir1)) {
+		pr_perror("Can't rmdir %s", path_dir1);
+		return 1;
+	}
+
+	test_daemon();
+	test_waitsig();
+
+	pass();
+	return 0;
+}

Comments

Pavel Tikhomirov Feb. 8, 2019, 8:59 a.m.
чт, 7 февр. 2019 г., 15:12 Cyrill Gorcunov gorcunov@gmail.com:

> To make sure we reorder ghost directories properly.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  test/zdtm/static/Makefile     |   1 +
>  test/zdtm/static/unlink_dir.c | 106 ++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+)
>  create mode 100644 test/zdtm/static/unlink_dir.c
>
> diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
> index ddbc0fdd67c8..4700ac783baa 100644
> --- a/test/zdtm/static/Makefile
> +++ b/test/zdtm/static/Makefile
> @@ -343,6 +343,7 @@ TST_DIR             =                               \
>                 shared_slave_mount_children     \
>                 non_uniform_share_propagation   \
>                 private_bind_propagation        \
> +               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..1d9a19bc481c
> --- /dev/null
> +++ b/test/zdtm/static/unlink_dir.c
> @@ -0,0 +1,106 @@
> +#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@openvz.org>";
> +
> +char *dirname;
> +TEST_OPTION(dirname, string, "directory name", 1);
> +
> +int main(int argc, char ** argv)
> +{
> +       int fds[4], dirfd1, dirfd2, i, len;
> +       char path_dir1[PATH_MAX];
> +       char path_dir2[PATH_MAX];
> +       char path[PATH_MAX];
> +
> +       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;
> +       }
> +
> +       len = snprintf(path_dir1, sizeof(path_dir1), "%s/%s", dirname,
> "gd1");
> +       if (len == sizeof(path_dir1)) path_dir1[len-1] = '\0';
>

Expression above is strange as snprintf always prints terminating null
byte, even if it needs to truncate the output. In other code we either
ignore the return of snprintf assuming that we can't have truncation or do
check error like: if (len >= sizeof(path_dir1)) return -1;

+       if (mkdir(path_dir1, 0700) < 0) {
> +               pr_perror("Can't create directory %s", path_dir1);
> +               return 1;
> +       }
> +
> +       len = snprintf(path_dir2, sizeof(path_dir2), "%s/%s/%s", dirname,
> "gd1", "gd2");
> +       if (len == sizeof(path_dir2)) path_dir2[len-1] = '\0';
>
/tmp/snprintf.c
same here


> +       if (mkdir(path_dir2, 0700) < 0) {
> +               pr_perror("Can't create directory %s", path_dir2);
> +               return 1;
> +       }
> +
> +       for (i = 0; i < lo; i++) {
> +               len = snprintf(path, sizeof(path), "%s/%d", path_dir1, i);
> +               if (len == sizeof(path)) path[len-1] = '\0';
>

same here


> +               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;
> +       }
> +
> +       for (i = lo; i < hi; i++) {
> +               len = snprintf(path, sizeof(path), "%s/%d", path_dir2, i);
> +               if (len == sizeof(path)) path[len-1] = '\0';
>

same here


> +               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;
> +       }
> +
> +       if (rmdir(path_dir2)) {
> +               pr_perror("Can't rmdir %s", path_dir2);
> +               return 1;
> +       }
> +
> +       if (rmdir(path_dir1)) {
> +               pr_perror("Can't rmdir %s", path_dir1);
> +               return 1;
> +       }
> +
> +       test_daemon();
> +       test_waitsig();
> +
> +       pass();
> +       return 0;
> +}
> --
> 2.20.1
>
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
>
Cyrill Gorcunov Feb. 8, 2019, 10:01 a.m.
On Fri, Feb 08, 2019 at 11:59:20AM +0300, Pavel Tikhomirov wrote:
>      +
>      +       len = snprintf(path_dir1, sizeof(path_dir1), "%s/%s", dirname,
>      "gd1");
>      +       if (len == sizeof(path_dir1)) path_dir1[len-1] = '\0';
> 
>    Expression above is strange as snprintf always prints terminating null
>    byte, even if it needs to truncate the output. In other code we either
>    ignore the return of snprintf assuming that we can't have truncation or do
>    check error like: if (len >= sizeof(path_dir1)) return -1;

It is for warnings suppression nothing else. Strictly speaking we might
need to check for negative value rather but since it is path-max limit
i don't expect here any errors at all, just to placate gcc.
Pavel Tikhomirov Feb. 8, 2019, 10:24 a.m.
пт, 8 февр. 2019 г. в 13:01, Cyrill Gorcunov <gorcunov@gmail.com>:

> On Fri, Feb 08, 2019 at 11:59:20AM +0300, Pavel Tikhomirov wrote:
> >      +
> >      +       len = snprintf(path_dir1, sizeof(path_dir1), "%s/%s",
> dirname,
> >      "gd1");
> >      +       if (len == sizeof(path_dir1)) path_dir1[len-1] = '\0';
> >
> >    Expression above is strange as snprintf always prints terminating null
> >    byte, even if it needs to truncate the output. In other code we either
> >    ignore the return of snprintf assuming that we can't have truncation
> or do
> >    check error like: if (len >= sizeof(path_dir1)) return -1;
>
> It is for warnings suppression nothing else. Strictly speaking we might
> need to check for negative value rather but since it is path-max limit
> i don't expect here any errors at all, just to placate gcc.
>

FMPOV actual overflow check is better here, it is what gcc really wants
from us by these warnings.

Except that, Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Cyrill Gorcunov Feb. 8, 2019, 12:08 p.m.
On Fri, Feb 08, 2019 at 01:24:27PM +0300, Pavel Tikhomirov wrote:
>      >    check error like: if (len >= sizeof(path_dir1)) return -1;
> 
>      It is for warnings suppression nothing else. Strictly speaking we might
>      need to check for negative value rather but since it is path-max limit
>      i don't expect here any errors at all, just to placate gcc.
> 
>    FMPOV actual overflow check is better here, it is what gcc really wants
>    from us by these warnings.
>    Except that, Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

I don't mind. Will update.