Hello,
The series adds support for the renameat2 system call RENAME_EXCHANGE flag (which allows to atomically replace two paths) to the vfat filesystem code.
There are many use cases for this, but we are particularly interested in making possible for vfat filesystems to be part of OSTree [0] deployments.
Currently OSTree relies on symbolic links to make the deployment updates an atomic transactional operation. But RENAME_EXCHANGE could be used [1] to achieve a similar level of robustness when using a vfat filesystem.
Patch #1 is just a preparatory patch to introduce the RENAME_EXCHANGE support, patch #2 moves some code blocks in vfat_rename() to a set of helper functions, that can be reused by tvfat_rename_exchange() that's added by patch #3 and finally patch #4 adds some kselftests to test it.
This is a v4 that addresses issues pointed out in the third version posted:
https://www.spinics.net/lists/kernel/msg4373694.html
[0]: https://github.com/ostreedev/ostree [1]: https://github.com/ostreedev/ostree/issues/1649
Changes in v4: - Add new patch from OGAWA Hirofumi to use the helpers in vfat_rename(). - Rebase the patch on top of OGAWA Hirofumi proposed changes. - Drop iversion increment for old and new file inodes (OGAWA Hirofumi). - Add Muhammad Usama Anjum Acked-by tag.
Changes in v3: - Add a .gitignore for the rename_exchange binary (Muhammad Usama Anjum). - Include $(KHDR_INCLUDES) instead of hardcoding a relative path in Makefile (Muhammad Usama Anjum).
Changes in v2: - Only update the new_dir inode version and timestamps if != old_dir (Alex Larsson). - Add some helper functions to avoid duplicating code (OGAWA Hirofumi). - Use braces for multi-lines blocks even if are one statement (OGAWA Hirofumi). - Mention in commit message that the operation is as transactional as possible but within the vfat limitations of not having a journal (Colin Walters). - Call sync to flush the page cache before checking the file contents (Alex Larsson). - Drop RFC prefix since the patches already got some review.
Javier Martinez Canillas (3): fat: add a vfat_rename2() and make existing .rename callback a helper fat: add renameat2 RENAME_EXCHANGE flag support selftests/filesystems: add a vfat RENAME_EXCHANGE test
OGAWA Hirofumi (1): fat: factor out reusable code in vfat_rename() as helper functions
MAINTAINERS | 1 + fs/fat/namei_vfat.c | 222 +++++++++++++++--- tools/testing/selftests/Makefile | 1 + .../selftests/filesystems/fat/.gitignore | 2 + .../selftests/filesystems/fat/Makefile | 7 + .../testing/selftests/filesystems/fat/config | 2 + .../filesystems/fat/rename_exchange.c | 37 +++ .../filesystems/fat/run_fat_tests.sh | 82 +++++++ 8 files changed, 315 insertions(+), 39 deletions(-) create mode 100644 tools/testing/selftests/filesystems/fat/.gitignore create mode 100644 tools/testing/selftests/filesystems/fat/Makefile create mode 100644 tools/testing/selftests/filesystems/fat/config create mode 100644 tools/testing/selftests/filesystems/fat/rename_exchange.c create mode 100755 tools/testing/selftests/filesystems/fat/run_fat_tests.sh
Add a test for the renameat2 RENAME_EXCHANGE support in vfat, but split it in a tool that just does the rename exchange and a script that is run by the kselftests framework on `make TARGETS="filesystems/fat" kselftest`.
That way the script can be easily extended to test other file operations.
The script creates a 1 MiB disk image, that is then formated with a vfat filesystem and mounted using a loop device. That way all file operations are done on an ephemeral filesystem.
Signed-off-by: Javier Martinez Canillas javierm@redhat.com Acked-by: Muhammad Usama Anjum usama.anjum@collabora.com ---
Changes in v4: - Add Muhammad Usama Anjum Acked-by tag.
Changes in v3: - Add a .gitignore for the rename_exchange binary (Muhammad Usama Anjum). - Include $(KHDR_INCLUDES) instead of hardcoding a relative path in Makefile (Muhammad Usama Anjum).
Changes in v2: - Call sync to flush the page cache before checking the file contents (Alex Larsson).
MAINTAINERS | 1 + tools/testing/selftests/Makefile | 1 + .../selftests/filesystems/fat/.gitignore | 2 + .../selftests/filesystems/fat/Makefile | 7 ++ .../testing/selftests/filesystems/fat/config | 2 + .../filesystems/fat/rename_exchange.c | 37 +++++++++ .../filesystems/fat/run_fat_tests.sh | 82 +++++++++++++++++++ 7 files changed, 132 insertions(+) create mode 100644 tools/testing/selftests/filesystems/fat/.gitignore create mode 100644 tools/testing/selftests/filesystems/fat/Makefile create mode 100644 tools/testing/selftests/filesystems/fat/config create mode 100644 tools/testing/selftests/filesystems/fat/rename_exchange.c create mode 100755 tools/testing/selftests/filesystems/fat/run_fat_tests.sh
diff --git a/MAINTAINERS b/MAINTAINERS index 4fdbbd6c1984..158771bb7755 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20841,6 +20841,7 @@ M: OGAWA Hirofumi hirofumi@mail.parknet.co.jp S: Maintained F: Documentation/filesystems/vfat.rst F: fs/fat/ +F: tools/testing/selftests/filesystems/fat/
VFIO DRIVER M: Alex Williamson alex.williamson@redhat.com diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 0aedcd76cf0f..fc59ad849a90 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -16,6 +16,7 @@ TARGETS += exec TARGETS += filesystems TARGETS += filesystems/binderfs TARGETS += filesystems/epoll +TARGETS += filesystems/fat TARGETS += firmware TARGETS += fpu TARGETS += ftrace diff --git a/tools/testing/selftests/filesystems/fat/.gitignore b/tools/testing/selftests/filesystems/fat/.gitignore new file mode 100644 index 000000000000..b89920ed841c --- /dev/null +++ b/tools/testing/selftests/filesystems/fat/.gitignore @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +rename_exchange diff --git a/tools/testing/selftests/filesystems/fat/Makefile b/tools/testing/selftests/filesystems/fat/Makefile new file mode 100644 index 000000000000..902033f6ef09 --- /dev/null +++ b/tools/testing/selftests/filesystems/fat/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 + +TEST_PROGS := run_fat_tests.sh +TEST_GEN_PROGS_EXTENDED := rename_exchange +CFLAGS += -O2 -g -Wall $(KHDR_INCLUDES) + +include ../../lib.mk diff --git a/tools/testing/selftests/filesystems/fat/config b/tools/testing/selftests/filesystems/fat/config new file mode 100644 index 000000000000..6cf95e787a17 --- /dev/null +++ b/tools/testing/selftests/filesystems/fat/config @@ -0,0 +1,2 @@ +CONFIG_BLK_DEV_LOOP=y +CONFIG_VFAT_FS=y diff --git a/tools/testing/selftests/filesystems/fat/rename_exchange.c b/tools/testing/selftests/filesystems/fat/rename_exchange.c new file mode 100644 index 000000000000..e488ad354fce --- /dev/null +++ b/tools/testing/selftests/filesystems/fat/rename_exchange.c @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Program that atomically exchanges two paths using + * the renameat2() system call RENAME_EXCHANGE flag. + * + * Copyright 2022 Red Hat Inc. + * Author: Javier Martinez Canillas javierm@redhat.com + */ + +#define _GNU_SOURCE +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> + +void print_usage(const char *program) +{ + printf("Usage: %s [oldpath] [newpath]\n", program); + printf("Atomically exchange oldpath and newpath\n"); +} + +int main(int argc, char *argv[]) +{ + int ret; + + if (argc != 3) { + print_usage(argv[0]); + exit(EXIT_FAILURE); + } + + ret = renameat2(AT_FDCWD, argv[1], AT_FDCWD, argv[2], RENAME_EXCHANGE); + if (ret) { + perror("rename exchange failed"); + exit(EXIT_FAILURE); + } + + exit(EXIT_SUCCESS); +} diff --git a/tools/testing/selftests/filesystems/fat/run_fat_tests.sh b/tools/testing/selftests/filesystems/fat/run_fat_tests.sh new file mode 100755 index 000000000000..7f35dc3d15df --- /dev/null +++ b/tools/testing/selftests/filesystems/fat/run_fat_tests.sh @@ -0,0 +1,82 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Run filesystem operations tests on an 1 MiB disk image that is formatted with +# a vfat filesystem and mounted in a temporary directory using a loop device. +# +# Copyright 2022 Red Hat Inc. +# Author: Javier Martinez Canillas javierm@redhat.com + +set -e +set -u +set -o pipefail + +BASE_DIR="$(dirname $0)" +TMP_DIR="$(mktemp -d /tmp/fat_tests_tmp.XXXX)" +IMG_PATH="${TMP_DIR}/fat.img" +MNT_PATH="${TMP_DIR}/mnt" + +cleanup() +{ + mountpoint -q "${MNT_PATH}" && unmount_image + rm -rf "${TMP_DIR}" +} +trap cleanup SIGINT SIGTERM EXIT + +create_loopback() +{ + touch "${IMG_PATH}" + chattr +C "${IMG_PATH}" >/dev/null 2>&1 || true + + truncate -s 1M "${IMG_PATH}" + mkfs.vfat "${IMG_PATH}" >/dev/null 2>&1 +} + +mount_image() +{ + mkdir -p "${MNT_PATH}" + sudo mount -o loop "${IMG_PATH}" "${MNT_PATH}" +} + +rename_exchange_test() +{ + local rename_exchange="${BASE_DIR}/rename_exchange" + local old_path="${MNT_PATH}/old_file" + local new_path="${MNT_PATH}/new_file" + + echo old | sudo tee "${old_path}" >/dev/null 2>&1 + echo new | sudo tee "${new_path}" >/dev/null 2>&1 + sudo "${rename_exchange}" "${old_path}" "${new_path}" >/dev/null 2>&1 + sudo sync -f "${MNT_PATH}" + grep new "${old_path}" >/dev/null 2>&1 + grep old "${new_path}" >/dev/null 2>&1 +} + +rename_exchange_subdir_test() +{ + local rename_exchange="${BASE_DIR}/rename_exchange" + local dir_path="${MNT_PATH}/subdir" + local old_path="${MNT_PATH}/old_file" + local new_path="${dir_path}/new_file" + + sudo mkdir -p "${dir_path}" + echo old | sudo tee "${old_path}" >/dev/null 2>&1 + echo new | sudo tee "${new_path}" >/dev/null 2>&1 + sudo "${rename_exchange}" "${old_path}" "${new_path}" >/dev/null 2>&1 + sudo sync -f "${MNT_PATH}" + grep new "${old_path}" >/dev/null 2>&1 + grep old "${new_path}" >/dev/null 2>&1 +} + +unmount_image() +{ + sudo umount "${MNT_PATH}" &> /dev/null +} + +create_loopback +mount_image +rename_exchange_test +rename_exchange_subdir_test +unmount_image + +exit 0
Hello OGAWA,
On 6/1/22 19:32, Javier Martinez Canillas wrote:
Hello,
The series adds support for the renameat2 system call RENAME_EXCHANGE flag (which allows to atomically replace two paths) to the vfat filesystem code.
There are many use cases for this, but we are particularly interested in making possible for vfat filesystems to be part of OSTree [0] deployments.
Currently OSTree relies on symbolic links to make the deployment updates an atomic transactional operation. But RENAME_EXCHANGE could be used [1] to achieve a similar level of robustness when using a vfat filesystem.
Patch #1 is just a preparatory patch to introduce the RENAME_EXCHANGE support, patch #2 moves some code blocks in vfat_rename() to a set of helper functions, that can be reused by tvfat_rename_exchange() that's added by patch #3 and finally patch #4 adds some kselftests to test it.
I think that addressed all the issues you pointed out in v3, please let me know if there's anything else that is needed for this patch series.
Would these be merged by you or should I ping someone else? I'm not that familiar with how filesystem patches make into the mainline tree.
Javier Martinez Canillas javierm@redhat.com writes:
Hello OGAWA,
On 6/1/22 19:32, Javier Martinez Canillas wrote:
Hello,
The series adds support for the renameat2 system call RENAME_EXCHANGE flag (which allows to atomically replace two paths) to the vfat filesystem code.
There are many use cases for this, but we are particularly interested in making possible for vfat filesystems to be part of OSTree [0] deployments.
Currently OSTree relies on symbolic links to make the deployment updates an atomic transactional operation. But RENAME_EXCHANGE could be used [1] to achieve a similar level of robustness when using a vfat filesystem.
Patch #1 is just a preparatory patch to introduce the RENAME_EXCHANGE support, patch #2 moves some code blocks in vfat_rename() to a set of helper functions, that can be reused by tvfat_rename_exchange() that's added by patch #3 and finally patch #4 adds some kselftests to test it.
I think that addressed all the issues you pointed out in v3, please let me know if there's anything else that is needed for this patch series.
Would these be merged by you or should I ping someone else? I'm not that familiar with how filesystem patches make into the mainline tree.
Sorry, it is just the my issue. I was traveling latest week, so is not reviewing yet. I'll do soon.
Thanks.
On 6/8/22 11:42, OGAWA Hirofumi wrote:
Javier Martinez Canillas javierm@redhat.com writes:
Hello OGAWA,
On 6/1/22 19:32, Javier Martinez Canillas wrote:
Hello,
The series adds support for the renameat2 system call RENAME_EXCHANGE flag (which allows to atomically replace two paths) to the vfat filesystem code.
There are many use cases for this, but we are particularly interested in making possible for vfat filesystems to be part of OSTree [0] deployments.
Currently OSTree relies on symbolic links to make the deployment updates an atomic transactional operation. But RENAME_EXCHANGE could be used [1] to achieve a similar level of robustness when using a vfat filesystem.
Patch #1 is just a preparatory patch to introduce the RENAME_EXCHANGE support, patch #2 moves some code blocks in vfat_rename() to a set of helper functions, that can be reused by tvfat_rename_exchange() that's added by patch #3 and finally patch #4 adds some kselftests to test it.
I think that addressed all the issues you pointed out in v3, please let me know if there's anything else that is needed for this patch series.
Would these be merged by you or should I ping someone else? I'm not that familiar with how filesystem patches make into the mainline tree.
Sorry, it is just the my issue. I was traveling latest week, so is not reviewing yet. I'll do soon.
Sure, there's no rush. I just wanted to confirm that. Thanks a lot!
linux-kselftest-mirror@lists.linaro.org
-
Javier Martinez Canillas -
OGAWA Hirofumi